ec.decode: check the rebuilt .dat is complete before the shards can be deleted (#10768)

A decode ends by deleting the shards it read, and the only thing standing
between that and a bad reconstruction is verifyDecodedVolumeBeforeDelete,
which asks whether .dat and .idx are non-empty. A .dat truncated to a
single byte passes, and the shards -- the only other copy of everything
past the cut -- are deleted on the strength of it.

The server already knows the answer it never checks: FindDatFileSize
returns the extent the EC index references, and WriteDatFile rebuilds to
it. Compare the two once the file is written and fail the decode instead
of reporting a short volume as a good one.

Longer than the extent still verifies -- padding is not missing data --
so only a genuinely short rebuild is rejected.

Needle counts cannot answer this: .idx is written from .ecx, so the count
matches by construction and a truncated .dat still reports every needle.
This commit is contained in:
Chris Lu
2026-08-15 13:28:49 -07:00
committed by GitHub
parent 829064af71
commit fbd85d31b0
3 changed files with 115 additions and 0 deletions
@@ -1007,6 +1007,13 @@ func (vs *VolumeServer) VolumeEcShardsToVolume(ctx context.Context, req *volume_
return nil, fmt.Errorf("WriteDatFile %s: %v", dataBaseFileName, err)
}
// Fail the decode rather than report a short volume: the caller deletes the
// shards once this call returns, and today its only check is that the files
// are non-empty.
if err := erasure_coding.VerifyDecodedDatFile(dataBaseFileName, datFileSize); err != nil {
return nil, err
}
// write .idx file from .ecx and .ecj files
if err := erasure_coding.WriteIdxFileFromEcIndex(indexBaseFileName); err != nil {
return nil, fmt.Errorf("WriteIdxFileFromEcIndex %s: %v", v.IndexBaseFileName(), err)
+18
View File
@@ -126,6 +126,24 @@ func FindDatFileSize(shard0FileName, indexBaseFileName string) (datSize int64, e
return
}
// VerifyDecodedDatFile checks that a reconstructed .dat is long enough to hold
// every needle its index references. datFileSize is the extent the EC index
// describes (see FindDatFileSize), so a shorter file cannot serve the needles
// past the cut -- and the caller is about to delete the shards that are their
// only other copy, which turns a short write into data loss rather than a
// failed decode.
func VerifyDecodedDatFile(dataBaseFileName string, datFileSize int64) error {
datPath := dataBaseFileName + ".dat"
stat, err := os.Stat(datPath)
if err != nil {
return fmt.Errorf("stat decoded %s: %w", datPath, err)
}
if stat.Size() < datFileSize {
return fmt.Errorf("decoded %s is %d bytes, short of the %d its ec index references", datPath, stat.Size(), datFileSize)
}
return nil
}
func readEcVolumeVersion(shard0FileName string) (version needle.Version, err error) {
// find volume version
@@ -0,0 +1,90 @@
package erasure_coding
import (
"os"
"path/filepath"
"strings"
"testing"
)
// A decode hands the caller a volume and the caller then deletes the shards it
// came from. VerifyDecodedDatFile is what stands between a reconstruction that
// came up short -- a truncated shard, a partial write, a full disk -- and the
// deletion of the only other copy of the needles past the cut.
func TestVerifyDecodedDatFile(t *testing.T) {
tests := []struct {
name string
written int64
referenced int64
wantErr bool
errContains string
}{
{
name: "exactly the referenced extent is complete",
written: 4096,
referenced: 4096,
},
{
name: "longer than referenced is fine: padding is not missing data",
written: 8192,
referenced: 4096,
},
{
name: "one byte short still loses the last needle",
written: 4095,
referenced: 4096,
wantErr: true,
errContains: "short of the 4096",
},
{
name: "an empty file is short of everything",
written: 0,
referenced: 4096,
wantErr: true,
errContains: "is 0 bytes",
},
{
name: "an index referencing nothing accepts an empty file",
written: 0,
referenced: 0,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
base := filepath.Join(t.TempDir(), "5")
if err := os.WriteFile(base+".dat", make([]byte, tt.written), 0644); err != nil {
t.Fatal(err)
}
err := VerifyDecodedDatFile(base, tt.referenced)
if tt.wantErr {
if err == nil {
t.Fatalf("wrote %d bytes against a %d-byte extent, want an error", tt.written, tt.referenced)
}
if !strings.Contains(err.Error(), tt.errContains) {
t.Errorf("error %q does not mention %q", err, tt.errContains)
}
return
}
if err != nil {
t.Fatalf("wrote %d bytes against a %d-byte extent: %v", tt.written, tt.referenced, err)
}
})
}
}
// A missing .dat is not a short one, and the two want different answers: the
// caller cannot tell whether the decode wrote nothing or wrote elsewhere, so
// the error has to name the file rather than a byte count.
func TestVerifyDecodedDatFileMissing(t *testing.T) {
base := filepath.Join(t.TempDir(), "7")
err := VerifyDecodedDatFile(base, 4096)
if err == nil {
t.Fatal("a missing .dat must not verify")
}
if !strings.Contains(err.Error(), "stat decoded") {
t.Errorf("error %q does not say the file could not be read", err)
}
}