volume: keep volume writable after a deletion-tail compaction (#9776)

makeupDiff replays post-snapshot changes onto the compacted volume. For a
replayed deletion it appended a tombstone to the new .dat but recorded the
.idx entry with offset 0. When that deletion is the last replayed change the
tombstone lands at the .dat tail, and the post-commit integrity check skips
offset-0 entries, so it sees 32 trailing bytes it can't account for and flips
the volume read-only, reloading it as a SortedFileNeedleMap instead of the
writable map.

Record the tombstone's real .dat offset, matching the normal delete path; the
needle map still treats it as deleted off the negative size, so lookups are
unchanged. Mirror the same fix into the Rust volume server.
This commit is contained in:
Chris Lu
2026-06-01 13:15:08 -07:00
committed by GitHub
parent 8c60408bfb
commit dfa86b4313
3 changed files with 61 additions and 5 deletions
+5 -1
View File
@@ -3071,11 +3071,15 @@ impl Volume {
let bytes = fake_del_needle.write_bytes(version); let bytes = fake_del_needle.write_bytes(version);
dst_dat.write_all(&bytes)?; dst_dat.write_all(&bytes)?;
// Record the tombstone's real .dat offset, like the normal delete path,
// so a deletion left at the .dat tail stays visible to the integrity
// check on reload. Offset 0 hid the trailing tombstone and falsely
// flipped the volume read-only.
let mut idx_entry_buf = [0u8; NEEDLE_MAP_ENTRY_SIZE]; let mut idx_entry_buf = [0u8; NEEDLE_MAP_ENTRY_SIZE];
crate::storage::types::idx_entry_to_bytes( crate::storage::types::idx_entry_to_bytes(
&mut idx_entry_buf, &mut idx_entry_buf,
key, key,
Offset::from_actual_offset(0), Offset::from_actual_offset(dat_offset as i64),
Size(crate::storage::types::TOMBSTONE_FILE_SIZE.into()), Size(crate::storage::types::TOMBSTONE_FILE_SIZE.into()),
); );
dst_idx.write_all(&idx_entry_buf)?; dst_idx.write_all(&idx_entry_buf)?;
+8 -4
View File
@@ -389,11 +389,15 @@ func (v *Volume) makeupDiff(newDatFileName, newIdxFileName, oldDatFileName, oldI
fakeDelNeedle.Id = key fakeDelNeedle.Id = key
fakeDelNeedle.Cookie = 0x12345678 fakeDelNeedle.Cookie = 0x12345678
fakeDelNeedle.AppendAtNs = uint64(time.Now().UnixNano()) fakeDelNeedle.AppendAtNs = uint64(time.Now().UnixNano())
_, _, _, err = fakeDelNeedle.Append(dstDatBackend, v.Version()) fakeDelOffset, _, _, appendErr := fakeDelNeedle.Append(dstDatBackend, v.Version())
if err != nil { if appendErr != nil {
return fmt.Errorf("append deleted %d failed: %v", key, err) return fmt.Errorf("append deleted %d failed: %v", key, appendErr)
} }
util.Uint32toBytes(idxEntryBytes[8:12], uint32(0)) // Record the tombstone's real .dat offset, like the normal delete path,
// so a deletion left at the .dat tail stays visible to the integrity
// check on reload. Offset 0 hid the trailing tombstone and falsely
// flipped the volume read-only.
idxEntryBytes = needle_map.ToBytes(key, ToOffset(int64(fakeDelOffset)), increIdxEntry.size)
} }
if _, err := idx.Seek(0, 2); err != nil { if _, err := idx.Seek(0, 2); err != nil {
+48
View File
@@ -151,6 +151,54 @@ func testCompactionByIndex(t *testing.T, needleMapKind NeedleMapKind) {
} }
// A deletion replayed by makeupDiff appends a tombstone to the compacted .dat.
// When that tombstone is the .dat tail, its .idx entry must carry the real
// offset, not 0, or the post-commit integrity check can't see the trailing
// tombstone and wrongly flips the volume read-only — loading a
// SortedFileNeedleMap instead of the writable LevelDb map.
func TestCommitCompactDeletionTailKeepsWritable(t *testing.T) {
dir := t.TempDir()
v, err := NewVolume(dir, dir, "", 1, NeedleMapLevelDb, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0)
if err != nil {
t.Fatalf("volume creation: %v", err)
}
for i := uint64(1); i <= 5; i++ {
if _, _, _, err := v.writeNeedle2(newRandomNeedle(i), true, false); err != nil {
t.Fatalf("write %d: %v", i, err)
}
}
v.CompactByIndex(nil)
// The sole change in the commit window is a deletion, so makeupDiff appends
// its tombstone last, putting it at the .dat tail.
if _, err := v.deleteNeedle2(newEmptyNeedle(3)); err != nil {
t.Fatalf("delete: %v", err)
}
if err := v.CommitCompact(); err != nil {
t.Fatalf("commit compact: %v", err)
}
if _, ok := v.nm.(*LevelDbNeedleMap); !ok {
t.Fatalf("after compaction v.nm is %T, want *LevelDbNeedleMap (volume wrongly marked read-only)", v.nm)
}
v.Close()
// Reload from disk to confirm the integrity check passes and the volume
// stays writable.
v, err = NewVolume(dir, dir, "", 1, NeedleMapLevelDb, nil, nil, 0, needle.GetCurrentVersion(), 0, 0)
if err != nil {
t.Fatalf("reload: %v", err)
}
defer v.Close()
if v.noWriteOrDelete {
t.Fatal("volume reloaded read-only after a deletion-tail compaction")
}
}
func TestCompactVolumeFilesOffline(t *testing.T) { func TestCompactVolumeFilesOffline(t *testing.T) {
dir := t.TempDir() dir := t.TempDir()
location := NewDiskLocation(dir, 10, util.MinFreeSpace{}, dir, "", nil) location := NewDiskLocation(dir, 10, util.MinFreeSpace{}, dir, "", nil)