Skip to content

Commit 03bdc56

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. Also updates disk_log_impl::compute_gc_offset to reset the space management GC hint on every invocation where it is present. Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
1 parent 3103408 commit 03bdc56

3 files changed

Lines changed: 23 additions & 13 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: 13 additions & 9 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
*/
@@ -1921,7 +1925,7 @@ disk_log_impl::compute_gc_offset(gc_config cfg) {
19211925
// local-target override engages there under retention_local_strict.
19221926
cfg = apply_kafka_retention_overrides(cfg);
19231927
if (_cloud_gc_offset.has_value()) {
1924-
co_return _cloud_gc_offset;
1928+
co_return std::exchange(_cloud_gc_offset, std::nullopt);
19251929
}
19261930
co_await maybe_adjust_retention_timestamps();
19271931
cfg = maybe_apply_local_storage_overrides(cfg);
@@ -3921,7 +3925,7 @@ disk_log_impl::disk_usage_and_reclaimable_space(gc_config input_cfg) {
39213925
&& seg->offsets().get_dirty_offset() <= retention_offset.value()) {
39223926
retention_segments.push_back(seg);
39233927
} else if (
3924-
is_cloud_retention_active()
3928+
is_cloud_gc_active()
39253929
&& seg->offsets().get_dirty_offset() <= max_removable) {
39263930
available_segments.push_back(seg);
39273931
} else {
@@ -3937,8 +3941,8 @@ disk_log_impl::disk_usage_and_reclaimable_space(gc_config input_cfg) {
39373941
* get_reclaimable_offsets is going to be merged together.
39383942
*/
39393943
if (
3940-
!config().is_read_replica_mode_enabled()
3941-
&& is_cloud_retention_active() && seg != _segs.back()
3944+
!config().is_read_replica_mode_enabled() && is_cloud_gc_active()
3945+
&& seg != _segs.back()
39423946
&& seg->offsets().get_dirty_offset() <= max_removable
39433947
&& local_retention_offset.has_value()
39443948
&& seg->offsets().get_dirty_offset()
@@ -4279,8 +4283,8 @@ ss::future<usage_report> disk_log_impl::disk_usage(gc_config cfg) {
42794283
chunked_vector<ss::lw_shared_ptr<segment>>
42804284
disk_log_impl::cloud_gc_eligible_segments() {
42814285
vassert(
4282-
is_cloud_retention_active(),
4283-
"Expected {} to have cloud retention enabled",
4286+
is_cloud_gc_active(),
4287+
"Expected cloud GC to be active for {}",
42844288
config().ntp());
42854289

42864290
constexpr size_t keep_segs = 1;
@@ -4313,7 +4317,7 @@ disk_log_impl::cloud_gc_eligible_segments() {
43134317
}
43144318

43154319
void disk_log_impl::set_cloud_gc_offset(model::offset offset) {
4316-
if (!is_cloud_retention_active()) {
4320+
if (!is_cloud_gc_active()) {
43174321
vlog(
43184322
stlog.debug,
43194323
"Ignoring request to set GC offset on non-cloud enabled partition "
@@ -4338,7 +4342,7 @@ disk_log_impl::get_reclaimable_offsets(gc_config cfg) {
43384342

43394343
reclaimable_offsets res;
43404344

4341-
if (!is_cloud_retention_active()) {
4345+
if (!is_cloud_gc_active()) {
43424346
vlog(
43434347
stlog.debug,
43444348
"Reporting no reclaimable offsets for non-cloud partition {}",
@@ -4540,7 +4544,7 @@ size_t disk_log_impl::reclaimable_size_bytes() const {
45404544
* local retention size may change. catch these before reporting potentially
45414545
* stale information.
45424546
*/
4543-
if (!is_cloud_retention_active()) {
4547+
if (!is_cloud_gc_active()) {
45444548
return 0;
45454549
}
45464550
if (config().is_read_replica_mode_enabled()) {

src/v/storage/disk_log_impl.h

Lines changed: 7 additions & 3 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

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

424424
bool is_cloud_retention_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_cloud_retention_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.

0 commit comments

Comments
 (0)