diff --git a/weed/server/volume_grpc_erasure_coding.go b/weed/server/volume_grpc_erasure_coding.go index 45cd2e6fe..f4a1d15ca 100644 --- a/weed/server/volume_grpc_erasure_coding.go +++ b/weed/server/volume_grpc_erasure_coding.go @@ -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) diff --git a/weed/storage/erasure_coding/ec_decoder.go b/weed/storage/erasure_coding/ec_decoder.go index 5b08a232f..5bf4342d3 100644 --- a/weed/storage/erasure_coding/ec_decoder.go +++ b/weed/storage/erasure_coding/ec_decoder.go @@ -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 diff --git a/weed/storage/erasure_coding/ec_decoder_completeness_test.go b/weed/storage/erasure_coding/ec_decoder_completeness_test.go new file mode 100644 index 000000000..50ec85359 --- /dev/null +++ b/weed/storage/erasure_coding/ec_decoder_completeness_test.go @@ -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) + } +}