storage: admit tiered_cloud partitions to space management

[oleiman]

tiered_cloud (cloud topics / TSv2) partitions were excluded from disk space
management, so their local log was never reclaimed under disk pressure and
could grow until the disk filled. This admits them.

• Relax the eviction-policy filter and the is_cloud_retention_active gates
  to include tiered_cloud. disk_space_manager then computes a reclaim
  offset and sets _cloud_gc_offset for them, as it already does for legacy
  topics.
• ctp_stm reads that offset through the shared compute_gc_offset and
  truncates the local log to it. do_gc (the legacy executor) skips
  tiered_cloud; consume_cloud_gc_offset clears the offset when read.
• For tiered_cloud, ctp_stm's truncate loop polls on a bounded timeout
  rather than waiting untimed, since the offset is set out of band (no
  _lro_advanced signal) and would otherwise be missed on an idle partition.
  Other modes keep the untimed wait.
• Reclamation is capped at the last-reconciled-to-L1 offset
  (max_removable_local_log_offset), so un-reconciled data is never dropped.
• Adds ducktape coverage: capacity-pressure reclaim during produce, and
  idle-partition reclaim.

Backports Required

• none - not a bug fix
• none - this is a backport
• none - issue does not exist in previous branches
• none - papercut/not impactful enough to backport
• v26.1.x
• v25.3.x
• v25.2.x

Release Notes

• none

[oleiman]

/ci-repeat 1
tests/rptest/tests/tiered_cloud_local_retention_test.py
tests/rptest/tests/log_storage_target_size_test.py

[oleiman]

/ci-repeat 1
tests/rptest/tests/tiered_cloud_local_retention_test.py
tests/rptest/tests/log_storage_target_size_test.py

[Copilot]

Pull request overview

This PR updates the space-management (disk_space_manager) eviction path to include storage.mode=tiered_cloud partitions, enabling capacity-pressure driven local log reclamation that was previously gated out for tiered_cloud topics.

Changes:

• Admit tiered_cloud partitions into the eviction policy’s reclaimable-offset collection (in addition to legacy remote_partition() topics).
• Adjust disk_log_impl cloud-GC gates and _cloud_gc_offset handling so tiered_cloud partitions can participate without do_gc consuming the pin, while compute_gc_offset folds and consumes the pin.
• Add an rptest that validates tiered_cloud local footprint converges under capacity pressure and asserts do_gc does not emit the “expected remote retention to be active” warning.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
tests/rptest/tests/tiered_cloud_local_retention_test.py	Adds an end-to-end test covering capacity-pressure reclamation for tiered_cloud local logs and pin-consumption behavior.
src/v/storage/disk_log_impl.cc	Relaxes cloud-retention gates for tiered_cloud and updates _cloud_gc_offset consumption/interaction between do_gc and compute_gc_offset.
src/v/resource_mgmt/storage.cc	Includes tiered_cloud partitions when collecting reclaimable offsets for space management decisions.

[claude]

Review: admit tiered_cloud partitions to space management

Reviewed as a spike (per the PR body). The core direction is sound: tiered_cloud partitions get is_locally_collectable() == false and is_cloud_retention_active() == false, so they were silently dropped from every eviction-policy gate. Relaxing the four gates + collect_reclaimable_offsets filter to admit them, bounded by max_removable_local_log_offset() (the reconciled L1 floor), is the right shape and matches how ctp_stm already drives trimming. The added ducktape test exercises the capacity path specifically (no local target, effectively-infinite retention), which is the field scenario, and asserts do_gc doesn't steal the pin.

I traced the paths that made me nervous and they hold up:

• do_gc returns early at is_locally_collectable() for cloud topics, so it never reaches compute_gc_offset for tiered_cloud — the pin genuinely can't be stolen there. The !config().is_tiered_cloud() guard on the early block is belt-and-suspenders but correct.
• compute_local_retention_offset (which consumes the pin) is only invoked for is_tiered_cloud(); pure storage.mode=cloud uses max_removable directly, so this PR doesn't change pure-cloud behavior. Good scoping.
• The std::max(retention, pin) fold is the right polarity — a higher offset is more aggressive eviction, so the pin only ever raises the trim point.

Concerns

1. Pin consume-on-read can drop the space-management intent (inline on compute_gc_offset). The pin is consumed when the target is computed, but the actual truncation happens later and may be capped below the pin; once consumed the next iteration falls back to retention-only until disk_space_manager re-pins. There's also a potential stall if prefix_truncate_bg blocks in _lro_advanced.wait() while a new pin is set (no signal). This is the semantics gap the PR body already calls out — flagging it so it's tracked before this leaves spike status.

