diff --git a/weed/storage/erasure_coding/ec_encoder.go b/weed/storage/erasure_coding/ec_encoder.go index cd4d95fd8..e5b687a89 100644 --- a/weed/storage/erasure_coding/ec_encoder.go +++ b/weed/storage/erasure_coding/ec_encoder.go @@ -13,7 +13,6 @@ import ( "github.com/seaweedfs/seaweedfs/weed/storage/needle_map" "github.com/seaweedfs/seaweedfs/weed/storage/types" "github.com/seaweedfs/seaweedfs/weed/storage/volume_info" - "github.com/seaweedfs/seaweedfs/weed/util" ) const ( @@ -128,19 +127,31 @@ func generateEcFiles(baseFileName string, bufferSize int, largeBlockSize int64, } // findShardFile looks for a shard file at baseFileName+ext, then in additionalDirs. -func findShardFile(baseFileName string, ext string, additionalDirs []string) string { - primary := baseFileName + ext - if util.FileExists(primary) { - return primary - } +// Returns the first non-empty shard file path found. Any 0-byte residue files +// (typically left behind by a previously aborted rebuild) are returned in ghosts +// so the caller can remove them — otherwise they shadow real shards on the next +// rebuild attempt and the volume gets silently mounted with empty stripes. +func findShardFile(baseFileName string, ext string, additionalDirs []string) (shardPath string, ghosts []string) { + candidates := make([]string, 0, 1+len(additionalDirs)) + candidates = append(candidates, baseFileName+ext) baseName := filepath.Base(baseFileName) for _, dir := range additionalDirs { - candidate := filepath.Join(dir, baseName+ext) - if util.FileExists(candidate) { - return candidate + candidates = append(candidates, filepath.Join(dir, baseName+ext)) + } + for _, c := range candidates { + fi, statErr := os.Stat(c) + if statErr != nil { + continue + } + if fi.Size() == 0 { + ghosts = append(ghosts, c) + continue + } + if shardPath == "" { + shardPath = c } } - return "" + return } func generateMissingEcFiles(baseFileName string, bufferSize int, largeBlockSize int64, smallBlockSize int64, ctx *ECContext, additionalDirs []string) (generatedShardIds []uint32, err error) { @@ -150,22 +161,35 @@ func generateMissingEcFiles(baseFileName string, bufferSize int, largeBlockSize shardHasData := make([]bool, ctx.Total()) shardPaths := make([]string, ctx.Total()) // non-empty for present shards inputFiles := make([]*os.File, ctx.Total()) + var ghostPaths []string + expectedShardSize := int64(-1) presentCount := 0 for shardId := 0; shardId < ctx.Total(); shardId++ { ext := ctx.ToExt(shardId) - shardPath := findShardFile(baseFileName, ext, additionalDirs) - if shardPath != "" { - shardHasData[shardId] = true - shardPaths[shardId] = shardPath - inputFiles[shardId], err = os.OpenFile(shardPath, os.O_RDONLY, 0) - if err != nil { - return nil, err - } - defer inputFiles[shardId].Close() - presentCount++ - } else { + shardPath, ghosts := findShardFile(baseFileName, ext, additionalDirs) + ghostPaths = append(ghostPaths, ghosts...) + if shardPath == "" { generatedShardIds = append(generatedShardIds, uint32(shardId)) + continue } + fi, statErr := os.Stat(shardPath) + if statErr != nil { + return nil, fmt.Errorf("stat shard %s: %w", shardPath, statErr) + } + if expectedShardSize < 0 { + expectedShardSize = fi.Size() + } else if fi.Size() != expectedShardSize { + return nil, fmt.Errorf("ec shard size mismatch: %s is %d bytes, expected %d (refusing to rebuild from inconsistent shards)", + shardPath, fi.Size(), expectedShardSize) + } + shardHasData[shardId] = true + shardPaths[shardId] = shardPath + inputFiles[shardId], err = os.OpenFile(shardPath, os.O_RDONLY, 0) + if err != nil { + return nil, err + } + defer inputFiles[shardId].Close() + presentCount++ } // Pre-check: bail out before creating any output files. @@ -174,8 +198,8 @@ func generateMissingEcFiles(baseFileName string, bufferSize int, largeBlockSize baseFileName, presentCount, ctx.DataShards, generatedShardIds) } - glog.V(0).Infof("rebuilding %s: %d shards present, %d missing %v, config %s", - baseFileName, presentCount, len(generatedShardIds), generatedShardIds, ctx.String()) + glog.V(0).Infof("rebuilding %s: %d shards present (size %d), %d missing %v, %d ghost residue %v, config %s", + baseFileName, presentCount, expectedShardSize, len(generatedShardIds), generatedShardIds, len(ghostPaths), ghostPaths, ctx.String()) // Pass 2: create output files for missing shards now that we know // reconstruction is possible. @@ -192,10 +216,32 @@ func generateMissingEcFiles(baseFileName string, bufferSize int, largeBlockSize defer outputFiles[shardId].Close() } - err = rebuildEcFiles(shardHasData, inputFiles, outputFiles, ctx) + err = rebuildEcFiles(shardHasData, inputFiles, outputFiles, ctx, expectedShardSize) if err != nil { return nil, fmt.Errorf("rebuildEcFiles: %w", err) } + + // Reconstruction succeeded — remove any 0-byte residue from prior aborted + // rebuilds so they don't shadow real shards on the next pass and don't get + // mounted as empty stripes. Skip residue paths that the output-file step + // has already overwritten with real data, since those entries in + // ghostPaths refer to the just-rebuilt shard's location. + freshOutputs := make(map[string]struct{}, len(outputFiles)) + for _, f := range outputFiles { + if f != nil { + freshOutputs[f.Name()] = struct{}{} + } + } + for _, ghost := range ghostPaths { + if _, overwritten := freshOutputs[ghost]; overwritten { + continue + } + if removeErr := os.Remove(ghost); removeErr != nil && !os.IsNotExist(removeErr) { + glog.Warningf("failed to remove 0-byte ec shard residue %s: %v", ghost, removeErr) + } else { + glog.V(0).Infof("removed 0-byte ec shard residue %s", ghost) + } + } return } @@ -320,7 +366,7 @@ func encodeDatFile(remainingSize int64, baseFileName string, bufferSize int, lar return nil } -func rebuildEcFiles(shardHasData []bool, inputFiles []*os.File, outputFiles []*os.File, ctx *ECContext) error { +func rebuildEcFiles(shardHasData []bool, inputFiles []*os.File, outputFiles []*os.File, ctx *ECContext, expectedShardSize int64) error { enc, err := ctx.CreateEncoder() if err != nil { @@ -334,23 +380,22 @@ func rebuildEcFiles(shardHasData []bool, inputFiles []*os.File, outputFiles []*o } } - var startOffset int64 - var inputBufferDataSize int - for { + for startOffset := int64(0); startOffset < expectedShardSize; { + chunkSize := int64(ErasureCodingSmallBlockSize) + if remaining := expectedShardSize - startOffset; remaining < chunkSize { + chunkSize = remaining + } // read the input data from files for i := 0; i < ctx.Total(); i++ { if shardHasData[i] { - n, _ := inputFiles[i].ReadAt(buffers[i], startOffset) - if n == 0 { - return nil - } - if inputBufferDataSize == 0 { - inputBufferDataSize = n - } - if inputBufferDataSize != n { - return fmt.Errorf("ec shard size expected %d actual %d", inputBufferDataSize, n) + buf := buffers[i][:chunkSize] + n, readErr := inputFiles[i].ReadAt(buf, startOffset) + if int64(n) != chunkSize { + return fmt.Errorf("short read on shard %s at offset %d: got %d, want %d (err=%v)", + inputFiles[i].Name(), startOffset, n, chunkSize, readErr) } + buffers[i] = buf } else { buffers[i] = nil } @@ -365,15 +410,16 @@ func rebuildEcFiles(shardHasData []bool, inputFiles []*os.File, outputFiles []*o // write the data to output files for i := 0; i < ctx.Total(); i++ { if !shardHasData[i] { - n, _ := outputFiles[i].WriteAt(buffers[i][:inputBufferDataSize], startOffset) - if inputBufferDataSize != n { - return fmt.Errorf("fail to write to %s", outputFiles[i].Name()) + n, writeErr := outputFiles[i].WriteAt(buffers[i][:chunkSize], startOffset) + if int64(n) != chunkSize { + return fmt.Errorf("short write to %s at offset %d: got %d, want %d (err=%v)", + outputFiles[i].Name(), startOffset, n, chunkSize, writeErr) } } } - startOffset += int64(inputBufferDataSize) + startOffset += chunkSize } - + return nil } func readNeedleMap(baseFileName string) (*needle_map.MemDb, error) { diff --git a/weed/storage/erasure_coding/ec_rebuild_test.go b/weed/storage/erasure_coding/ec_rebuild_test.go new file mode 100644 index 000000000..36b60bf4f --- /dev/null +++ b/weed/storage/erasure_coding/ec_rebuild_test.go @@ -0,0 +1,249 @@ +package erasure_coding + +import ( + "bytes" + "crypto/rand" + "os" + "path/filepath" + "sort" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb/volume_server_pb" + "github.com/seaweedfs/seaweedfs/weed/storage/volume_info" +) + +// TestRebuildEcFiles_BasicTwoMissing covers the simple repair path: 12 of 14 +// shards present, 2 truly missing — must rebuild both and produce shards +// identical to the originals. +func TestRebuildEcFiles_BasicTwoMissing(t *testing.T) { + dir := t.TempDir() + base := filepath.Join(dir, "_109") + originalShards := encodeFixture(t, base, 50*1024*1024) + + // Drop shards 1 and 8. + mustRemove(t, base+ToExt(1)) + mustRemove(t, base+ToExt(8)) + + rebuilt, err := RebuildEcFiles(base) + if err != nil { + t.Fatalf("RebuildEcFiles: %v", err) + } + sort.Slice(rebuilt, func(i, j int) bool { return rebuilt[i] < rebuilt[j] }) + if want := []uint32{1, 8}; !equalUint32(rebuilt, want) { + t.Fatalf("rebuilt = %v, want %v", rebuilt, want) + } + + for _, idx := range []int{1, 8} { + got := mustReadFile(t, base+ToExt(idx)) + if !bytes.Equal(got, originalShards[idx]) { + t.Fatalf("shard %d content does not match the original encoding", idx) + } + } +} + +// TestRebuildEcFiles_RejectsZeroByteResidue is the regression for issue #9340. +// +// When a previous ec.rebuild attempt aborts (e.g. crash, network error, +// reedsolomon "too few shards" mid-rebuild), it leaves 0-byte placeholder +// files for the shards it was about to write. On the next attempt, the old +// findShardFile saw those 0-byte ghosts as "present", short-circuited the +// reconstruction loop on the first n==0 read, and returned nil — making +// ec.rebuild claim success while leaving the volume with empty shards. +// +// The fix: skip 0-byte files when scanning, treat the shards as still-missing, +// and remove the residue after a successful rebuild so they don't shadow real +// shards on the next pass. +func TestRebuildEcFiles_RejectsZeroByteResidue(t *testing.T) { + dir := t.TempDir() + base := filepath.Join(dir, "_109") + originalShards := encodeFixture(t, base, 50*1024*1024) + + // Simulate the residue from a prior aborted rebuild: shard files for the + // truly-missing shards exist on disk but are 0 bytes. + for _, idx := range []int{1, 8} { + mustRemove(t, base+ToExt(idx)) + mustWriteFile(t, base+ToExt(idx), nil) + } + + rebuilt, err := RebuildEcFiles(base) + if err != nil { + t.Fatalf("RebuildEcFiles: %v", err) + } + sort.Slice(rebuilt, func(i, j int) bool { return rebuilt[i] < rebuilt[j] }) + if want := []uint32{1, 8}; !equalUint32(rebuilt, want) { + t.Fatalf("rebuilt = %v, want %v (the old code reported [] and silently left 0-byte residue)", rebuilt, want) + } + + for _, idx := range []int{1, 8} { + fi, err := os.Stat(base + ToExt(idx)) + if err != nil { + t.Fatalf("stat shard %d: %v", idx, err) + } + if fi.Size() == 0 { + t.Fatalf("shard %d still 0 bytes after rebuild — residue was not cleared", idx) + } + got := mustReadFile(t, base+ToExt(idx)) + if !bytes.Equal(got, originalShards[idx]) { + t.Fatalf("rebuilt shard %d does not match the original encoding", idx) + } + } +} + +// TestRebuildEcFiles_ResidueOnAdditionalDisk reproduces the multi-disk variant +// of the residue bug. The .ecx and the real shards live on the rebuild disk; +// a sibling disk holds 0-byte placeholders for the shards that need to be +// regenerated (e.g. an earlier rebuild crashed while writing them there). The +// pre-fix findShardFile preferred the first existing path it saw — including +// the 0-byte one — so the rebuild silently succeeded with empty stripes. +func TestRebuildEcFiles_ResidueOnAdditionalDisk(t *testing.T) { + root := t.TempDir() + disk1 := filepath.Join(root, "d1") + disk2 := filepath.Join(root, "d2") + mustMkdir(t, disk1) + mustMkdir(t, disk2) + + stagingBase := filepath.Join(root, "_109") + originalShards := encodeFixture(t, stagingBase, 50*1024*1024) + + // Real shards (except 1 and 8) and .vif live on disk1. + disk1Base := filepath.Join(disk1, "_109") + for i := 0; i < TotalShardsCount; i++ { + if i == 1 || i == 8 { + continue + } + mustRename(t, stagingBase+ToExt(i), disk1Base+ToExt(i)) + } + mustRename(t, stagingBase+".vif", disk1Base+".vif") + + // disk2 carries 0-byte residue for the missing shards. + disk2Base := filepath.Join(disk2, "_109") + for _, idx := range []int{1, 8} { + mustWriteFile(t, disk2Base+ToExt(idx), nil) + } + + rebuilt, err := RebuildEcFiles(disk1Base, disk2) + if err != nil { + t.Fatalf("RebuildEcFiles: %v", err) + } + sort.Slice(rebuilt, func(i, j int) bool { return rebuilt[i] < rebuilt[j] }) + if want := []uint32{1, 8}; !equalUint32(rebuilt, want) { + t.Fatalf("rebuilt = %v, want %v", rebuilt, want) + } + + for _, idx := range []int{1, 8} { + got := mustReadFile(t, disk1Base+ToExt(idx)) + if !bytes.Equal(got, originalShards[idx]) { + t.Fatalf("rebuilt shard %d on disk1 does not match the original", idx) + } + // Residue on the sibling disk must be cleaned up so it can't shadow + // the real shard on a future scan. + if _, err := os.Stat(disk2Base + ToExt(idx)); !os.IsNotExist(err) { + t.Fatalf("shard %d residue on disk2 was not removed (err=%v)", idx, err) + } + } +} + +// TestRebuildEcFiles_RejectsMismatchedShardSizes ensures we surface a clear +// error when surviving shards disagree on size, instead of feeding inconsistent +// data into reedsolomon and producing corrupted output. +func TestRebuildEcFiles_RejectsMismatchedShardSizes(t *testing.T) { + dir := t.TempDir() + base := filepath.Join(dir, "_109") + encodeFixture(t, base, 50*1024*1024) + + mustRemove(t, base+ToExt(1)) + // Truncate one surviving shard so its size disagrees with the others. + if err := os.Truncate(base+ToExt(2), 1024); err != nil { + t.Fatalf("truncate: %v", err) + } + + if _, err := RebuildEcFiles(base); err == nil { + t.Fatalf("expected size-mismatch error, got nil") + } +} + +// encodeFixture writes a .dat file of the requested size, EC-encodes it, saves +// a matching .vif, and returns a snapshot of every original shard's bytes. +func encodeFixture(t *testing.T, baseFileName string, datSize int64) [TotalShardsCount][]byte { + t.Helper() + + data := make([]byte, datSize) + if _, err := rand.Read(data); err != nil { + t.Fatalf("rand: %v", err) + } + if err := os.WriteFile(baseFileName+".dat", data, 0644); err != nil { + t.Fatalf("write dat: %v", err) + } + if err := WriteEcFiles(baseFileName); err != nil { + t.Fatalf("WriteEcFiles: %v", err) + } + + // A minimal .vif so RebuildEcFiles picks up the right ec config rather + // than warning and falling back to defaults. + vif := &volume_server_pb.VolumeInfo{ + Version: 3, + DatFileSize: datSize, + EcShardConfig: &volume_server_pb.EcShardConfig{ + DataShards: DataShardsCount, + ParityShards: ParityShardsCount, + }, + } + if err := volume_info.SaveVolumeInfo(baseFileName+".vif", vif); err != nil { + t.Fatalf("save vif: %v", err) + } + + var shards [TotalShardsCount][]byte + for i := 0; i < TotalShardsCount; i++ { + shards[i] = mustReadFile(t, baseFileName+ToExt(i)) + } + return shards +} + +func mustRemove(t *testing.T, path string) { + t.Helper() + if err := os.Remove(path); err != nil { + t.Fatalf("remove %s: %v", path, err) + } +} + +func mustWriteFile(t *testing.T, path string, data []byte) { + t.Helper() + if err := os.WriteFile(path, data, 0644); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} + +func mustReadFile(t *testing.T, path string) []byte { + t.Helper() + data, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read %s: %v", path, err) + } + return data +} + +func mustRename(t *testing.T, from, to string) { + t.Helper() + if err := os.Rename(from, to); err != nil { + t.Fatalf("rename %s -> %s: %v", from, to, err) + } +} + +func mustMkdir(t *testing.T, dir string) { + t.Helper() + if err := os.MkdirAll(dir, 0755); err != nil { + t.Fatalf("mkdir %s: %v", dir, err) + } +} + +func equalUint32(a, b []uint32) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +}