Skip to content

Commit 52b9849

Browse files
archival: clamp local-log trim while the manifest holds archived data
A cloud-mode partition whose storage mode has been flipped reports is_archival_enabled() == false, which would make max_removable_local_log_offset collect_all and stop constraining local-log truncation -- evicting tiered-storage data not yet uploaded. Keep constraining via cloud_recoverable_offset() while the partition still holds archived data (a non-empty live manifest or a spillover archive), introduced here as holds_archived_data(); once the manifest is emptied, trimming resumes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 29f156c commit 52b9849

3 files changed

Lines changed: 57 additions & 0 deletions

File tree

src/v/cluster/archival/archival_metadata_stm.cc

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1373,6 +1373,18 @@ model::offset archival_metadata_stm::max_removable_local_log_offset() {
13731373
collect_all = false;
13741374
}
13751375

1376+
// The manifest, not the storage mode, governs local-log truncation. While
1377+
// the partition still holds archived data (a non-empty live manifest or a
1378+
// spillover archive) it is still served from tiered storage and may hold
1379+
// local data not yet uploaded, so its local log must not be trimmed past
1380+
// cloud_recoverable_offset(). is_archival_enabled() can report false while
1381+
// archived data is still present -- e.g. a partition mid tiered->cloud
1382+
// migration whose effective storage mode is cloud -- which would otherwise
1383+
// collect_all and stop constraining truncation, evicting the not-yet-
1384+
// uploaded data. Gate collect_all on holds_archived_data() so it takes
1385+
// effect only once the manifest is cleared.
1386+
collect_all = collect_all && !holds_archived_data();
1387+
13761388
if (collect_all || is_read_replica || (uploads_paused && gaps_allowed)) {
13771389
// The archival is disabled but the state machine still exists so we
13781390
// shouldn't stop eviction from happening.
@@ -1690,6 +1702,11 @@ model::offset archival_metadata_stm::get_archive_start_offset() const {
16901702
return _manifest->get_archive_start_offset();
16911703
}
16921704

1705+
bool archival_metadata_stm::holds_archived_data() const {
1706+
return _manifest->size() > 0
1707+
|| _manifest->get_archive_start_offset() != model::offset{};
1708+
}
1709+
16931710
model::offset archival_metadata_stm::get_archive_clean_offset() const {
16941711
return _manifest->get_archive_clean_offset();
16951712
}

src/v/cluster/archival/archival_metadata_stm.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,10 @@ class archival_metadata_stm final : public raft::persisted_stm<> {
252252
model::offset get_last_offset() const;
253253
model::offset get_archive_start_offset() const;
254254
model::offset get_archive_clean_offset() const;
255+
256+
/// True if the partition still has data in tiered storage -- either in the
257+
/// live STM manifest or offloaded to the spillover archive.
258+
bool holds_archived_data() const;
255259
kafka::offset get_start_kafka_offset() const;
256260

257261
// Return list of all segments that has to be

src/v/cluster/archival/tests/archival_metadata_stm_test.cc

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,42 @@ FIXTURE_TEST(
256256
archival_stm->manifest().begin()->committed_offset, model::offset(99));
257257
}
258258

259+
// With archival disabled (the flipped, mid-migration storage mode) but a
260+
// non-empty manifest, max_removable_local_log_offset keeps constraining
261+
// local-log truncation rather than collecting all -- protecting tiered-storage
262+
// data that has not yet been uploaded.
263+
FIXTURE_TEST(test_migration_local_trim_clamp, archival_metadata_stm_fixture) {
264+
wait_for_confirmed_leader();
265+
266+
// The fixture's partition has archival disabled. With an empty manifest,
267+
// nothing constrains local-log truncation.
268+
BOOST_REQUIRE_EQUAL(
269+
archival_stm->max_removable_local_log_offset(), model::offset::max());
270+
271+
// Add a segment so the manifest is non-empty.
272+
std::vector<cloud_storage::segment_meta> m;
273+
m.push_back(
274+
segment_meta{
275+
.base_offset = model::offset(0),
276+
.committed_offset = model::offset(99),
277+
.archiver_term = model::term_id(1),
278+
.segment_term = model::term_id(1)});
279+
archival_stm
280+
->add_segments(
281+
m,
282+
std::nullopt,
283+
model::producer_id{},
284+
ss::lowres_clock::now() + 10s,
285+
never_abort,
286+
cluster::segment_validated::yes)
287+
.get();
288+
BOOST_REQUIRE_EQUAL(archival_stm->manifest().size(), 1);
289+
290+
// Now truncation is constrained (no longer offset::max()).
291+
BOOST_REQUIRE_NE(
292+
archival_stm->max_removable_local_log_offset(), model::offset::max());
293+
}
294+
259295
FIXTURE_TEST(test_archival_stm_segment_replace, archival_metadata_stm_fixture) {
260296
wait_for_confirmed_leader();
261297
std::vector<cloud_storage::segment_meta> m1;

0 commit comments

Comments
 (0)