diff --git a/seaweed-volume/src/storage/idx/mod.rs b/seaweed-volume/src/storage/idx/mod.rs index 42045d706..15392cb39 100644 --- a/seaweed-volume/src/storage/idx/mod.rs +++ b/seaweed-volume/src/storage/idx/mod.rs @@ -59,9 +59,18 @@ pub fn check_index_file( 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))]); diff --git a/seaweed-volume/src/storage/volume.rs b/seaweed-volume/src/storage/volume.rs index f0b2048d6..77b0ccc0f 100644 --- a/seaweed-volume/src/storage/volume.rs +++ b/seaweed-volume/src/storage/volume.rs @@ -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 diff --git a/weed/storage/idx/check.go b/weed/storage/idx/check.go index c0ff3267d..abc54801f 100644 --- a/weed/storage/idx/check.go +++ b/weed/storage/idx/check.go @@ -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 diff --git a/weed/storage/idx/check_test.go b/weed/storage/idx/check_test.go index c3f8a3b9b..d6af36e96 100644 --- a/weed/storage/idx/check_test.go +++ b/weed/storage/idx/check_test.go @@ -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 diff --git a/weed/storage/volume_checking.go b/weed/storage/volume_checking.go index 7460f8755..c758152fb 100644 --- a/weed/storage/volume_checking.go +++ b/weed/storage/volume_checking.go @@ -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)) diff --git a/weed/storage/volume_checking_test.go b/weed/storage/volume_checking_test.go index a554c226c..c553b1621 100644 --- a/weed/storage/volume_checking_test.go +++ b/weed/storage/volume_checking_test.go @@ -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