Skip to content

Commit 535f4f3

Browse files
committed
storage: include tiered_cloud partitions in disk space management
tiered_cloud partition local logs were excluded from the disk space manager, so they were never reclaimed under disk pressure. A new is_cloud_gc_active() predicate (archival retention or tiered_cloud) gates the space manager's reclaim paths, so tiered_cloud partitions are now collected below the reconciled L1 offset, like legacy tiered storage. For these partitions ctp_stm is the reclaim executor: do_gc leaves the _cloud_gc_offset pin for it, and compute_gc_offset consumes the pin and folds it with retention.
1 parent 3103408 commit 535f4f3

3 files changed

Lines changed: 48 additions & 24 deletions

File tree

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: 37 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1315,6 +1315,10 @@ bool disk_log_impl::is_cloud_retention_active() const {
13151315
&& (config().is_archival_enabled());
13161316
}
13171317

1318+
bool disk_log_impl::is_cloud_gc_active() const {
1319+
return is_cloud_retention_active() || config().is_tiered_cloud();
1320+
}
1321+
13181322
/*
13191323
* applies overrides for non-cloud storage settings
13201324
*/
@@ -1779,7 +1783,11 @@ ss::future<std::optional<model::offset>> disk_log_impl::do_gc(gc_config cfg) {
17791783
* process such as disk space management drives this process such that after
17801784
* a round of gc has run the intent flag can be cleared.
17811785
*/
1782-
if (_cloud_gc_offset.has_value()) {
1786+
// Cloud-topic (tiered_cloud) partitions do not evict through do_gc /
1787+
// request_eviction_until_offset; ctp_stm consumes _cloud_gc_offset via
1788+
// compute_gc_offset instead. Leave the pin alone for them so do_gc does not
1789+
// steal and discard it before ctp_stm sees it.
1790+
if (_cloud_gc_offset.has_value() && !config().is_tiered_cloud()) {
17831791
const auto offset = _cloud_gc_offset.value();
17841792
_cloud_gc_offset.reset();
17851793

@@ -1912,20 +1920,29 @@ disk_log_impl::maybe_adjusted_retention_offset(gc_config cfg) {
19121920

19131921
ss::future<std::optional<model::offset>>
19141922
disk_log_impl::compute_gc_offset(gc_config cfg) {
1915-
// Single GC-offset computation shared by gc() housekeeping and by
1916-
// ctp_stm for cloud-topic partitions. The offset is retention-driven
1917-
// unless space management has pinned _cloud_gc_offset, which then takes
1918-
// precedence. maybe_apply_local_storage_overrides
1919-
// bypasses the is_cloud_retention_active() gate for cloud-topic partitions
1920-
// (the gate is false for storage.mode in {cloud, tiered_cloud}), so the
1921-
// local-target override engages there under retention_local_strict.
1923+
// Single GC-offset computation shared by gc() housekeeping and by ctp_stm
1924+
// for cloud-topic partitions. Retention (time/bytes plus the local-storage
1925+
// overrides, which maybe_apply_local_storage_overrides engages for
1926+
// cloud-topic partitions under retention_local_strict even though the
1927+
// is_cloud_retention_active() gate is false) sets the base offset. Space
1928+
// management can raise it under disk pressure via a pinned
1929+
// _cloud_gc_offset.
1930+
//
1931+
// ctp_stm is the sole consumer of the pin for cloud topics (do_gc leaves it
1932+
// alone), so consume it here instead of letting it persist: a stale pin
1933+
// would otherwise keep shadowing retention after disk pressure relents.
1934+
// Fold it with retention so the pin only ever raises the trim point.
19221935
cfg = apply_kafka_retention_overrides(cfg);
1923-
if (_cloud_gc_offset.has_value()) {
1924-
co_return _cloud_gc_offset;
1925-
}
19261936
co_await maybe_adjust_retention_timestamps();
19271937
cfg = maybe_apply_local_storage_overrides(cfg);
1928-
co_return retention_offset(cfg);
1938+
auto retention = retention_offset(cfg);
1939+
1940+
auto pin = _cloud_gc_offset;
1941+
_cloud_gc_offset.reset();
1942+
if (pin.has_value()) {
1943+
co_return retention.has_value() ? std::max(*retention, *pin) : pin;
1944+
}
1945+
co_return retention;
19291946
}
19301947

19311948
std::optional<model::offset>
@@ -3921,7 +3938,7 @@ disk_log_impl::disk_usage_and_reclaimable_space(gc_config input_cfg) {
39213938
&& seg->offsets().get_dirty_offset() <= retention_offset.value()) {
39223939
retention_segments.push_back(seg);
39233940
} else if (
3924-
is_cloud_retention_active()
3941+
is_cloud_gc_active()
39253942
&& seg->offsets().get_dirty_offset() <= max_removable) {
39263943
available_segments.push_back(seg);
39273944
} else {
@@ -3937,8 +3954,8 @@ disk_log_impl::disk_usage_and_reclaimable_space(gc_config input_cfg) {
39373954
* get_reclaimable_offsets is going to be merged together.
39383955
*/
39393956
if (
3940-
!config().is_read_replica_mode_enabled()
3941-
&& is_cloud_retention_active() && seg != _segs.back()
3957+
!config().is_read_replica_mode_enabled() && is_cloud_gc_active()
3958+
&& seg != _segs.back()
39423959
&& seg->offsets().get_dirty_offset() <= max_removable
39433960
&& local_retention_offset.has_value()
39443961
&& seg->offsets().get_dirty_offset()
@@ -4279,8 +4296,8 @@ ss::future<usage_report> disk_log_impl::disk_usage(gc_config cfg) {
42794296
chunked_vector<ss::lw_shared_ptr<segment>>
42804297
disk_log_impl::cloud_gc_eligible_segments() {
42814298
vassert(
4282-
is_cloud_retention_active(),
4283-
"Expected {} to have cloud retention enabled",
4299+
is_cloud_gc_active(),
4300+
"Expected cloud GC to be active for {}",
42844301
config().ntp());
42854302

42864303
constexpr size_t keep_segs = 1;
@@ -4313,7 +4330,7 @@ disk_log_impl::cloud_gc_eligible_segments() {
43134330
}
43144331

43154332
void disk_log_impl::set_cloud_gc_offset(model::offset offset) {
4316-
if (!is_cloud_retention_active()) {
4333+
if (!is_cloud_gc_active()) {
43174334
vlog(
43184335
stlog.debug,
43194336
"Ignoring request to set GC offset on non-cloud enabled partition "
@@ -4338,7 +4355,7 @@ disk_log_impl::get_reclaimable_offsets(gc_config cfg) {
43384355

43394356
reclaimable_offsets res;
43404357

4341-
if (!is_cloud_retention_active()) {
4358+
if (!is_cloud_gc_active()) {
43424359
vlog(
43434360
stlog.debug,
43444361
"Reporting no reclaimable offsets for non-cloud partition {}",
@@ -4540,7 +4557,7 @@ size_t disk_log_impl::reclaimable_size_bytes() const {
45404557
* local retention size may change. catch these before reporting potentially
45414558
* stale information.
45424559
*/
4543-
if (!is_cloud_retention_active()) {
4560+
if (!is_cloud_gc_active()) {
45444561
return 0;
45454562
}
45464563
if (config().is_read_replica_mode_enabled()) {

src/v/storage/disk_log_impl.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,10 @@ 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 retention and local-storage overrides;
214+
/// when space management has pinned _cloud_gc_offset it is folded in (max)
215+
/// so the pin only raises the trim point. The pin is consumed (reset) here;
216+
/// for tiered_cloud, ctp_stm is its sole consumer (do_gc leaves it alone).
216217
ss::future<std::optional<model::offset>>
217218
compute_gc_offset(gc_config cfg) final;
218219

@@ -422,6 +423,10 @@ class disk_log_impl final : public log {
422423
gc_config apply_local_storage_overrides(gc_config) const;
423424

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

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

0 commit comments

Comments
 (0)