Skip to content

Commit 5669baf

Browse files
authored
Merge pull request #30993 from redpanda-data/ct/notickect/space-mgmt
storage: admit tiered_cloud partitions to space management
2 parents bbada32 + d673b6b commit 5669baf

6 files changed

Lines changed: 185 additions & 23 deletions

File tree

src/v/cloud_topics/level_zero/stm/ctp_stm.cc

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,9 @@ ss::future<> ctp_stm::prefix_truncate_bg() {
116116
// trims aggressively (the local log only holds placeholders);
117117
// storage.mode=tiered_cloud honors local retention plus the compaction
118118
// floor and so keeps more data locally.
119+
const bool is_tiered = _raft->log()->config().is_tiered_cloud();
119120
model::offset target;
120-
if (_raft->log()->config().is_tiered_cloud()) {
121+
if (is_tiered) {
121122
target = co_await compute_local_retention_offset();
122123
} else {
123124
// storage.mode=cloud keeps only placeholders locally; the
@@ -133,10 +134,12 @@ ss::future<> ctp_stm::prefix_truncate_bg() {
133134
_raft->last_snapshot_index());
134135
try {
135136
if (
136-
_raft->last_snapshot_index() >= target
137-
&& _active_readers.empty()) {
137+
_raft->last_snapshot_index() >= target && _active_readers.empty()
138+
&& !is_tiered) {
138139
// Only wait without a timeout if there are no active readers
139-
// that could be holding us back.
140+
// that could be holding us back. tiered_cloud always polls
141+
// because its space-management offset is set without signalling
142+
// _lro_advanced.
140143
co_await _lro_advanced.wait();
141144
} else {
142145
co_await _lro_advanced.wait(retry_backoff_time);

src/v/resource_mgmt/storage.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,9 @@ eviction_policy::collect_reclaimable_offsets() {
289289
*/
290290
chunked_vector<ss::lw_shared_ptr<cluster::partition>> partitions;
291291
for (const auto& p : _pm->local().partitions()) {
292-
if (!p.second->remote_partition()) {
292+
if (
293+
!p.second->remote_partition()
294+
&& !p.second->log()->config().is_tiered_cloud()) {
293295
continue;
294296
}
295297
partitions.push_back(p.second);

src/v/storage/disk_log_impl.cc

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1228,9 +1228,9 @@ disk_log_impl::maybe_apply_local_storage_overrides(gc_config cfg) const {
12281228

12291229
// cloud_retention is disabled, do not override. Cloud-topic partitions
12301230
// (storage.mode in {cloud, tiered_cloud}) bypass this gate:
1231-
// is_cloud_retention_active() is false for them, but ctp_stm still needs
1231+
// is_archival_active() is false for them, but ctp_stm still needs
12321232
// the local-target override to engage under retention_local_strict.
1233-
if (!is_cloud_retention_active() && !config().cloud_topic_enabled()) {
1233+
if (!is_archival_active() && !config().cloud_topic_enabled()) {
12341234
return cfg;
12351235
}
12361236

@@ -1310,11 +1310,15 @@ gc_config disk_log_impl::apply_local_storage_overrides(gc_config cfg) const {
13101310
return cfg;
13111311
}
13121312

1313-
bool disk_log_impl::is_cloud_retention_active() const {
1313+
bool disk_log_impl::is_archival_active() const {
13141314
return config::shard_local_cfg().cloud_storage_enabled()
13151315
&& (config().is_archival_enabled());
13161316
}
13171317

1318+
bool disk_log_impl::is_cloud_gc_active() const {
1319+
return is_archival_active() || config().is_tiered_cloud();
1320+
}
1321+
13181322
/*
13191323
* applies overrides for non-cloud storage settings
13201324
*/
@@ -1769,6 +1773,10 @@ ss::future<> disk_log_impl::gc(gc_config cfg) {
17691773

17701774
ss::future<std::optional<model::offset>> disk_log_impl::do_gc(gc_config cfg) {
17711775
vassert(!_closed, "gc on closed log - {}", *this);
1776+
vassert(
1777+
!config().cloud_topic_enabled(),
1778+
"[{}] gc on cloud topic partition",
1779+
config().ntp());
17721780

17731781
cfg = apply_overrides(cfg);
17741782

@@ -1783,7 +1791,7 @@ ss::future<std::optional<model::offset>> disk_log_impl::do_gc(gc_config cfg) {
17831791
const auto offset = _cloud_gc_offset.value();
17841792
_cloud_gc_offset.reset();
17851793

1786-
if (!is_cloud_retention_active()) {
1794+
if (!is_archival_active()) {
17871795
vlog(
17881796
gclog.warn,
17891797
"[{}] expected remote retention to be active",
@@ -1916,12 +1924,12 @@ disk_log_impl::compute_gc_offset(gc_config cfg) {
19161924
// ctp_stm for cloud-topic partitions. The offset is retention-driven
19171925
// unless space management has pinned _cloud_gc_offset, which then takes
19181926
// precedence. maybe_apply_local_storage_overrides
1919-
// bypasses the is_cloud_retention_active() gate for cloud-topic partitions
1927+
// bypasses the is_archival_active() gate for cloud-topic partitions
19201928
// (the gate is false for storage.mode in {cloud, tiered_cloud}), so the
19211929
// local-target override engages there under retention_local_strict.
19221930
cfg = apply_kafka_retention_overrides(cfg);
19231931
if (_cloud_gc_offset.has_value()) {
1924-
co_return _cloud_gc_offset;
1932+
co_return std::exchange(_cloud_gc_offset, std::nullopt);
19251933
}
19261934
co_await maybe_adjust_retention_timestamps();
19271935
cfg = maybe_apply_local_storage_overrides(cfg);
@@ -3921,7 +3929,7 @@ disk_log_impl::disk_usage_and_reclaimable_space(gc_config input_cfg) {
39213929
&& seg->offsets().get_dirty_offset() <= retention_offset.value()) {
39223930
retention_segments.push_back(seg);
39233931
} else if (
3924-
is_cloud_retention_active()
3932+
is_cloud_gc_active()
39253933
&& seg->offsets().get_dirty_offset() <= max_removable) {
39263934
available_segments.push_back(seg);
39273935
} else {
@@ -3937,8 +3945,8 @@ disk_log_impl::disk_usage_and_reclaimable_space(gc_config input_cfg) {
39373945
* get_reclaimable_offsets is going to be merged together.
39383946
*/
39393947
if (
3940-
!config().is_read_replica_mode_enabled()
3941-
&& is_cloud_retention_active() && seg != _segs.back()
3948+
!config().is_read_replica_mode_enabled() && is_cloud_gc_active()
3949+
&& seg != _segs.back()
39423950
&& seg->offsets().get_dirty_offset() <= max_removable
39433951
&& local_retention_offset.has_value()
39443952
&& seg->offsets().get_dirty_offset()
@@ -4279,8 +4287,8 @@ ss::future<usage_report> disk_log_impl::disk_usage(gc_config cfg) {
42794287
chunked_vector<ss::lw_shared_ptr<segment>>
42804288
disk_log_impl::cloud_gc_eligible_segments() {
42814289
vassert(
4282-
is_cloud_retention_active(),
4283-
"Expected {} to have cloud retention enabled",
4290+
is_cloud_gc_active(),
4291+
"Expected cloud GC to be active for {}",
42844292
config().ntp());
42854293

42864294
constexpr size_t keep_segs = 1;
@@ -4313,7 +4321,7 @@ disk_log_impl::cloud_gc_eligible_segments() {
43134321
}
43144322

43154323
void disk_log_impl::set_cloud_gc_offset(model::offset offset) {
4316-
if (!is_cloud_retention_active()) {
4324+
if (!is_cloud_gc_active()) {
43174325
vlog(
43184326
stlog.debug,
43194327
"Ignoring request to set GC offset on non-cloud enabled partition "
@@ -4338,7 +4346,7 @@ disk_log_impl::get_reclaimable_offsets(gc_config cfg) {
43384346

43394347
reclaimable_offsets res;
43404348

4341-
if (!is_cloud_retention_active()) {
4349+
if (!is_cloud_gc_active()) {
43424350
vlog(
43434351
stlog.debug,
43444352
"Reporting no reclaimable offsets for non-cloud partition {}",
@@ -4540,7 +4548,7 @@ size_t disk_log_impl::reclaimable_size_bytes() const {
45404548
* local retention size may change. catch these before reporting potentially
45414549
* stale information.
45424550
*/
4543-
if (!is_cloud_retention_active()) {
4551+
if (!is_cloud_gc_active()) {
45444552
return 0;
45454553
}
45464554
if (config().is_read_replica_mode_enabled()) {

src/v/storage/disk_log_impl.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,9 @@ class disk_log_impl final : public log {
210210
/// Applies retention overrides (callers need not pre-apply them) and
211211
/// adjusts bogus (future) retention timestamps, mutating segment indexes
212212
/// when an entire segment is bogus, then returns the offset GC would
213-
/// evict to -- usually derived from local retention, but when space
214-
/// management has set _cloud_gc_offset that offset is returned instead
215-
/// (without resetting it -- that is do_gc's responsibility).
213+
/// evict to. Usually derived from local retention; when space management
214+
/// has set _cloud_gc_offset, that offset is returned and consumed (reset)
215+
/// here, so the caller is responsible for acting on it.
216216
ss::future<std::optional<model::offset>>
217217
compute_gc_offset(gc_config cfg) final;
218218

@@ -421,7 +421,11 @@ class disk_log_impl final : public log {
421421
gc_config maybe_apply_local_storage_overrides(gc_config) const;
422422
gc_config apply_local_storage_overrides(gc_config) const;
423423

424-
bool is_cloud_retention_active() const;
424+
bool is_archival_active() const;
425+
// True when local segments are a reclaimable cache of cloud-resident data
426+
// (legacy tiered storage or tiered_cloud); broader than
427+
// is_archival_active(), which is archival-only.
428+
bool is_cloud_gc_active() const;
425429

426430
// returns retention_offset(cfg) but may also first apply adjustments to
427431
// future timestamps if this option is turned on in configuration.

src/v/storage/log_manager.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,13 @@ log_manager::housekeeping_scan(model::timestamp collection_threshold) {
292292

293293
current_log.flags |= bflags::compaction_checked;
294294

295+
// Cloud topics manage their local log via ctp_stm (compaction is at
296+
// L1), so skip gc/compaction housekeeping for them. segment.ms rolling
297+
// ran above and still applies.
298+
if (current_log.handle->config().cloud_topic_enabled()) {
299+
continue;
300+
}
301+
295302
// Hold the housekeeping gate to prevent issues with concurrent removal
296303
// of the log meta.
297304
auto gate = current_log.housekeeping_gate.hold();
@@ -510,6 +517,12 @@ ss::future<> log_manager::gc_loop() {
510517
continue;
511518
}
512519

520+
// Cloud topics are gc'd by ctp_stm, not here; exclude them from
521+
// the reclaim estimate (segment.ms above still applies).
522+
if (log_meta.handle->config().cloud_topic_enabled()) {
523+
continue;
524+
}
525+
513526
auto ntp = log_meta.handle->config().ntp();
514527
auto usage = co_await log_meta.handle->disk_usage(
515528
gc_config(lowest_ts_to_retain(), _config.retention_bytes()));

tests/rptest/tests/tiered_cloud_local_retention_test.py

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,138 @@ def test_local_data_grows_in_tiered_cloud(self):
299299
f"expected near {replication * self.local_target_bytes} bytes"
300300
)
301301

302+
@cluster(num_nodes=4)
303+
def test_capacity_pressure_reclaims_tiered_cloud(self):
304+
"""
305+
Cluster-wide disk pressure (the disk_space_manager capacity path)
306+
reclaims a tiered_cloud partition's local log, not just the
307+
retention.local.target path.
308+
309+
The topic sets no local target and effectively infinite retention, so
310+
only the capacity path can bound it. On a build that excludes
311+
tiered_cloud partitions from disk_space_manager the local log grows
312+
unbounded and this fails; with them admitted, the eviction policy pins
313+
_cloud_gc_offset and ctp_stm trims to it, so the footprint converges
314+
near the capacity target.
315+
"""
316+
assert self.redpanda is not None
317+
318+
# Node-wide log-data target, small enough that the produced topic must
319+
# be trimmed to meet it. disk_reservation_percent=0 and percent=100 so
320+
# the bytes target is the effective one.
321+
capacity_target = 16 * 1024 * 1024
322+
self.redpanda.set_cluster_config(
323+
{
324+
"retention_local_target_capacity_bytes": capacity_target,
325+
"retention_local_target_capacity_percent": 100,
326+
"disk_reservation_percent": 0,
327+
}
328+
)
329+
330+
# No retention.local.target.bytes: the local-target path stays inert
331+
# (only the 1-day default applies, and nothing is that old), so only
332+
# the capacity path can bound this topic. retention.bytes is
333+
# effectively unlimited, mirroring the field scenario that filled the
334+
# disk.
335+
rpk = RpkTool(self.redpanda)
336+
rpk.create_topic(
337+
topic=self.topic_name,
338+
partitions=1,
339+
replicas=3,
340+
config={
341+
TopicSpec.PROPERTY_STORAGE_MODE: (TopicSpec.STORAGE_MODE_TIERED_CLOUD),
342+
"cleanup.policy": TopicSpec.CLEANUP_DELETE,
343+
"retention.bytes": str(self.retention_bytes),
344+
"segment.bytes": str(self.segment_size),
345+
},
346+
)
347+
self._wait_for_partition_info()
348+
349+
# Produce well past the capacity target so an untrimmed build grows far
350+
# beyond it.
351+
produce_bytes = 64 * 1024 * 1024
352+
self._produce(produce_bytes)
353+
self.wait_until_reconciled(topic=self.topic_name, partition=0)
354+
355+
# The target is per node; the metric sums this topic's 3 replicas and
356+
# excludes internal logs (topic-filtered). Allow 3x the target plus
357+
# per-replica headroom for the never-evicted active segment and
358+
# trim-cycle latency. Still far below the ~3x produce an untrimmed
359+
# build retains.
360+
replication = 3
361+
ceiling = replication * (capacity_target + 4 * self.segment_size)
362+
# The topic has no local retention target and a lenient retention.bytes,
363+
# so ctp_stm's own retention never trims at this volume; only a
364+
# disk_space_manager capacity pin can drive the reclaim. Staying below
365+
# the ceiling therefore proves the capacity path engaged for a
366+
# tiered_cloud partition.
367+
self._wait_local_below(ceiling_bytes=ceiling, timeout_sec=180)
368+
369+
@cluster(num_nodes=4)
370+
def test_idle_partition_reclaims_on_pin(self):
371+
"""
372+
A tiered_cloud partition that has gone idle still reclaims when disk
373+
pressure sets _cloud_gc_offset afterwards.
374+
375+
With no produce the LRO is quiescent, so nothing signals ctp_stm's
376+
truncate loop. It wakes on its bounded poll interval, re-reads the
377+
offset disk_space_manager set, and trims to it, so an idle partition
378+
reclaims without further produce.
379+
"""
380+
assert self.redpanda is not None
381+
382+
capacity_target = 16 * 1024 * 1024
383+
replication = 3
384+
ceiling = replication * (capacity_target + 4 * self.segment_size)
385+
386+
# Capacity path effectively off: percent=100 with no bytes target lets
387+
# the backlog accumulate without pressure, so nothing trims until the
388+
# bytes target is set below.
389+
self.redpanda.set_cluster_config(
390+
{
391+
"retention_local_target_capacity_percent": 100,
392+
"disk_reservation_percent": 0,
393+
}
394+
)
395+
396+
rpk = RpkTool(self.redpanda)
397+
rpk.create_topic(
398+
topic=self.topic_name,
399+
partitions=1,
400+
replicas=3,
401+
config={
402+
TopicSpec.PROPERTY_STORAGE_MODE: (TopicSpec.STORAGE_MODE_TIERED_CLOUD),
403+
"cleanup.policy": TopicSpec.CLEANUP_DELETE,
404+
"retention.bytes": str(self.retention_bytes),
405+
"segment.bytes": str(self.segment_size),
406+
},
407+
)
408+
self._wait_for_partition_info()
409+
410+
# Fill the local log, then let reconciliation catch up so the LRO is
411+
# quiescent (nothing to drive the truncate loop). Nothing trims yet
412+
# (no pressure, lenient retention.bytes), so the footprint holds near
413+
# the produced size.
414+
self._produce(64 * 1024 * 1024)
415+
self.wait_until_reconciled(topic=self.topic_name, partition=0)
416+
417+
# Confirm the backlog is present and untrimmed before applying pressure,
418+
# so the reclaim assertion below cannot pass trivially.
419+
wait_until(
420+
lambda: self._local_partition_bytes() > ceiling,
421+
timeout_sec=60,
422+
backoff_sec=2,
423+
err_msg="backlog never reached the expected size before pressure",
424+
)
425+
426+
# Apply pressure: disk_space_manager sets _cloud_gc_offset on its next
427+
# cycle. The LRO is quiescent, so the truncate loop only observes it on
428+
# its next bounded poll, then trims to it.
429+
self.redpanda.set_cluster_config(
430+
{"retention_local_target_capacity_bytes": capacity_target}
431+
)
432+
self._wait_local_below(ceiling_bytes=ceiling, timeout_sec=120)
433+
302434
@cluster(num_nodes=4)
303435
def test_compact_topic_evicts_via_l1_floor(self):
304436
"""

0 commit comments

Comments
 (0)