From bea1357d38535692c68f66d56b523f87f75b936e Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 30 Jun 2026 20:01:55 -0700 Subject: [PATCH] ec: skip physically near-full disks when placing EC shards (#10167) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit EC placement scored destinations purely by free EC shard slots (derived from maxVolumeCount) and shard counts, blind to real disk fullness — the same defect as volume balancing. A disk that is physically full but still shows free EC slots kept being chosen, and EC shard bytes are captured by statfs free space yet not by any slot accounting, so the slot math is exactly the metric that can't see EC fullness. Treat a disk at/above 90% physical usage as having zero free EC slots at snapshot-build time, so every existing freeSlots>0 placement predicate excludes it. Applied in all three snapshot builders (shell countFreeShardSlots, the shared ecbalancer FromActiveTopology, and the worker ec_balance buildBalancerTopology) via the shared balancer.DiskTooFullAfter gate. Servers not reporting disk bytes fall back to slot-only behavior. ec.rebuild recovery is left ungated so shard recovery can still complete onto fuller disks. --- weed/shell/command_ec_common.go | 8 ++++ weed/shell/command_ec_common_test.go | 37 ++++++++++++++ .../erasure_coding/ecbalancer/snapshot.go | 13 ++++- .../ecbalancer/snapshot_test.go | 48 +++++++++++++++++++ weed/worker/tasks/ec_balance/detection.go | 30 ++++++++++-- 5 files changed, 130 insertions(+), 6 deletions(-) create mode 100644 weed/shell/command_ec_common_test.go diff --git a/weed/shell/command_ec_common.go b/weed/shell/command_ec_common.go index ef12e0d18..fefaba3cb 100644 --- a/weed/shell/command_ec_common.go +++ b/weed/shell/command_ec_common.go @@ -19,6 +19,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/storage/needle" "github.com/seaweedfs/seaweedfs/weed/storage/super_block" "github.com/seaweedfs/seaweedfs/weed/storage/types" + "github.com/seaweedfs/seaweedfs/weed/topology/balancer" "google.golang.org/grpc" ) @@ -469,6 +470,13 @@ func countFreeShardSlots(dn *master_pb.DataNodeInfo, diskType types.DiskType) (c return 0 } + // A physically near-full disk has no room for more EC shards regardless of + // slot math (an over-set maxVolumeCount hides real fullness; statfs free bytes + // already include EC shard files). No-opinion when the server reports no bytes. + if balancer.DiskTooFullAfter(diskInfo.DiskTotalBytes, diskInfo.DiskFreeBytes, 0, balancer.DefaultMaxDiskUsagePercent) { + return 0 + } + slots := int(diskInfo.MaxVolumeCount-diskInfo.VolumeCount)*erasure_coding.DataShardsCount - countShards(diskInfo.EcShardInfos) if slots < 0 { return 0 diff --git a/weed/shell/command_ec_common_test.go b/weed/shell/command_ec_common_test.go new file mode 100644 index 000000000..2ec0600e5 --- /dev/null +++ b/weed/shell/command_ec_common_test.go @@ -0,0 +1,37 @@ +package shell + +import ( + "testing" + + "github.com/seaweedfs/seaweedfs/weed/pb/master_pb" + "github.com/seaweedfs/seaweedfs/weed/storage/types" +) + +// countFreeShardSlots must report zero for a physically near-full disk even +// though its slot math (MaxVolumeCount - VolumeCount) says it has room, so EC +// placement never piles shards onto a full disk. Mirrors the volume #10160 gate. +func TestCountFreeShardSlotsPhysicalDiskGate(t *testing.T) { + const gb = uint64(1) << 30 + + mk := func(total, free uint64) *master_pb.DataNodeInfo { + return &master_pb.DataNodeInfo{ + Id: "n1", + DiskInfos: map[string]*master_pb.DiskInfo{ + "": {MaxVolumeCount: 100, VolumeCount: 0, DiskTotalBytes: total, DiskFreeBytes: free}, + }, + } + } + + // Physically empty: slot math applies, positive free slots. + if got := countFreeShardSlots(mk(1000*gb, 900*gb), types.HardDriveType); got <= 0 { + t.Errorf("physically empty disk free shard slots = %d, want > 0", got) + } + // Physically 96% full: gated to zero regardless of slot room. + if got := countFreeShardSlots(mk(1000*gb, 40*gb), types.HardDriveType); got != 0 { + t.Errorf("physically full disk free shard slots = %d, want 0", got) + } + // No byte report (older server): fall back to slot math, positive. + if got := countFreeShardSlots(mk(0, 0), types.HardDriveType); got <= 0 { + t.Errorf("unreported-bytes disk free shard slots = %d, want > 0 (slot fallback)", got) + } +} diff --git a/weed/storage/erasure_coding/ecbalancer/snapshot.go b/weed/storage/erasure_coding/ecbalancer/snapshot.go index a8380caac..b0641cd29 100644 --- a/weed/storage/erasure_coding/ecbalancer/snapshot.go +++ b/weed/storage/erasure_coding/ecbalancer/snapshot.go @@ -4,8 +4,17 @@ import ( "github.com/seaweedfs/seaweedfs/weed/admin/topology" "github.com/seaweedfs/seaweedfs/weed/pb" "github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding" + "github.com/seaweedfs/seaweedfs/weed/topology/balancer" ) +// diskTooFull reports whether a disk's physical filesystem is at/above the +// balancing high-water mark, making it ineligible as an EC-shard target even +// when slot math says it has room. Statfs free bytes already include EC shard +// files. No-opinion when the disk reports no bytes (slot-only fallback). +func diskTooFull(d *topology.DiskInfo) bool { + return balancer.DiskTooFullAfter(d.DiskInfo.DiskTotalBytes, d.DiskInfo.DiskFreeBytes, 0, balancer.DefaultMaxDiskUsagePercent) +} + // FromActiveTopology builds a Topology snapshot from the cluster's ActiveTopology // using the reservation-aware effective-capacity view that EC encode and repair // rely on. It collects ALL EC-eligible disks with no hard disk-type filter; @@ -39,7 +48,7 @@ func FromActiveTopology(at *topology.ActiveTopology, dataShards int) *Topology { if d == nil || d.DiskInfo == nil { continue } - if free := perDiskFreeECSlots(at, d, dataShards); free > 0 { + if free := perDiskFreeECSlots(at, d, dataShards); free > 0 && !diskTooFull(d) { nodeFree[d.NodeID] += free } nodeDC[d.NodeID] = d.DataCenter @@ -59,7 +68,7 @@ func FromActiveTopology(at *topology.ActiveTopology, dataShards int) *Topology { node.SetHost(pb.ServerAddress(addr).ToHost()) for _, d := range ds { free := perDiskFreeECSlots(at, d, dataShards) - if free < 0 { + if free < 0 || diskTooFull(d) { free = 0 } node.AddDisk(d.DiskID, d.DiskType, free, ecShardCountOnDisk(d)) diff --git a/weed/storage/erasure_coding/ecbalancer/snapshot_test.go b/weed/storage/erasure_coding/ecbalancer/snapshot_test.go index 4376beb17..dc43a2c1f 100644 --- a/weed/storage/erasure_coding/ecbalancer/snapshot_test.go +++ b/weed/storage/erasure_coding/ecbalancer/snapshot_test.go @@ -102,6 +102,54 @@ func TestFromActiveTopology(t *testing.T) { } } +// A physically near-full disk must contribute zero free EC slots to the snapshot, +// even though its slot math (MaxVolumeCount - VolumeCount) says it has room, so +// the planner never places shards onto it. Mirrors the volume-balance #10160 gate. +func TestFromActiveTopologySkipsPhysicallyFullDisk(t *testing.T) { + const gb = uint64(1) << 30 + at := topology.NewActiveTopology(10) + + // Both disks look slot-empty; only their physical byte fullness differs. + full := &master_pb.DataNodeInfo{ + Id: "10.0.0.1:8080", + DiskInfos: map[string]*master_pb.DiskInfo{ + "hdd": {DiskId: 0, MaxVolumeCount: 100, VolumeCount: 0, DiskTotalBytes: 1000 * gb, DiskFreeBytes: 40 * gb}, // 96% used + }, + } + empty := &master_pb.DataNodeInfo{ + Id: "10.0.0.2:8080", + DiskInfos: map[string]*master_pb.DiskInfo{ + "hdd": {DiskId: 0, MaxVolumeCount: 100, VolumeCount: 0, DiskTotalBytes: 1000 * gb, DiskFreeBytes: 900 * gb}, // 10% used + }, + } + if err := at.UpdateTopology(&master_pb.TopologyInfo{ + DataCenterInfos: []*master_pb.DataCenterInfo{{ + Id: "dc1", + RackInfos: []*master_pb.RackInfo{{Id: "rack1", DataNodeInfos: []*master_pb.DataNodeInfo{full, empty}}}, + }}, + }); err != nil { + t.Fatalf("UpdateTopology: %v", err) + } + + topo := FromActiveTopology(at, 0) + + fullNode := topo.nodes["10.0.0.1:8080"] + if fullNode == nil { + t.Fatal("full node missing") + } + if fullNode.freeSlots != 0 { + t.Errorf("physically full node freeSlots = %d, want 0", fullNode.freeSlots) + } + if d := fullNode.disks[0]; d == nil || d.freeSlots != 0 { + t.Errorf("physically full disk freeSlots = %v, want 0", d) + } + + emptyNode := topo.nodes["10.0.0.2:8080"] + if emptyNode == nil || emptyNode.freeSlots <= 0 { + t.Errorf("physically empty node should keep free slots, got %v", emptyNode) + } +} + // TestFromActiveTopologyGroupsByAddressHost verifies the snapshot derives a node's // machine from its address, not its (possibly opaque) id: two volume servers with // distinct ids but the same host must land on one machine so EC placement spreads diff --git a/weed/worker/tasks/ec_balance/detection.go b/weed/worker/tasks/ec_balance/detection.go index 1b334f459..6d55b23cb 100644 --- a/weed/worker/tasks/ec_balance/detection.go +++ b/weed/worker/tasks/ec_balance/detection.go @@ -13,6 +13,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding/ecbalancer" "github.com/seaweedfs/seaweedfs/weed/storage/super_block" storagetypes "github.com/seaweedfs/seaweedfs/weed/storage/types" + "github.com/seaweedfs/seaweedfs/weed/topology/balancer" "github.com/seaweedfs/seaweedfs/weed/util/wildcard" "github.com/seaweedfs/seaweedfs/weed/worker/tasks/base" "github.com/seaweedfs/seaweedfs/weed/worker/types" @@ -178,6 +179,7 @@ func buildBalancerTopology(topoInfo *master_pb.TopologyInfo, config *Config) (*e freeSlots := 0 diskTypeOf := make(map[uint32]string) // physical disk_id -> disk type diskShardCount := make(map[uint32]int) + fullDiskTypes := make(map[string]bool) // physically near-full, ineligible as a target hasMatchingDisk := false for diskType, diskInfo := range dn.DiskInfos { @@ -186,8 +188,15 @@ func buildBalancerTopology(topoInfo *master_pb.TopologyInfo, config *Config) (*e } hasMatchingDisk = true + // Don't place EC shards on a physically near-full disk, even if slot + // math says it has room (an over-set maxVolumeCount hides real + // fullness). Statfs free bytes already include EC shard files. + if balancer.DiskTooFullAfter(diskInfo.DiskTotalBytes, diskInfo.DiskFreeBytes, 0, balancer.DefaultMaxDiskUsagePercent) { + fullDiskTypes[diskType] = true + } + fs := int(diskInfo.MaxVolumeCount-diskInfo.VolumeCount)*erasure_coding.DataShardsCount - countEcShards(diskInfo.EcShardInfos) - if fs > 0 { + if fs > 0 && !fullDiskTypes[diskType] { freeSlots += fs } // Discover physical disks from regular volumes too, so an @@ -216,12 +225,25 @@ func buildBalancerTopology(topoInfo *master_pb.TopologyInfo, config *Config) (*e // machines, not just nodes (servers on one host are one fault domain). node.SetHost(pb.NewServerAddressFromDataNode(dn).ToHost()) + // Spread the node's free slots only over its non-full disks, so a + // physically full disk type doesn't dilute the share the usable disks + // advertise (its own disks get 0 below). The total is preserved. + nonFullDiskCount := 0 + for _, dt := range diskTypeOf { + if !fullDiskTypes[dt] { + nonFullDiskCount++ + } + } perDiskFree := 0 - if diskCount := len(diskTypeOf); diskCount > 0 && freeSlots > 0 { - perDiskFree = freeSlots / diskCount + if nonFullDiskCount > 0 && freeSlots > 0 { + perDiskFree = freeSlots / nonFullDiskCount } for diskID, diskType := range diskTypeOf { - node.AddDisk(diskID, diskType, perDiskFree, diskShardCount[diskID]) + diskFree := perDiskFree + if fullDiskTypes[diskType] { + diskFree = 0 + } + node.AddDisk(diskID, diskType, diskFree, diskShardCount[diskID]) } // Add shards only for volumes whose collection passes the filter;