mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-19 13:46:58 +00:00
fix(scrub): don't flag offset-0 logical tombstones in volume scrub (#10148)
* fix(scrub): don't flag offset-0 logical tombstones in volume scrub A remote-tier delete records a tombstone at .idx offset 0 with no physical .dat bytes. Full scrub double-flagged a healthy remote-tiered volume with deletes: scrubVolumeData counted the tombstone's GetActualSize(-1)=32 toward totalRead (want > physical .dat), and CheckIndexFile treated it as occupying [0,31] and flagged the first live needle as overlapping. Skip offset-0 logical tombstones from both the size reconcile and the overlap check; they are still counted for the index-size check. Local deletes (offset != 0) are unaffected. Claude-Session: https://claude.ai/code/session_015EE9Sc9EvNp8BCVva4RKdo * fix(scrub): mirror offset-0 logical tombstone handling into Rust Same fix as the Go volume_checking.go + idx/check.go change: Volume::scrub skips offset-0 logical tombstones from total_read, and check_index_file excludes them from the overlap check (still counted for the index-size check). Claude-Session: https://claude.ai/code/session_015EE9Sc9EvNp8BCVva4RKdo
This commit is contained in:
@@ -59,9 +59,18 @@ pub fn check_index_file<R: Read + Seek>(
|
||||
|
||||
entries.sort_by(|a, b| a.2.cmp(&b.2).then(a.3 .0.cmp(&b.3 .0)));
|
||||
|
||||
for j in 1..entries.len() {
|
||||
let (index, id, offset, size) = entries[j];
|
||||
let (_, last_id, last_offset, last_size) = entries[j - 1];
|
||||
// Offset-0 logical tombstones (remote-tier deletes) occupy no physical extent,
|
||||
// so they cannot overlap anything — exclude them from the overlap check. They
|
||||
// are still counted below for the index-size check.
|
||||
let physical: Vec<(usize, NeedleId, i64, Size)> = entries
|
||||
.iter()
|
||||
.copied()
|
||||
.filter(|e| !(e.2 == 0 && e.3.is_deleted()))
|
||||
.collect();
|
||||
|
||||
for j in 1..physical.len() {
|
||||
let (index, id, offset, size) = physical[j];
|
||||
let (_, last_id, last_offset, last_size) = physical[j - 1];
|
||||
|
||||
let end = match get_actual_size(size, version) {
|
||||
0 => offset,
|
||||
@@ -193,6 +202,20 @@ mod tests {
|
||||
assert!(errs[0].contains("overlaps"), "{:?}", errs);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_check_index_file_ignores_offset0_tombstone() {
|
||||
// A live needle near the start plus an offset-0 logical tombstone (remote-tier
|
||||
// delete) must NOT be flagged as overlapping; the tombstone row still counts.
|
||||
let data = idx_bytes(&[
|
||||
(NeedleId(1), Offset::from_actual_offset(8), Size(100)),
|
||||
(NeedleId(2), Offset::from_actual_offset(0), Size(-1)),
|
||||
]);
|
||||
let size = data.len() as i64;
|
||||
let (count, errs) = check_index_file(&mut Cursor::new(data), size, Version(3));
|
||||
assert_eq!(count, 2, "tombstone row is still counted: {:?}", errs);
|
||||
assert!(errs.is_empty(), "offset-0 tombstone must not overlap: {:?}", errs);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_check_index_file_detects_size_mismatch() {
|
||||
let data = idx_bytes(&[(NeedleId(1), Offset::from_actual_offset(0), Size(50))]);
|
||||
|
||||
@@ -2041,6 +2041,11 @@ impl Volume {
|
||||
let mut total_read: i64 = 0;
|
||||
let walk = crate::storage::idx::walk_index_file(&mut idx_file, 0, |needle_id, offset, size| {
|
||||
count += 1;
|
||||
// A remote-tier delete records an offset-0 tombstone with no physical
|
||||
// .dat bytes, so it must not contribute to total_read.
|
||||
if offset.is_zero() && size.is_deleted() {
|
||||
return Ok(());
|
||||
}
|
||||
// Deleted needles still occupy .dat space: count their size, don't read.
|
||||
total_read += get_actual_size(size, version);
|
||||
if size.is_deleted() {
|
||||
@@ -3937,6 +3942,45 @@ mod tests {
|
||||
assert_eq!(count, 3, "scrub must count every .idx row");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_scrub_ignores_offset0_tombstone() {
|
||||
// A remote-tier delete appends an offset-0 tombstone to the .idx with no
|
||||
// physical .dat bytes; it must not flag the volume (neither the size
|
||||
// reconcile nor the structural overlap check).
|
||||
let tmp = TempDir::new().unwrap();
|
||||
let dir = tmp.path().to_str().unwrap();
|
||||
let mut v = make_test_volume(dir);
|
||||
|
||||
let data = b"needle data".to_vec();
|
||||
let mut n = Needle {
|
||||
id: NeedleId(1),
|
||||
cookie: Cookie(1),
|
||||
data: data.clone(),
|
||||
data_size: data.len() as u32,
|
||||
..Needle::default()
|
||||
};
|
||||
v.write_needle(&mut n, true).unwrap();
|
||||
v.sync_to_disk().unwrap();
|
||||
|
||||
// Append an offset-0 logical tombstone to the on-disk .idx.
|
||||
let mut f = std::fs::OpenOptions::new()
|
||||
.append(true)
|
||||
.open(v.file_name(".idx"))
|
||||
.unwrap();
|
||||
crate::storage::idx::write_index_entry(&mut f, NeedleId(2), Offset::from_actual_offset(0), Size(-1))
|
||||
.unwrap();
|
||||
f.sync_all().unwrap();
|
||||
drop(f);
|
||||
|
||||
let (count, broken) = v.scrub().unwrap();
|
||||
assert!(
|
||||
broken.is_empty(),
|
||||
"offset-0 tombstone must not flag the volume, got {:?}",
|
||||
broken
|
||||
);
|
||||
assert_eq!(count, 2, "both .idx rows are walked");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_scrub_index_flags_zero_size_idx_with_data() {
|
||||
// A populated .dat with an empty .idx is corruption — the pre-allocated
|
||||
|
||||
@@ -56,7 +56,18 @@ func CheckIndexFile(r io.ReaderAt, indexFileSize int64, version needle.Version)
|
||||
return entries[i].Compare(entries[j]) < 0
|
||||
})
|
||||
|
||||
for i, e := range entries {
|
||||
// Offset-0 logical tombstones (remote-tier deletes) occupy no physical .dat
|
||||
// extent, so they cannot overlap anything — exclude them from the overlap
|
||||
// check. They are still counted below for the index-size check.
|
||||
physical := make([]*indexEntry, 0, len(entries))
|
||||
for _, e := range entries {
|
||||
if e.offset == 0 && e.size.IsDeleted() {
|
||||
continue
|
||||
}
|
||||
physical = append(physical, e)
|
||||
}
|
||||
|
||||
for i, e := range physical {
|
||||
if i == 0 {
|
||||
// nothing to check for the first entry
|
||||
continue
|
||||
@@ -67,7 +78,7 @@ func CheckIndexFile(r io.ReaderAt, indexFileSize int64, version needle.Version)
|
||||
end += size - 1
|
||||
}
|
||||
|
||||
last := entries[i-1]
|
||||
last := physical[i-1]
|
||||
lastStart, lastEnd := last.offset, last.offset
|
||||
if lastSize := needle.GetActualSize(last.size, version); lastSize != 0 {
|
||||
lastEnd += lastSize - 1
|
||||
|
||||
@@ -1,14 +1,41 @@
|
||||
package idx
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"fmt"
|
||||
"os"
|
||||
"reflect"
|
||||
"testing"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/storage/needle"
|
||||
"github.com/seaweedfs/seaweedfs/weed/storage/types"
|
||||
)
|
||||
|
||||
func idxEntryBytes(id types.NeedleId, offset types.Offset, size types.Size) []byte {
|
||||
b := make([]byte, types.NeedleIdSize+types.OffsetSize+types.SizeSize)
|
||||
types.NeedleIdToBytes(b[0:types.NeedleIdSize], id)
|
||||
types.OffsetToBytes(b[types.NeedleIdSize:types.NeedleIdSize+types.OffsetSize], offset)
|
||||
types.SizeToBytes(b[types.NeedleIdSize+types.OffsetSize:], size)
|
||||
return b
|
||||
}
|
||||
|
||||
// An offset-0 logical tombstone (remote-tier delete) occupies no physical .dat
|
||||
// extent, so it must not be flagged as overlapping a real needle. The tombstone
|
||||
// row is still counted for the index-size check.
|
||||
func TestCheckIndexFile_IgnoresOffset0Tombstone(t *testing.T) {
|
||||
var buf []byte
|
||||
buf = append(buf, idxEntryBytes(types.NeedleId(1), types.ToOffset(8), types.Size(100))...)
|
||||
buf = append(buf, idxEntryBytes(types.NeedleId(2), types.ToOffset(0), types.TombstoneFileSize)...)
|
||||
|
||||
count, errs := CheckIndexFile(bytes.NewReader(buf), int64(len(buf)), needle.Version3)
|
||||
if count != 2 {
|
||||
t.Errorf("tombstone row must still count: got %d", count)
|
||||
}
|
||||
if len(errs) != 0 {
|
||||
t.Errorf("offset-0 tombstone must not overlap: %v", errs)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCheckIndexFile(t *testing.T) {
|
||||
testCases := []struct {
|
||||
name string
|
||||
|
||||
@@ -79,6 +79,11 @@ func (v *Volume) scrubVolumeData(idxFile *os.File, idxFileSize int64) (int64, []
|
||||
version := v.Version()
|
||||
err := idx.WalkIndexFile(idxFile, 0, func(id types.NeedleId, offset types.Offset, size types.Size) error {
|
||||
count++
|
||||
// A remote-tier delete records an offset-0 tombstone with no physical .dat
|
||||
// bytes, so it must not contribute to totalRead.
|
||||
if offset.IsZero() && size.IsDeleted() {
|
||||
return nil
|
||||
}
|
||||
// compute the actual size of the needle in disk, including needle header, body and alignment padding.
|
||||
actualSize := int64(needle.GetActualSize(size, version))
|
||||
|
||||
|
||||
@@ -130,6 +130,55 @@ func TestScrubVolumeData(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestScrubVolumeData_IgnoresOffset0Tombstone: a remote-tier delete appends an
|
||||
// offset-0 tombstone to the .idx with no physical .dat bytes; a full scrub must
|
||||
// not flag the volume — neither via the structural overlap check nor the size
|
||||
// reconcile.
|
||||
func TestScrubVolumeData_IgnoresOffset0Tombstone(t *testing.T) {
|
||||
dir := t.TempDir()
|
||||
v, err := NewVolume(dir, dir, "", 1, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0)
|
||||
if err != nil {
|
||||
t.Fatalf("volume creation: %v", err)
|
||||
}
|
||||
defer v.Close()
|
||||
|
||||
if _, _, _, err := v.writeNeedle2(newRandomNeedle(1), true, false); err != nil {
|
||||
t.Fatalf("write needle: %v", err)
|
||||
}
|
||||
if err := v.DataBackend.Sync(); err != nil {
|
||||
t.Fatalf("sync .dat: %v", err)
|
||||
}
|
||||
if err := v.nm.Sync(); err != nil {
|
||||
t.Fatalf("sync .idx: %v", err)
|
||||
}
|
||||
|
||||
// Append an offset-0 logical tombstone, as a remote-tier delete would.
|
||||
entry := needle_map.NeedleValue{Key: types.NeedleId(99), Offset: types.ToOffset(0), Size: types.TombstoneFileSize}.ToBytes()
|
||||
f, err := os.OpenFile(v.FileName(".idx"), os.O_WRONLY|os.O_APPEND, 0644)
|
||||
if err != nil {
|
||||
t.Fatalf("open .idx: %v", err)
|
||||
}
|
||||
if _, err := f.Write(entry); err != nil {
|
||||
f.Close()
|
||||
t.Fatalf("append tombstone: %v", err)
|
||||
}
|
||||
f.Close()
|
||||
|
||||
idxFile, err := os.OpenFile(v.FileName(".idx"), os.O_RDONLY, 0644)
|
||||
if err != nil {
|
||||
t.Fatalf("reopen .idx: %v", err)
|
||||
}
|
||||
defer idxFile.Close()
|
||||
idxStat, err := idxFile.Stat()
|
||||
if err != nil {
|
||||
t.Fatalf("stat .idx: %v", err)
|
||||
}
|
||||
|
||||
if _, errs := v.scrubVolumeData(idxFile, idxStat.Size()); len(errs) != 0 {
|
||||
t.Fatalf("offset-0 tombstone must not flag the volume, got %v", errs)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCheckVolumeDataIntegrityWithDeletionTombstone guards the size argument
|
||||
// passed to ReadData when verifying a trailing deletion tombstone. The .idx
|
||||
// entry carries TombstoneFileSize (-1) but the appended tombstone needle in
|
||||
|
||||
Reference in New Issue
Block a user