2. Usage/health reporting was not relaxed in parallel. disk_usage() (lines ~3937 and ~3954) still gates available_segments / local_retention_segments on is_cloud_retention_active() without the || is_tiered_cloud() addition. The comment there notes it's "effectively identical to the condition in get_reclaimable_offsets," which this PR did relax. Net effect: _reclaimable_size_bytes (consumed by the health monitor) will report ~0 reclaimable for tiered_cloud partitions even though they are now actually reclaimable. Actual eviction still works via collect_reclaimable_offsets, but the reporting is now inconsistent with behavior — worth relaxing these two gates too for consistency.

3. Dead-code note (inline): cloud_gc_eligible_segments() has no callers, so its vassert relaxation is currently a no-op.

Nothing here blocks a spike merge; items 1 and 2 are the follow-ups I'd want resolved before this is production-ready. Test coverage looks good and correctly reuses the existing fixture helpers.

[claude]

Review — storage: admit tiered_cloud partitions to space management

Read through the C++ and the new ducktape test. The change is coherent and the design holds together; leaving notes for the eventual un-draft.

What the change does (as I read it)

• resource_mgmt/storage.cc: admits tiered_cloud partitions into collect_reclaimable_offsets. Necessary because cloud topics use ctp_stm rather than archival, so _cloud_storage_partition (i.e. remote_partition()) is nullptr for them (confirmed in partition.cc:509 — it's only constructed for archival). The added is_tiered_cloud() clause is the correct way to let them through.
• is_cloud_gc_active() = is_cloud_retention_active() || is_tiered_cloud(): cleanly captures "local segments are a reclaimable cache of cloud-resident data," and the four gate swaps (disk_usage_and_reclaimable_space, cloud_gc_eligible_segments, set_cloud_gc_offset, get_reclaimable_offsets, reclaimable_size_bytes) are consistent. Reclaim stays bounded by max_removable_local_log_offset() (the L1-reconciled offset), which is the right ceiling.
• do_gc / compute_gc_offset split: for tiered_cloud, do_gc bails at !is_locally_collectable() (true for cloud topics) before reaching compute_gc_offset, so the pin is left untouched by do_gc and consumed only by ctp_stm. The max(retention, pin) fold plus unconditional reset is a sound fix for the stale-pin-shadows-retention problem the old co_return _cloud_gc_offset had.

Things worth confirming / following up (see inline)

1. Pin consumption semantics (compute_gc_offset): the reset is a read side-effect that relies on ctp_stm being the sole consumer, and the pin is cleared even when the trim is capped below it by an un-reconciled L1. Converges via space-management re-pinning, but this is the refresh-semantics caveat you already called out — hardening the single-consumer invariant (assert / dedicated entry point) would prevent a future caller from silently breaking it.
2. Test assertion scope: search_log_any("expected remote retention to be active") is cluster-wide; consider scoping it to this ntp/node so an unrelated partition can't flap it.

Minor

• is_cloud_gc_active() is small and non-virtual — fine. No formatting/idiom concerns; comments are appropriately dense for the non-obvious control flow here.

Nothing blocking from a correctness standpoint; the main open item is the ctp_stm pin clear/refresh work you've already flagged. Since it's marked a draft/spike, this is calibrated accordingly.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

[claude]

With && !is_tiered here, every tiered_cloud partition now falls into the bounded wait(retry_backoff_time) branch unconditionally, so an idle partition wakes and re-runs compute_local_retention_offset() every 5s forever (previously it could block untimed until _lro_advanced). The snapshot_and_truncate_log call is a cheap no-op when the target hasn't advanced (it early-returns on _last_snapshot_index >= eviction_point), so this is mostly the periodic recompute cost — but on a broker hosting many tiered_cloud partitions it's a non-trivial number of steady-state wakeups.

The root cause is that disk_space_manager sets the pin out of band without signalling _lro_advanced. A cleaner long-term fix would be for the eviction-policy install path / set_cloud_gc_offset to signal the stm so it could keep the untimed wait. Not blocking — worth a follow-up note.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

[claude]

Review

Solid, well-scoped change. The core idea — introduce is_cloud_gc_active() (= is_cloud_retention_active() || is_tiered_cloud()) and split the _cloud_gc_offset pin's consumption so that legacy archival topics keep consuming it in do_gc while tiered_cloud partitions consume it in ctp_stm — is coherent, and the docstrings you added on compute_gc_offset / consume_cloud_gc_offset make the ownership contract clear. I traced the paths that most worried me and they hold up:

• No double-eviction for tiered_cloud. In do_gc, a tiered_cloud partition has is_cloud_gc_active()==true (backstop skipped) and is_cloud_retention_active()==false (legacy-consume block skipped), then falls through to !is_locally_collectable() → returns nullopt without touching the pin, because is_locally_collectable() is false for any cloud-topic mode. So only ctp_stm consumes the pin. Verified is_archival_enabled() is always false for storage_mode==tiered_cloud, so the two consumers can never race on the same partition.
• storage.mode=cloud is correctly unaffected. is_cloud_gc_active() is false for pure-cloud partitions (neither archival nor tiered_cloud), matching the prior is_cloud_retention_active() behavior in set_cloud_gc_offset/get_reclaimable_offsets; and cloud-mode's ctp_stm path uses max_removable_local_log_offset() and never reads the pin. No regression.
• consume_cloud_gc_offset() added as pure-virtual on storage::log — both implementors (disk_log_impl, failure_injectable_log) are updated, so no missing-override build breaks.
• disk_usage_and_reclaimable_space / reclaimable_size_bytes / get_reclaimable_offsets now gate on is_cloud_gc_active(), which is what admits tiered_cloud into the space-management accounting. Consistent with the eviction-policy filter change in storage.cc.

Notes (all non-blocking, left inline)

1. Idle-partition polling. && !is_tiered forces every tiered_cloud partition into the 5s bounded wait unconditionally, so idle partitions now re-run compute_local_retention_offset() every 5s instead of blocking untimed. The truncate call itself is a cheap no-op when the target hasn't advanced, so the cost is the periodic recompute — but it scales with partition count. The cleaner fix is to have the eviction-policy install path signal _lro_advanced when it sets the pin; worth a follow-up.
2. Pin dropped on truncate failure. The pin is consumed each iteration after being folded into target; if snapshot_and_truncate_log throws, the intent is lost until disk_space_manager re-pins (~retention_local_trim_interval). Self-heals given the 5s poll, so acceptable as best-effort.

Tests

The two new ducktape cases are well-constructed — test_capacity_pressure_reclaims_tiered_cloud deliberately disables the local-target path so only the capacity pin can bound the log, and test_idle_partition_reclaims_on_pin first asserts the backlog exceeds the ceiling before applying pressure so the reclaim assertion can't pass trivially. Good coverage of exactly the two behaviors this PR adds.

Nice work — no correctness concerns from my read.

[vbotbuildovich]

CI test results

test results on build#86687

test_status	test_class	test_method	test_arguments	test_kind	job_url	passed	reason	test_history
FLAKY(PASS)	ShadowLinkingReplicationTests	test_replication_with_failures	{"storage_mode": "tiered"}	integration	https://buildkite.com/redpanda/redpanda/builds/86687#019f2540-a0f6-4b34-909b-ab748b22db21	10/11	Test PASSES after retries.No significant increase in flaky rate(baseline=0.0016, p0=1.0000, reject_threshold=0.0100. adj_baseline=0.1000, p1=0.3487, trust_threshold=0.5000)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=ShadowLinkingReplicationTests&test_method=test_replication_with_failures

test results on build#86727

test_status	test_class	test_method	test_arguments	test_kind	job_url	passed	reason	test_history
FLAKY(PASS)	NodeWiseRecoveryTest	test_node_wise_recovery	{"dead_node_count": 2}	integration	https://buildkite.com/redpanda/redpanda/builds/86727#019f293c-c82d-4601-8251-1953f9919dcd	10/11	Test PASSES after retries.No significant increase in flaky rate(baseline=0.0231, p0=1.0000, reject_threshold=0.0100. adj_baseline=0.1000, p1=0.3487, trust_threshold=0.5000)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=NodeWiseRecoveryTest&test_method=test_node_wise_recovery

test results on build#86736

test_status	test_class	test_method	test_arguments	test_kind	job_url	passed	reason	test_history
FLAKY(PASS)	RedpandaNodeOperationsSmokeTest	test_node_ops_smoke_test	{"cloud_storage_type": 1, "mixed_versions": false}	integration	https://buildkite.com/redpanda/redpanda/builds/86736#019f2aa1-86e8-4d0b-a265-8e3d128f4fe7	19/21	Test PASSES after retries.No significant increase in flaky rate(baseline=0.0145, p0=0.2532, reject_threshold=0.0100. adj_baseline=0.1000, p1=0.3917, trust_threshold=0.5000)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=RedpandaNodeOperationsSmokeTest&test_method=test_node_ops_smoke_test

[oleiman]

@Lazin talked about this a little bit with Andrew earlier today. with regard to the _lro_advanced.wait change, not totally sold that this is the right tradeoff. alternatives are to do something more involved at the STM interface or a deeper refactor of housekeeping. lmk what you think.

[Lazin]

I think the clunker misunderstood the code here. This is not resetting the _cloud_gc_offset if it's set on a wrong type of log. It's the shortcut used by the space management. The _cloud_gc_offset is set by the disk space manager and then do_gc goes into this branch and requests eviction based on this offset exclusively. The code makes no sense given that the do_gc method can't be called for cloud topic partition.

[oleiman]

clunker misunderstood the code

nah that was all me 😅. would you settle for a vassert though? many paths through disk log impl, and calling do_gc for a tiered_cloud partition would certainly break prefix truncation in ctp_stm (and is a logic error, I think?).

[oleiman]

so I added the vassert and it immediately triggered in ducktape... added a blanket "skip housekeeping" check for cloud topics, but I'm not 100% sure that's the right shape. ptal

[Lazin]

the housekeeping loop is not supposed to run for both tsv2 and CT, so the blanket "skip housekeeping" sounds right

[Lazin]

The offset should be reset after it's returned.
The compute_cloud_gc method is not a simple "getter" and it potentially mutates the state of the log. This is what the doc comment says:

    /// via set_cloud_gc_offset. This is not a pure query -- it may mutate the
    /// underlying log -- which is why it is async. It does NOT request
    /// eviction itself. Shared by disk_log_impl::do_gc and cloud-topic
    /// ctp_stm housekeeping.

[Lazin]

The funny thing is that I had this in my feature branch - https://github.com/redpanda-data/redpanda/pull/30512/changes#diff-8caa0c9719de25bd209fa674959d9327cd55237de2fb4683761acf9bcc6de217R1314
and it was lost in the process (PR stacking / review). Damn.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[claude]

Review: admit tiered_cloud partitions to space management

Read through the whole change. The design is coherent and the moving parts line up well — the eviction-policy filter, the is_cloud_gc_active() broadening of the archival-only gates, and the ctp_stm truncate loop all consistently route reclaim of tiered_cloud local logs through the space manager → _cloud_gc_offset → compute_gc_offset → snapshot_and_truncate_log path, while do_gc (segment-level eviction) is cleanly kept out of it. The max_removable_local_log_offset() cap correctly prevents dropping un-reconciled data, and the tests exercise both the capacity-pressure and idle-partition angles (the idle test's explicit pre-check that the backlog exceeds the ceiling before pressure is a nice touch against trivially-passing assertions).

A couple of things I verified and found sound:

• std::exchange consume-on-read is safe against double-firing. do_gc short-circuits on _cloud_gc_offset.has_value() (line 1800) before reaching its compute_gc_offset call, so legacy tiered storage never double-consumes. Only the ctp_stm path relies on the new consume-in-compute_gc_offset behavior, and compute_local_retention_offset is the single caller.
• The timed-poll change is precisely scoped. Adding && !is_tiered only affects the "nothing to truncate right now, no active readers" case (previously an untimed _lro_advanced.wait()), which is exactly the idle-reclaim gap the offset-set-without-signalling problem creates. The "something to truncate" path already took the timed branch, so no latency regression there.

Two smaller points left as inline comments:

1. Consume-on-read + truncate failure (disk_log_impl.cc:1942): the offset is consumed at the top of the prefix_truncate_bg iteration; if snapshot_and_truncate_log throws and the loop continues, the pin is lost until the space manager re-pins. Almost certainly fine given periodic re-evaluation, but worth a confirming comment.
2. Assert could match the real invariant (disk_log_impl.cc:1786): since both gc() and housekeeping() early-return on cloud_topic_enabled(), do_gc is unreachable for all cloud topics; the !is_tiered_cloud() assert is narrower than what's actually guaranteed. Suggested strengthening to !cloud_topic_enabled().

Note the housekeeping()/gc() early-returns are keyed on cloud_topic_enabled(), so this also changes behavior for cloud (placeholder-only) mode, not just tiered_cloud. That looks intentional and correct (cloud-mode trimming is fully covered by ctp_stm's max_removable path), but flagging it since the PR title/description are framed around tiered_cloud specifically.

Reminder per CONTRIBUTING/CLAUDE.md: run tools/type-checking/type-check.sh check tests/rptest/tests/tiered_cloud_local_retention_test.py for the modified test file.

Nothing here blocks; overall LGTM.