From d10f7e381f1496e4b485517bb329de046c2f68b2 Mon Sep 17 00:00:00 2001 From: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Tue, 20 Oct 2026 18:00:27 -0700 Subject: [PATCH] volume: repair a .dat tail left past an empty .idx The writable index loader creates the .idx before the first needle append, so a crash after that append leaves a volume whose data nothing indexes. CheckVolumeDataIntegrity skipped an empty index outright, so the volume loaded writable and the surviving first needle stayed invisible while later writes appended past it. A zero-size index now runs the same repair with tailEntryPos -1: the walk starts at the first record past the superblock with no anchor needle and no append timestamp to compare against, gets the same complete-record validation and incomplete-tail policy, and re-verifies through the normal tail check once it wrote entries. A tail the repair cannot explain fails the check, so the volume loads read-only instead of writable on an unaccounted-for .dat. Review feedback on the Rust mirror (#11878): the same bug exists here; fixing both sides identically. Generated with [Devin](https://devin.ai)\n\nCo-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- weed/storage/volume_checking.go | 89 +++++++++---- .../volume_checking_tail_repair_test.go | 126 ++++++++++++++++++ 2 files changed, 188 insertions(+), 27 deletions(-) diff --git a/weed/storage/volume_checking.go b/weed/storage/volume_checking.go index 7b5c028342..7ee0e478b2 100644 --- a/weed/storage/volume_checking.go +++ b/weed/storage/volume_checking.go @@ -145,7 +145,25 @@ func CheckVolumeDataIntegrity(v *Volume, indexFile *os.File) (lastAppendAtNs uin return 0, fmt.Errorf("verifyIndexFileIntegrity %s failed: %v", indexFile.Name(), err) } if indexSize == 0 { - return 0, nil + if v.noWriteOrDelete || v.noWriteCanDelete { + return 0, nil + } + // An empty index can hide writes a crash stranded in the .dat: the + // writable index loader creates the .idx before the first needle + // append, so a crash after that append leaves a volume whose data + // nothing indexes — and every check below skips an empty index. The + // same repair runs from the first record past the superblock; a tail + // it cannot explain fails the check, since an unaccounted-for .dat + // must not load writable either. + if _, _, err = repairUnindexedDatTail(v, indexFile, 0, -1); err != nil { + return 0, fmt.Errorf("CheckVolumeDataIntegrity %s: repair unindexed .dat tail: %v", indexFile.Name(), err) + } + if indexSize, err = verifyIndexFileIntegrity(indexFile); err != nil { + return 0, fmt.Errorf("verifyIndexFileIntegrity %s after re-indexing failed: %v", indexFile.Name(), err) + } + if indexSize == 0 { + return 0, nil + } } // The deeper-than-tail structural check (every (offset + actual size) // fits inside .dat) lives in volume.load(): it reads @@ -258,7 +276,11 @@ var errTailNotTorn = errors.New("not a torn tail") // A write appends the needle to the .dat, then its entry to the .idx, then // answers the client. A process killed between the first two steps leaves a // needle, or part of one, past the end of the needle at the highest indexed -// offset. After a power loss the order on disk is whatever the page cache +// offset; a crash after the very first append leaves it past an index with +// no entry at all, which tailEntryPos < 0 marks: the walk then starts at the +// first record past the superblock with no anchor needle and no append +// timestamp to compare against. After a power loss the order on disk is +// whatever the page cache // flushed: a complete needle can survive whose .idx entry did not, although // the client was answered. So a complete needle is never dropped: every // complete needle in the tail (its header, body, checksum, timestamp and @@ -298,18 +320,45 @@ func repairUnindexedDatTail(v *Volume, indexFile *os.File, indexSize int64, tail // does not name its needle's position on its own return 0, 0, nil } - entry, err := readIndexEntryAtOffset(indexFile, tailEntryPos) - if err != nil { - return 0, 0, fmt.Errorf("read index entry at %d: %v", tailEntryPos, err) + var start int64 + var lastAppendAtNs uint64 + if tailEntryPos < 0 { + // The index holds no row at all, so no entry can anchor the walk: + // it starts at the first record past the superblock. + start = int64(v.SuperBlock.BlockSize()) + } else { + entry, err := readIndexEntryAtOffset(indexFile, tailEntryPos) + if err != nil { + return 0, 0, fmt.Errorf("read index entry at %d: %v", tailEntryPos, err) + } + key, offset, size := idx.IdxFileEntry(entry) + if offset.IsZero() { + return 0, 0, nil + } + start = needleDiskEnd(offset, size, version) + if start >= datSize { + // the .dat ends at the last needle (clean), or the index references + // bytes the .dat does not have: not a torn tail, not ours to fix + return 0, 0, nil + } + + // Anchor: the needle at the highest indexed offset must be the one its + // entry describes, whole and with a valid CRC. + onDiskSize := size + if size.IsDeleted() { + onDiskSize = 0 + } + anchor, err := readTailNeedle(v.DataBackend, version, offset.ToActualOffset(), onDiskSize) + if err != nil { + return 0, 0, fmt.Errorf("last indexed needle %v at %d: %v: %w", key, offset.ToActualOffset(), err, errTailNotTorn) + } + if anchor.Id != key { + return 0, 0, fmt.Errorf("needle at %d is id %v, its index entry says id %v: %w", offset.ToActualOffset(), anchor.Id, key, errTailNotTorn) + } + lastAppendAtNs = anchor.AppendAtNs } - key, offset, size := idx.IdxFileEntry(entry) - if offset.IsZero() { - return 0, 0, nil - } - start := needleDiskEnd(offset, size, version) if start >= datSize { - // the .dat ends at the last needle (clean), or the index references - // bytes the .dat does not have: not a torn tail, not ours to fix + // no record past the superblock: nothing to index return 0, 0, nil } // The backend rounds its size up to the needle alignment; the walk needs @@ -319,21 +368,7 @@ func repairUnindexedDatTail(v *Volume, indexFile *os.File, indexSize int64, tail fileSize = st.Size() } - // Anchor: the needle at the highest indexed offset must be the one its - // entry describes, whole and with a valid CRC. - onDiskSize := size - if size.IsDeleted() { - onDiskSize = 0 - } - anchor, err := readTailNeedle(v.DataBackend, version, offset.ToActualOffset(), onDiskSize) - if err != nil { - return 0, 0, fmt.Errorf("last indexed needle %v at %d: %v: %w", key, offset.ToActualOffset(), err, errTailNotTorn) - } - if anchor.Id != key { - return 0, 0, fmt.Errorf("needle at %d is id %v, its index entry says id %v: %w", offset.ToActualOffset(), anchor.Id, key, errTailNotTorn) - } - - entries, cutAt, err := walkUnindexedDatTail(v.DataBackend, version, start, fileSize, anchor.AppendAtNs) + entries, cutAt, err := walkUnindexedDatTail(v.DataBackend, version, start, fileSize, lastAppendAtNs) if err != nil { return 0, 0, err } diff --git a/weed/storage/volume_checking_tail_repair_test.go b/weed/storage/volume_checking_tail_repair_test.go index 996f3b8573..cbf3e9a40f 100644 --- a/weed/storage/volume_checking_tail_repair_test.go +++ b/weed/storage/volume_checking_tail_repair_test.go @@ -302,6 +302,132 @@ func TestVolumeLoadRepairsUnindexedDatTail(t *testing.T) { } } +// An empty index can hide writes a crash stranded in the .dat: the writable +// index loader creates the .idx before the first needle append, so a crash +// after that append leaves a volume whose data nothing indexes. The same +// repair runs from the first record past the superblock. +func TestVolumeLoadRepairsTailPastEmptyIndex(t *testing.T) { + cases := []struct { + name string + ops []tornOp + damage func(t *testing.T, f *tornFixture) + wantSize func(f *tornFixture) int64 // .dat size after the repair + reindexed int + readable []uint64 + empty []uint64 + absent []uint64 + }{ + {name: "the only write is re-indexed", + ops: writes(7), + wantSize: func(f *tornFixture) int64 { return f.end }, reindexed: 1, + readable: []uint64{7}}, + {name: "every write is re-indexed", + ops: writes(1, 2, 3), + wantSize: func(f *tornFixture) int64 { return f.end }, reindexed: 3, + readable: []uint64{1, 2, 3}}, + {name: "an unindexed delete is re-indexed as an empty live needle", + ops: append(writes(1, 2, 3), tornOp{id: 2, del: true}), + wantSize: func(f *tornFixture) int64 { return f.end }, reindexed: 4, + readable: []uint64{1, 3}, empty: []uint64{2}}, + {name: "a complete needle is re-indexed and the partial one after it cut", + ops: writes(1, 2), + damage: func(t *testing.T, f *tornFixture) { f.truncateDat(t, f.midRecord(1)) }, + wantSize: func(f *tornFixture) int64 { return f.starts[1] }, reindexed: 1, + readable: []uint64{1}, absent: []uint64{2}}, + {name: "a last needle with a bad CRC is cut", + ops: writes(1, 2), + damage: func(t *testing.T, f *tornFixture) { f.flipDataByte(t, f.starts[1]) }, + wantSize: func(f *tornFixture) int64 { return f.starts[1] }, reindexed: 1, + readable: []uint64{1}, absent: []uint64{2}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f := buildTornFixture(t, tc.ops, len(tc.ops)) + if tc.damage != nil { + tc.damage(t, f) + } + + v := newTornTailTestVolume(t, f.dir) + defer v.Close() + if v.noWriteOrDelete || v.noWriteCanDelete { + t.Fatal("volume with an unindexed .dat tail loaded read-only") + } + if got, want := fileSize(t, f.datName), tc.wantSize(f); got != want { + t.Fatalf(".dat is %d bytes after load, want %d", got, want) + } + if got, want := fileSize(t, f.idxName), int64(tc.reindexed)*types.NeedleMapEntrySize; got != want { + t.Fatalf(".idx is %d bytes after load, want %d (%d entries re-indexed)", got, want, tc.reindexed) + } + for _, id := range tc.readable { + assertReadable(t, v, f.written[id]) + } + for _, id := range tc.empty { + assertEmpty(t, v, id) + } + for _, id := range tc.absent { + assertNotReadable(t, v, id) + } + v.Close() + + // the repaired volume reloads clean, and keeps what it indexed + v2 := newTornTailTestVolume(t, f.dir) + defer v2.Close() + if v2.noWriteOrDelete { + t.Fatal("repaired volume loaded read-only on the next load") + } + for _, id := range tc.readable { + assertReadable(t, v2, f.written[id]) + } + for _, id := range tc.empty { + assertEmpty(t, v2, id) + } + }) + } +} + +// An .dat an interrupted append does not explain, over an index with no rows +// at all, loads the volume read-only: the checks that would quarantine it +// otherwise all skip an empty index. +func TestVolumeLoadKeepsUnexplainedTailPastEmptyIndexReadOnly(t *testing.T) { + cases := []struct { + name string + damage func(t *testing.T, f *tornFixture) + }{ + {name: "zero bytes where the first record would be (id 0)", + damage: func(t *testing.T, f *tornFixture) { + file, err := os.OpenFile(f.datName, os.O_RDWR|os.O_APPEND, 0644) + if err != nil { + t.Fatal(err) + } + defer file.Close() + if _, err := file.Write(make([]byte, 64)); err != nil { + t.Fatal(err) + } + }}, + {name: "a damaged record more data follows", + damage: func(t *testing.T, f *tornFixture) { f.flipDataByte(t, f.starts[0]) }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + f := buildTornFixture(t, writes(1, 2), 2) + tc.damage(t, f) + datBefore, idxBefore := fileSize(t, f.datName), fileSize(t, f.idxName) + + v := newTornTailTestVolume(t, f.dir) + defer v.Close() + if !v.noWriteOrDelete { + t.Fatal("volume with an unexplained tail past an empty index loaded writable") + } + if got := fileSize(t, f.datName); got != datBefore { + t.Fatalf(".dat changed from %d to %d bytes", datBefore, got) + } + if got := fileSize(t, f.idxName); got != idxBefore { + t.Fatalf(".idx changed from %d to %d bytes", idxBefore, got) + } + }) + } +} + // Every tail an interrupted append does not explain is left exactly as it is, // and the volume loads read-only as before. func TestVolumeLoadKeepsUnexplainedTailReadOnly(t *testing.T) {