Skip to content

Commit 0560690

Browse files
committed
cs/manifest: Fix scrubber false positive for replaced segments
The scrub false-positive filter in process_anomalies() only checked whether a segment with the same offset range existed in the manifest. A compacted reupload produces a replacement segment at the same offset range but with a different name (different size). When GC deleted the old segment from cloud storage while the scrubber was still referencing a stale manifest, the filter kept the anomaly because the offset range still matched—even though the current segment at that range was a different (replacement) object that existed in cloud storage. Compare generate_remote_segment_name() for the manifest entry and the reported-missing segment so that replacements with the same offset range but different identity are correctly recognized as false positives. Fixes CORE-6913. Signed-off-by: Oren Leiman <oren.leiman@redpanda.com>
1 parent cb543a7 commit 0560690

1 file changed

Lines changed: 31 additions & 9 deletions

File tree

src/v/cloud_storage/partition_manifest.cc

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2769,11 +2769,25 @@ void partition_manifest::process_anomalies(
27692769

27702770
if (meta.committed_offset >= get_start_offset()) {
27712771
// The segment might have been missing because it was merged with
2772-
// something else. If the offset range doesn't match a segment
2773-
// exactly, discard the anomaly. Only segments from the STM
2774-
// manifest may be merged/reuploaded.
2775-
return !segment_with_offset_range_exists(
2776-
meta.base_offset, meta.committed_offset);
2772+
// something else or replaced by a compacted reupload. Only
2773+
// segments from the STM manifest may be merged/reuploaded.
2774+
auto iter = find(meta.base_offset);
2775+
if (
2776+
iter == end()
2777+
|| iter->committed_offset != meta.committed_offset) {
2778+
// Offset range no longer matches any segment in the
2779+
// manifest (merged or reuploaded with different
2780+
// boundaries). Discard the anomaly.
2781+
return true;
2782+
}
2783+
// The offset range still exists. Verify the current segment
2784+
// is the same one reported as missing, not a replacement
2785+
// (e.g., a compacted reupload with the same offset range but
2786+
// different size/term). If a different segment now covers
2787+
// the range, the old one being absent from cloud storage is
2788+
// expected.
2789+
return generate_remote_segment_name(*iter)
2790+
!= generate_remote_segment_name(meta);
27772791
} else {
27782792
// Segment belongs to the archive. No reuploads are done here.
27792793
return false;
@@ -2790,10 +2804,18 @@ void partition_manifest::process_anomalies(
27902804
}
27912805

27922806
if (anomaly_meta.at.committed_offset >= get_start_offset()) {
2793-
// Similarly to the missing segment case, if the boundaries of the
2794-
// segment where the anomaly was detected changed, drop it.
2795-
return !segment_with_offset_range_exists(
2796-
anomaly_meta.at.base_offset, anomaly_meta.at.committed_offset);
2807+
// Similarly to the missing segment case, if the boundaries
2808+
// of the segment where the anomaly was detected changed or
2809+
// the segment was replaced (e.g., compacted reupload),
2810+
// drop the anomaly.
2811+
auto iter = find(anomaly_meta.at.base_offset);
2812+
if (
2813+
iter == end()
2814+
|| iter->committed_offset != anomaly_meta.at.committed_offset) {
2815+
return true;
2816+
}
2817+
return generate_remote_segment_name(*iter)
2818+
!= generate_remote_segment_name(anomaly_meta.at);
27972819
} else {
27982820
return false;
27992821
}

0 commit comments

Comments
 (0)