diff --git a/weed/admin/topology/capacity.go b/weed/admin/topology/capacity.go index a4ad150b4..4d3eaf4b9 100644 --- a/weed/admin/topology/capacity.go +++ b/weed/admin/topology/capacity.go @@ -162,6 +162,7 @@ func (at *ActiveTopology) GetDisksWithEffectiveCapacity(taskType TaskType, exclu // Create a new DiskInfo with current capacity information diskCopy := DiskInfo{ NodeID: disk.DiskInfo.NodeID, + Address: disk.DiskInfo.Address, DiskID: disk.DiskInfo.DiskID, DiskType: disk.DiskInfo.DiskType, DataCenter: disk.DiskInfo.DataCenter, @@ -226,6 +227,7 @@ func (at *ActiveTopology) GetDisksForPlanning(taskType TaskType, excludeNodeID s // Create a new DiskInfo with planning information diskCopy := DiskInfo{ NodeID: disk.DiskInfo.NodeID, + Address: disk.DiskInfo.Address, DiskID: disk.DiskInfo.DiskID, DiskType: disk.DiskInfo.DiskType, DataCenter: disk.DiskInfo.DataCenter, diff --git a/weed/admin/topology/structs.go b/weed/admin/topology/structs.go index ab4941888..a8417d7c6 100644 --- a/weed/admin/topology/structs.go +++ b/weed/admin/topology/structs.go @@ -43,6 +43,7 @@ type taskState struct { // DiskInfo represents a disk with its current state and ongoing tasks (public for external access) type DiskInfo struct { NodeID string `json:"node_id"` + Address string `json:"address"` // volume server ip:port; NodeID may be an opaque id, so this is the source for the physical-machine host DiskID uint32 `json:"disk_id"` DiskType string `json:"disk_type"` DataCenter string `json:"data_center"` diff --git a/weed/admin/topology/topology_management.go b/weed/admin/topology/topology_management.go index 05acecde8..72fad39f5 100644 --- a/weed/admin/topology/topology_management.go +++ b/weed/admin/topology/topology_management.go @@ -87,6 +87,7 @@ func (at *ActiveTopology) UpdateTopology(topologyInfo *master_pb.TopologyInfo) e disk := &activeDisk{ DiskInfo: &DiskInfo{ NodeID: nodeInfo.Id, + Address: nodeInfo.Address, DiskID: perDisk.DiskId, DiskType: diskType, DataCenter: dc.Id, diff --git a/weed/plugin/worker/volume_metrics.go b/weed/plugin/worker/volume_metrics.go index 8452f40c9..0a9ff97a6 100644 --- a/weed/plugin/worker/volume_metrics.go +++ b/weed/plugin/worker/volume_metrics.go @@ -164,6 +164,7 @@ func buildVolumeMetrics( DataCenter: dc.Id, Rack: rack.Id, NodeID: node.Id, + Host: pb.NewServerAddressFromDataNode(node).ToHost(), }) if collectionRegex != nil && !collectionRegex.MatchString(volume.Collection) { diff --git a/weed/shell/command_ec_common.go b/weed/shell/command_ec_common.go index 199641442..d1066d10f 100644 --- a/weed/shell/command_ec_common.go +++ b/weed/shell/command_ec_common.go @@ -802,6 +802,9 @@ func toBalancerTopology(ecNodes []*EcNode, collections []string, diskType types. for _, en := range ecNodes { rackKey := string(en.dc) + ":" + string(en.rack) node := topo.AddNode(en.info.Id, string(en.dc), rackKey, en.freeEcSlot) + // Group by physical machine (host) so shards spread across machines, not just + // nodes; the id stays the node identity used for moves. + node.SetHost(pb.NewServerAddressFromDataNode(en.info).ToHost()) for diskId, d := range en.disks { node.AddDisk(diskId, d.diskType, d.freeEcSlots, d.ecShardCount) } diff --git a/weed/shell/command_volume_balance.go b/weed/shell/command_volume_balance.go index 616e30b35..d3c88df9b 100644 --- a/weed/shell/command_volume_balance.go +++ b/weed/shell/command_volume_balance.go @@ -553,6 +553,16 @@ func isGoodMove(placement *super_block.ReplicaPlacement, existingReplicas []*Vol } } + // Don't move a replica onto a machine (host) that already holds one of this + // volume's replicas: servers sharing a host are one fault domain, so both would + // die together. Best-effort -- skip and let balancing try the next target. + targetHost := pb.NewServerAddressFromDataNode(targetNode.info).ToHost() + for _, replica := range existingReplicasExceptSourceNode { + if pb.NewServerAddressFromDataNode(replica.location.dataNode).ToHost() == targetHost { + return false + } + } + // target location targetLocation := location{ dc: targetNode.dc, diff --git a/weed/shell/command_volume_balance_test.go b/weed/shell/command_volume_balance_test.go index 35600e2b8..fb037e4f8 100644 --- a/weed/shell/command_volume_balance_test.go +++ b/weed/shell/command_volume_balance_test.go @@ -227,6 +227,41 @@ func TestIsGoodMove(t *testing.T) { targetLocation: location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "dn4"}}, expected: false, }, + + { + // rep 001 allows two copies in one rack; replica-placement alone would + // permit this, but the target shares a host with another replica, so the + // machine anti-affinity must reject it. + name: "test 001 reject move onto a machine already holding a replica", + replication: "001", + replicas: []*VolumeReplica{ + { + location: &location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "10.0.0.1:8080"}}, + }, + { + location: &location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "10.0.0.2:8080"}}, + }, + }, + sourceLocation: location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "10.0.0.2:8080"}}, + targetLocation: location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "10.0.0.1:8081"}}, + expected: false, + }, + + { + name: "test 001 allow move onto a different machine in the rack", + replication: "001", + replicas: []*VolumeReplica{ + { + location: &location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "10.0.0.1:8080"}}, + }, + { + location: &location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "10.0.0.2:8080"}}, + }, + }, + sourceLocation: location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "10.0.0.2:8080"}}, + targetLocation: location{"dc1", "r1", &master_pb.DataNodeInfo{Id: "10.0.0.3:8080"}}, + expected: true, + }, } for _, tt := range tests { diff --git a/weed/storage/erasure_coding/ecbalancer/balancer.go b/weed/storage/erasure_coding/ecbalancer/balancer.go index 3793df83c..7679dada8 100644 --- a/weed/storage/erasure_coding/ecbalancer/balancer.go +++ b/weed/storage/erasure_coding/ecbalancer/balancer.go @@ -32,9 +32,11 @@ type volKey struct { // Node is a volume server in the snapshot. Fields are set through the builder // methods; only its identity is read back (via Move). type Node struct { - id string - dc string - rack string // composite rack key (e.g. "dc1:rack1") + id string + host string // physical machine (host/IP); nodes sharing a host are one fault domain + dc string + rack string // composite rack key (e.g. "dc1:rack1") + freeSlots int disks map[uint32]*disk shards map[volKey]*volumeShards @@ -116,6 +118,7 @@ func NewTopology() *Topology { func (t *Topology) AddNode(id, dc, rackKey string, freeSlots int) *Node { n := &Node{ id: id, + host: id, // default: each node is its own machine until SetHost overrides dc: dc, rack: rackKey, freeSlots: freeSlots, @@ -126,6 +129,14 @@ func (t *Topology) AddNode(id, dc, rackKey string, freeSlots int) *Node { return n } +// SetHost sets the physical machine (host/IP) a node runs on; nodes sharing a host +// are one fault domain. Defaults to the node id (one machine per node) if unset. +func (n *Node) SetHost(host string) { + if host != "" { + n.host = host + } +} + // AddDisk registers a physical disk. shardCount is the disk's total EC shard // count across all volumes (used for disk scoring); freeSlots is the per-disk // free EC shard slots. @@ -188,11 +199,13 @@ func Plan(topo *Topology, opts Options) []Move { } collections := make([]string, 0, len(byCollection)) dataShardsByCollection := make(map[string]int) + parityShardsByCollection := make(map[string]int) for c := range byCollection { collections = append(collections, c) sort.Slice(byCollection[c], func(i, j int) bool { return byCollection[c][i].vid < byCollection[c][j].vid }) - d, _ := ratio(c) + d, p := ratio(c) dataShardsByCollection[c] = d + parityShardsByCollection[c] = p } sort.Strings(collections) @@ -217,7 +230,7 @@ func Plan(topo *Topology, opts Options) []Move { } } - all = append(all, detectGlobalImbalance(nodes, racks, opts.DiskType, opts.ImbalanceThreshold, dataShardsByCollection, opts.GlobalMaxMovesPerRack, opts.GlobalUtilizationBased)...) + all = append(all, detectGlobalImbalance(nodes, racks, opts.DiskType, opts.ImbalanceThreshold, dataShardsByCollection, parityShardsByCollection, opts.GlobalMaxMovesPerRack, opts.GlobalUtilizationBased)...) out := make([]Move, 0, len(all)) for _, m := range all { @@ -405,10 +418,15 @@ func balanceShardTypeAcrossRacks(vk volKey, nodes map[string]*Node, racks map[st } func pickNodeInRack(r *rack, vk volKey, rp *super_block.ReplicaPlacement) *Node { + return pickBestNodeForVolume(sortedNodeSlice(r.nodes), vk, rp) +} + +// pickBestNodeForVolume returns the node with the fewest shards of the volume that +// has a free slot and is under the SameRackCount cap, or nil. +func pickBestNodeForVolume(nodes []*Node, vk volKey, rp *super_block.ReplicaPlacement) *Node { var best *Node bestCount := -1 - for _, id := range sortedNodeKeys(r.nodes) { - node := r.nodes[id] + for _, node := range nodes { if node.freeSlots <= 0 { continue } @@ -423,43 +441,152 @@ func pickNodeInRack(r *rack, vk volKey, rp *super_block.ReplicaPlacement) *Node return best } -// detectWithinRackImbalance spreads a volume's shards across the nodes of each -// rack, again data then parity with anti-affinity. +// detectWithinRackImbalance spreads a volume's shards within each rack, data then +// parity with anti-affinity. It spreads across machines (the fault domain) only when +// the rack has enough that each can stay within EC's parity tolerance; otherwise +// machine spreading buys no durability and would only fight capacity (e.g. cramming +// a 2-server box while a 12-server box sits idle), so it spreads across nodes and +// lets capacity/global balancing decide. +// The imbalance threshold gates only the node fallback (cosmetic load distribution +// that should defer to the global utilization phase). Machine spreading bypasses it: +// the even cap is a durability bound, and a relative-skew gate would skip e.g. a +// 5/4/3 machine layout for a 10+4 volume ((5-3)/4 = 0.5), leaving 5 shards -- past +// parity -- on one machine. func detectWithinRackImbalance(vk volKey, nodes map[string]*Node, racks map[string]*rack, diskType string, threshold float64, dataShards, parityShards int, rp *super_block.ReplicaPlacement) []*move { var moves []*move for _, rackID := range sortedKeys(racks) { r := racks[rackID] - if len(r.nodes) <= 1 { - continue - } + machines := buildMachines(r) + numMachines, numNodes := len(machines), len(r.nodes) - numNodes := len(r.nodes) - // Gate on per-type spread across the rack's nodes (see cross-rack phase). - gateData, gateParity := shardsByGroup(vk, r.nodes, dataShards, func(n *Node) string { return n.id }) - if !typeImbalanced(gateData, numNodes, threshold) && !typeImbalanced(gateParity, numNodes, threshold) { - continue + // Feasibility is about this rack's share of the volume (cross-rack spreading + // already moved the rest elsewhere), not the whole volume: a rack holding 7 of + // a 10+4 volume's shards can keep each of 2 machines within parity even though + // all 14 could not. + rackShards := rackVolumeShardCount(r, vk) + if numMachines > 1 && numMachines < numNodes && parityShards > 0 && ceilDivide(rackShards, numMachines) <= parityShards { + moves = append(moves, withinRackMachineSpread(vk, r, machines, diskType, dataShards, rp)...) + } else if numNodes > 1 { + moves = append(moves, withinRackNodeSpread(vk, r, diskType, threshold, dataShards, rp)...) } - nodeShardCount := countShardsByNode(vk, r.nodes) - - dataPerNode, _ := shardsByGroup(vk, r.nodes, dataShards, func(n *Node) string { return n.id }) - moves = append(moves, balanceShardTypeAcrossNodes(vk, r, diskType, dataShards, - dataPerNode, nodeShardCount, ceilDivide(sumLens(dataPerNode), numNodes), nil, rp)...) - - dataPerNode, parityPerNode := shardsByGroup(vk, r.nodes, dataShards, func(n *Node) string { return n.id }) - antiAffinity := make(map[string]bool) - for nodeID, shards := range dataPerNode { - if len(shards) > 0 { - antiAffinity[nodeID] = true - } - } - moves = append(moves, balanceShardTypeAcrossNodes(vk, r, diskType, dataShards, - parityPerNode, nodeShardCount, ceilDivide(sumLens(parityPerNode), numNodes), antiAffinity, rp)...) } - return moves } +// withinRackMachineSpread spreads a volume's shards across a rack's machines so no +// machine holds more than ceil(rackShards/numMachines). EC recovers from any loss +// within parity regardless of shard type, so what matters per machine is the +// combined count, not data and parity separately: spreading the two independently +// can stack their remainders onto one machine (ceil(d/M)+ceil(p/M) > ceil(total/M)) +// and push it past parity. Data/parity anti-affinity is kept at the disk level by +// pickBestDiskOnNode. With one node per machine this reduces to the node spread. +func withinRackMachineSpread(vk volKey, r *rack, machines map[string][]*Node, diskType string, dataShards int, rp *super_block.ReplicaPlacement) []*move { + machineKeys := sortedKeys(machines) + shardsPerMachine := make(map[string][]int, len(machines)) + total := 0 + for _, host := range machineKeys { + for _, n := range machines[host] { + if info, ok := n.shards[vk]; ok { + for sid := range info.shardBits.All() { + shardsPerMachine[host] = append(shardsPerMachine[host], int(sid)) + } + } + } + sort.Ints(shardsPerMachine[host]) + total += len(shardsPerMachine[host]) + } + if total == 0 { + return nil + } + // Cap = even share. The move loop below sheds only what exceeds it, so a balanced + // rack is a no-op while any machine over the cap (a parity risk) is always fixed. + maxPerMachine := ceilDivide(total, len(machines)) + if maxPerMachine < 1 { + maxPerMachine = 1 + } + + type pending struct { + shardID int + src *Node + } + var toMove []pending + for _, host := range machineKeys { + shards := shardsPerMachine[host] + for i := 0; i < len(shards)-maxPerMachine; i++ { + if src := nodeHoldingShard(machines[host], vk, shards[i]); src != nil { + toMove = append(toMove, pending{shards[i], src}) + } + } + } + + var moves []*move + for _, pm := range toMove { + // A machine is a viable target only if a node on it can actually take the + // shard (free slot, under SameRackCount), so a capped machine is skipped + // rather than settled on and the move dropped. + destHost, ok := pickTarget(machineKeys, shardsPerMachine, maxPerMachine, nil, + func(h string) bool { return h != pm.src.host && pickBestNodeForVolume(machines[h], vk, rp) != nil }, + func(string) bool { return true }) + if !ok { + continue + } + destNode := pickBestNodeForVolume(machines[destHost], vk, rp) + if destNode == nil { + continue + } + destDisk := pickBestDiskOnNode(destNode, vk, diskType, pm.shardID, dataShards) + moves = append(moves, &move{ + volumeID: vk.vid, + shardID: pm.shardID, + collection: vk.collection, + source: pm.src, + sourceDisk: shardDiskID(pm.src, vk, pm.shardID), + target: destNode, + targetDisk: destDisk, + phase: "within_rack", + }) + releaseShard(pm.src, vk, pm.shardID) + reserveShard(destNode, vk, pm.shardID, destDisk) + shardsPerMachine[destHost] = append(shardsPerMachine[destHost], pm.shardID) + shardsPerMachine[pm.src.host] = removeInt(shardsPerMachine[pm.src.host], pm.shardID) + pm.src.freeSlots++ + destNode.freeSlots-- + } + return moves +} + +// withinRackNodeSpread spreads a volume's shards evenly across a rack's nodes (data +// then parity, parity anti-affine to data-bearing nodes). This fallback runs when +// machine fault tolerance is unachievable, so it is cosmetic load distribution: +// honor the imbalance threshold and defer to the global utilization phase rather than +// churning a count-balancing move that can worsen utilization (machine spreading, +// which is durability, bypasses the threshold instead). +func withinRackNodeSpread(vk volKey, r *rack, diskType string, threshold float64, dataShards int, rp *super_block.ReplicaPlacement) []*move { + numNodes := len(r.nodes) + gateData, gateParity := shardsByGroup(vk, r.nodes, dataShards, func(n *Node) string { return n.id }) + if !typeImbalanced(gateData, numNodes, threshold) && !typeImbalanced(gateParity, numNodes, threshold) { + return nil + } + nodeShardCount := countShardsByNode(vk, r.nodes) + + dataPerNode, _ := shardsByGroup(vk, r.nodes, dataShards, func(n *Node) string { return n.id }) + moves := balanceShardTypeAcrossNodes(vk, r, diskType, dataShards, + dataPerNode, nodeShardCount, ceilDivide(sumLens(dataPerNode), numNodes), nil, rp) + + dataPerNode, parityPerNode := shardsByGroup(vk, r.nodes, dataShards, func(n *Node) string { return n.id }) + antiAffinity := make(map[string]bool) + for nodeID, shards := range dataPerNode { + if len(shards) > 0 { + antiAffinity[nodeID] = true + } + } + return append(moves, balanceShardTypeAcrossNodes(vk, r, diskType, dataShards, + parityPerNode, nodeShardCount, ceilDivide(sumLens(parityPerNode), numNodes), antiAffinity, rp)...) +} + +// balanceShardTypeAcrossNodes spreads one shard type of a volume across a rack's +// nodes, moving from nodes over maxPerNode to under-loaded ones. func balanceShardTypeAcrossNodes(vk volKey, r *rack, diskType string, dataShards int, shardsPerNode map[string][]int, nodeShardCount map[string]int, maxPerNode int, antiAffinity map[string]bool, rp *super_block.ReplicaPlacement) []*move { if maxPerNode < 1 { maxPerNode = 1 @@ -523,7 +650,7 @@ func balanceShardTypeAcrossNodes(vk volKey, r *rack, diskType string, dataShards // detectGlobalImbalance balances total EC shard load across the nodes of each // rack (across all volumes), using utilization ratios so heterogeneous-capacity // nodes are compared fairly. -func detectGlobalImbalance(nodes map[string]*Node, racks map[string]*rack, diskType string, threshold float64, dataShardsByCollection map[string]int, maxMovesPerRack int, byUtilization bool) []*move { +func detectGlobalImbalance(nodes map[string]*Node, racks map[string]*rack, diskType string, threshold float64, dataShardsByCollection, parityShardsByCollection map[string]int, maxMovesPerRack int, byUtilization bool) []*move { var moves []*move for _, rackID := range sortedKeys(racks) { @@ -531,6 +658,7 @@ func detectGlobalImbalance(nodes map[string]*Node, racks map[string]*rack, diskT if len(r.nodes) <= 1 { continue } + rackMachineCount := len(buildMachines(r)) nodeShardCounts := make(map[string]int) totalShards := 0 @@ -602,9 +730,8 @@ func detectGlobalImbalance(nodes map[string]*Node, racks map[string]*rack, diskT break } - // Prefer moving a shard of a volume the destination does not hold at - // all (pass 0) before adding another shard of an already-present volume - // (pass 1), to keep each volume's shards spread across nodes. + // Prefer a volume absent from the destination's machine (pass 0) before + // adding to one already there (pass 1), to keep volumes spread. moved := false for pass := 0; pass < 2 && !moved; pass++ { for _, vk := range sortedVolumeKeys(maxNode.shards) { @@ -613,12 +740,26 @@ func detectGlobalImbalance(nodes map[string]*Node, racks map[string]*rack, diskT } info := maxNode.shards[vk] minInfo := minNode.shards[vk] - volumeOnMin := minInfo != nil && minInfo.shardBits != 0 - if pass == 0 && volumeOnMin { - continue // pass 0: only volumes absent from the destination + volumeOnMinMachine := machineHoldsVolume(r, minNode.host, vk) + if pass == 0 && volumeOnMinMachine { + continue // pass 0: only volumes absent from the destination machine } - if pass == 1 && !volumeOnMin { - continue // pass 1: only volumes already on the destination + if pass == 1 { + if !volumeOnMinMachine { + continue + } + // Protect the volume's machine spread only where it's achievable + // (enough machines for this rack's shards to each stay within + // parity); there a cross-machine load move is allowed only if it + // doesn't raise the destination machine's count past the source's. + // Where it isn't achievable, capacity rules and any leveling move + // is fine. Feasibility uses the rack's shards, not the whole volume. + parity := parityShardsByCollection[vk.collection] + spreadFeasible := parity > 0 && rackMachineCount >= ceilDivide(rackVolumeShardCount(r, vk), parity) + if spreadFeasible && minNode.host != maxNode.host && + machineVolumeCount(r, minNode.host, vk) >= machineVolumeCount(r, maxNode.host, vk) { + continue + } } // Walk the volume's actual shard bitmap so custom ratios with more // than the standard total (ids 14..MaxShardCount-1) are candidates too. @@ -893,12 +1034,19 @@ func volumeShardCount(node *Node, vk volKey) int { } func nodeInRackHoldingShard(nodes map[string]*Node, rackID string, vk volKey, shardID int) *Node { - sid := erasure_coding.ShardId(shardID) + var inRack []*Node for _, id := range sortedNodeKeys(nodes) { - node := nodes[id] - if node.rack != rackID { - continue + if nodes[id].rack == rackID { + inRack = append(inRack, nodes[id]) } + } + return nodeHoldingShard(inRack, vk, shardID) +} + +// nodeHoldingShard returns the first node holding the given shard of the volume, or nil. +func nodeHoldingShard(nodes []*Node, vk volKey, shardID int) *Node { + sid := erasure_coding.ShardId(shardID) + for _, node := range nodes { if info, ok := node.shards[vk]; ok && info.shardBits.Has(sid) { return node } @@ -906,6 +1054,52 @@ func nodeInRackHoldingShard(nodes map[string]*Node, rackID string, vk volKey, sh return nil } +// buildMachines groups a rack's nodes by host, each slice sorted by node id. +func buildMachines(r *rack) map[string][]*Node { + machines := make(map[string][]*Node) + for _, n := range sortedNodeSlice(r.nodes) { + machines[n.host] = append(machines[n.host], n) + } + return machines +} + +// rackVolumeShardCount returns how many of the volume's shards the whole rack holds. +func rackVolumeShardCount(r *rack, vk volKey) int { + count := 0 + for _, n := range r.nodes { + count += volumeShardCount(n, vk) + } + return count +} + +// machineVolumeCount returns how many of the volume's shards the machine (host) holds. +func machineVolumeCount(r *rack, host string, vk volKey) int { + count := 0 + for _, n := range r.nodes { + if n.host != host { + continue + } + if info, ok := n.shards[vk]; ok { + count += info.shardBits.Count() + } + } + return count +} + +// machineHoldsVolume reports whether any node on the machine holds a shard of the volume. +func machineHoldsVolume(r *rack, host string, vk volKey) bool { + return machineVolumeCount(r, host, vk) > 0 +} + +func sortedNodeSlice(nodes map[string]*Node) []*Node { + ids := sortedNodeKeys(nodes) + out := make([]*Node, 0, len(ids)) + for _, id := range ids { + out = append(out, nodes[id]) + } + return out +} + func countShardsByRack(vk volKey, nodes map[string]*Node) map[string]int { m := make(map[string]int) for _, node := range nodes { @@ -916,6 +1110,16 @@ func countShardsByRack(vk volKey, nodes map[string]*Node) map[string]int { return m } +func countShardsByHost(vk volKey, nodes map[string]*Node) map[string]int { + m := make(map[string]int) + for _, node := range nodes { + if info, ok := node.shards[vk]; ok { + m[node.host] += info.shardBits.Count() + } + } + return m +} + func countShardsByNode(vk volKey, nodes map[string]*Node) map[string]int { m := make(map[string]int) for id, node := range nodes { diff --git a/weed/storage/erasure_coding/ecbalancer/balancer_test.go b/weed/storage/erasure_coding/ecbalancer/balancer_test.go index 2fac3a5ae..677c4b193 100644 --- a/weed/storage/erasure_coding/ecbalancer/balancer_test.go +++ b/weed/storage/erasure_coding/ecbalancer/balancer_test.go @@ -268,7 +268,7 @@ func TestGlobalImbalanceMovesFromFullToEmpty(t *testing.T) { n2 := topo.AddNode("node2", "dc1", "dc1:rack1", 30) n2.AddShards(300, "col1", 0, allBits(2)) - moves := detectGlobalImbalance(topo.nodes, buildRacks(topo.nodes), "", 0.01, nil, 0, true) + moves := detectGlobalImbalance(topo.nodes, buildRacks(topo.nodes), "", 0.01, nil, nil, 0, true) if len(moves) == 0 { t.Fatal("expected global balance moves") } @@ -288,7 +288,7 @@ func TestGlobalImbalanceHeterogeneousCapacity(t *testing.T) { n2 := topo.AddNode("node2", "dc1", "dc1:rack1", 2) n2.AddShards(200, "col1", 0, allBits(3)) - moves := detectGlobalImbalance(topo.nodes, buildRacks(topo.nodes), "", 0.01, nil, 0, true) + moves := detectGlobalImbalance(topo.nodes, buildRacks(topo.nodes), "", 0.01, nil, nil, 0, true) if len(moves) == 0 { t.Fatal("expected moves from high-util node2 to low-util node1") } @@ -312,7 +312,7 @@ func TestGlobalImbalanceSkipsFullNodes(t *testing.T) { n2 := topo.AddNode("node2", "dc1", "dc1:rack1", 0) // full, cannot receive n2.AddShards(200, "col1", 0, allBits(2)) - if moves := detectGlobalImbalance(topo.nodes, buildRacks(topo.nodes), "", 0.01, nil, 0, true); len(moves) != 0 { + if moves := detectGlobalImbalance(topo.nodes, buildRacks(topo.nodes), "", 0.01, nil, nil, 0, true); len(moves) != 0 { t.Fatalf("expected 0 moves (node2 full), got %d", len(moves)) } } @@ -361,7 +361,7 @@ func TestGlobalPrefersVolumeAbsentFromDestination(t *testing.T) { n2 := topo.AddNode("node2", "dc1", "dc1:rack1", 3) n2.AddShards(100, "col1", 0, bits(2)) - moves := detectGlobalImbalance(topo.nodes, buildRacks(topo.nodes), "", 0.01, nil, 0, true) + moves := detectGlobalImbalance(topo.nodes, buildRacks(topo.nodes), "", 0.01, nil, nil, 0, true) if len(moves) == 0 { t.Fatal("expected a global move from the full node") } @@ -429,3 +429,300 @@ func TestDedupFreesCapacityForLaterPhases(t *testing.T) { t.Error("slot freed by dedup on node2 was not usable by a later phase") } } + +// TestWithinRackSpreadsAcrossMachines: a 10+4 volume's 14 shards all sit on boxA +// (two of its servers), with three other machines free. Four machines is enough for +// the spread to stay within parity, so the within-rack phase must move shards out +// until no machine holds more than ceil(14/4)=4, even though they looked spread +// across boxA's two nodes. +func TestWithinRackSpreadsAcrossMachines(t *testing.T) { + topo := NewTopology() + mk := func(id, host string) *Node { + n := topo.AddNode(id, "dc1", "dc1:rack1", 100) + n.SetHost(host) + n.AddDisk(0, "", 100, 0) + return n + } + a1 := mk("a1", "boxA") + a2 := mk("a2", "boxA") + mk("b1", "boxB") + mk("c1", "boxC") + mk("d1", "boxD") + a1.AddShards(100, "col1", 0, bits(0, 1, 2, 3, 4, 5, 6)) // 7 shards on boxA + a2.AddShards(100, "col1", 0, bits(7, 8, 9, 10, 11, 12, 13)) // 7 shards on boxA (all 14) + + detectWithinRackImbalance(volKey{"col1", 100}, topo.nodes, buildRacks(topo.nodes), "", 0, 10, 4, nil) + + perHost := map[string]int{} + for _, n := range topo.nodes { + if info, ok := n.shards[volKey{"col1", 100}]; ok { + perHost[n.host] += info.shardBits.Count() + } + } + if len(perHost) < 3 { + t.Fatalf("shards not spread across machines: %v", perHost) + } + for h, c := range perHost { + if c > 4 { + t.Errorf("machine %s holds %d shards, want <=4 (ceil(14/4), within parity)", h, c) + } + } +} + +// TestWithinRackMachineSpreadBalancesCombinedOccupancy: a 5+5 volume on two machines +// where each shard type is already within its own per-type cap (boxA 3 data + 3 +// parity = 6, boxB 2 data + 2 parity = 4). Spreading data and parity independently +// would leave boxA at 6, past parity; balancing the combined count must move one +// shard to reach 5/5 so a machine loss stays recoverable. +func TestWithinRackMachineSpreadBalancesCombinedOccupancy(t *testing.T) { + topo := NewTopology() + mk := func(id, host string) *Node { + n := topo.AddNode(id, "dc1", "dc1:rack1", 100) + n.SetHost(host) + n.AddDisk(0, "", 100, 0) + return n + } + a1 := mk("a1", "boxA") + a2 := mk("a2", "boxA") // two nodes on boxA -> numMachines(2) < numNodes(3) + b1 := mk("b1", "boxB") + a1.AddShards(100, "col1", 0, bits(0, 1, 2)) // 3 data + a2.AddShards(100, "col1", 0, bits(5, 6, 7)) // 3 parity (ids >= 5) + b1.AddShards(100, "col1", 0, bits(3, 4, 8, 9)) // 2 data + 2 parity + + // dataShards=5, parityShards=5: feasibility ceil(10/2)=5 <= 5, so machine spread runs. + detectWithinRackImbalance(volKey{"col1", 100}, topo.nodes, buildRacks(topo.nodes), "", 0, 5, 5, nil) + + perHost := map[string]int{} + for _, n := range topo.nodes { + if info, ok := n.shards[volKey{"col1", 100}]; ok { + perHost[n.host] += info.shardBits.Count() + } + } + if perHost["boxA"] > 5 { + t.Errorf("boxA holds %d shards, want <=5 (parity); data and parity spread independently", perHost["boxA"]) + } +} + +// TestWithinRackMachineSpreadActsOnExactlyHalfSkew: a 5/4/3 machine layout for a 10+4 +// volume is only 50% skewed ((5-3)/4 = 0.5), which a 0.5 relative-imbalance threshold +// would skip -- but the 5-shard machine is already past parity. The spread must act +// regardless (the even cap, not a skew threshold, is the bound) and bring every +// machine to <=ceil(12/3)=4. +func TestWithinRackMachineSpreadActsOnExactlyHalfSkew(t *testing.T) { + topo := NewTopology() + mk := func(id, host string) *Node { + n := topo.AddNode(id, "dc1", "dc1:rack1", 100) + n.SetHost(host) + n.AddDisk(0, "", 100, 0) + return n + } + a1 := mk("a1", "boxA") + a2 := mk("a2", "boxA") + b1 := mk("b1", "boxB") + b2 := mk("b2", "boxB") + c1 := mk("c1", "boxC") + c2 := mk("c2", "boxC") + a1.AddShards(100, "col1", 0, bits(0, 1, 2)) + a2.AddShards(100, "col1", 0, bits(3, 4)) // boxA = 5 + b1.AddShards(100, "col1", 0, bits(5, 6)) + b2.AddShards(100, "col1", 0, bits(7, 8)) // boxB = 4 + c1.AddShards(100, "col1", 0, bits(9, 10)) + c2.AddShards(100, "col1", 0, bits(11)) // boxC = 3 + + detectWithinRackImbalance(volKey{"col1", 100}, topo.nodes, buildRacks(topo.nodes), "", 0, 10, 4, nil) + + perHost := map[string]int{} + for _, n := range topo.nodes { + if info, ok := n.shards[volKey{"col1", 100}]; ok { + perHost[n.host] += info.shardBits.Count() + } + } + for h, c := range perHost { + if c > 4 { + t.Errorf("machine %s holds %d shards, want <=4 (ceil(12/3)); a 50%% skew was skipped", h, c) + } + } +} + +// TestWithinRackSpreadUsesRackLocalShardCount: a rack holds only 7 shards of a 10+4 +// volume (cross-rack spreading moved the rest to other racks). Two machines can hold +// those 7 within parity (ceil(7/2)=4), so machine spread must apply -- gating on the +// full 14-shard total would wrongly fall back to node spread and pile 6 onto boxA's +// two nodes, exceeding parity. +func TestWithinRackSpreadUsesRackLocalShardCount(t *testing.T) { + topo := NewTopology() + mk := func(id, host string) *Node { + n := topo.AddNode(id, "dc1", "dc1:rack1", 100) + n.SetHost(host) + n.AddDisk(0, "", 100, 0) + return n + } + a1 := mk("a1", "boxA") + a2 := mk("a2", "boxA") + mk("b1", "boxB") + a1.AddShards(100, "col1", 0, bits(0, 1, 2, 3)) + a2.AddShards(100, "col1", 0, bits(4, 5, 6)) // boxA holds all 7 of this rack's shards + + detectWithinRackImbalance(volKey{"col1", 100}, topo.nodes, buildRacks(topo.nodes), "", 0, 10, 4, nil) + + perHost := map[string]int{} + for _, n := range topo.nodes { + if info, ok := n.shards[volKey{"col1", 100}]; ok { + perHost[n.host] += info.shardBits.Count() + } + } + if perHost["boxA"] > 4 { + t.Errorf("boxA holds %d shards, want <=4 (parity); rack-local feasibility not used", perHost["boxA"]) + } + if perHost["boxB"] == 0 { + t.Error("no shards moved to boxB; machine spread did not apply") + } +} + +// TestWithinRackNodeFallbackHonorsThreshold: one server per host with a 6/4 data +// split is the cosmetic node fallback (a 2-node rack can't be machine-fault-tolerant +// for a 10+4 volume). At 40% skew it's below the 0.5 threshold, so it must be left to +// the global utilization phase rather than churned -- a count move here can worsen +// utilization on unequal-capacity nodes. +func TestWithinRackNodeFallbackHonorsThreshold(t *testing.T) { + topo := NewTopology() + mk := func(id string) *Node { + n := topo.AddNode(id, "dc1", "dc1:rack1", 100) + n.AddDisk(0, "", 100, 0) + return n + } + n1 := mk("n1") // distinct hosts (default host = id) -> node fallback + n2 := mk("n2") + n1.AddShards(100, "col1", 0, bits(0, 1, 2, 3, 4, 5)) // 6 data + n2.AddShards(100, "col1", 0, bits(6, 7, 8, 9)) // 4 data + + moves := detectWithinRackImbalance(volKey{"col1", 100}, topo.nodes, buildRacks(topo.nodes), "", 0.5, 10, 4, nil) + if len(moves) != 0 { + t.Errorf("node fallback moved %d shards on a 40%%-skewed 6/4 layout at threshold 0.5; want 0", len(moves)) + } +} + +// TestWithinRackSpreadDefaultsToNodes: with no SetHost each node is its own +// machine, so the within-rack phase still spreads a volume off an overloaded node +// exactly as before (machine grouping reduces to node grouping). +func TestWithinRackSpreadDefaultsToNodes(t *testing.T) { + topo := NewTopology() + mk := func(id string) *Node { + n := topo.AddNode(id, "dc1", "dc1:rack1", 100) + n.AddDisk(0, "", 100, 0) + return n + } + n1 := mk("n1") + mk("n2") + mk("n3") + n1.AddShards(100, "col1", 0, allBits(14)) // all 14 piled on one node + + detectWithinRackImbalance(volKey{"col1", 100}, topo.nodes, buildRacks(topo.nodes), "", 0, 10, 4, nil) + + perNode := map[string]int{} + for id, n := range topo.nodes { + if info, ok := n.shards[volKey{"col1", 100}]; ok { + perNode[id] = info.shardBits.Count() + } + } + if perNode["n1"] == 14 { + t.Fatal("no within-rack spread happened with one server per host") + } + if len(perNode) < 3 { + t.Errorf("shards not spread across all three nodes: %v", perNode) + } + // Per-type even caps: ceil(10/3) data + ceil(4/3) parity = 4+2 per node. + for id, c := range perNode { + if c > 6 { + t.Errorf("node %s holds %d shards, want <=6 (dataCap+parityCap)", id, c) + } + } +} + +// TestGlobalDoesNotConcentrateVolumeAcrossMachines: when the volume's machine spread +// is achievable (here 2 machines, a 2+2 volume, one machine can hold <= parity), a +// load move must not raise a machine's shard count of the volume past the source's. +// boxA's only volume is also on boxB, so pass 0 finds nothing and the sole pass-1 +// option is the cross-machine boxA->boxB move, which must be rejected (boxB already +// holds as many shards as boxA). Same-machine node rebalancing would still be fine. +func TestGlobalDoesNotConcentrateVolumeAcrossMachines(t *testing.T) { + topo := NewTopology() + a1 := topo.AddNode("a1", "dc1", "dc1:rack1", 0) // full -> high util, the max node + a1.SetHost("boxA") + a1.AddShards(100, "col1", 0, bits(0, 1)) + b1 := topo.AddNode("b1", "dc1", "dc1:rack1", 0) // full -> cannot receive + b1.SetHost("boxB") + b1.AddShards(100, "col1", 0, bits(2, 3)) + b2 := topo.AddNode("b2", "dc1", "dc1:rack1", 10) // empty -> low util, the min node + b2.SetHost("boxB") + + data := map[string]int{"col1": 2} + parity := map[string]int{"col1": 2} + for _, m := range detectGlobalImbalance(topo.nodes, buildRacks(topo.nodes), "", 0.01, data, parity, 0, true) { + if m.source.host != m.target.host { + t.Errorf("cross-machine global move %d.%d from %s to %s concentrates the volume on a machine", + m.volumeID, m.shardID, m.source.host, m.target.host) + } + } +} + +// TestWithinRackMachineSkipsCappedMachine: a machine whose only node is already at +// the per-node SameRackCount cap is not a viable target, and the within-rack spread +// must move shards to the next machine that is, rather than picking the capped one +// and skipping the move. Four machines (boxA has two nodes) make the spread active; +// boxB's node holds two parity shards (== SameRackCount), so boxA's over-concentrated +// data shards must land on the viable boxC/boxD. +func TestWithinRackMachineSkipsCappedMachine(t *testing.T) { + rp, _ := super_block.NewReplicaPlacementFromString("002") // SameRackCount=2 + topo := NewTopology() + mk := func(id, host string) *Node { + n := topo.AddNode(id, "dc1", "dc1:rack1", 100) + n.SetHost(host) + n.AddDisk(0, "", 100, 0) + return n + } + a1 := mk("a1", "boxA") + mk("a2", "boxA") // boxA's second node makes numMachines(4) < numNodes(5) + b1 := mk("b1", "boxB") + c1 := mk("c1", "boxC") + d1 := mk("d1", "boxD") + a1.AddShards(100, "col1", 0, bits(0, 1, 2, 3)) // 4 data shards, over-concentrated on boxA + b1.AddShards(100, "col1", 0, bits(10, 11)) // 2 parity -> b1 at SameRackCount cap + + detectWithinRackImbalance(volKey{"col1", 100}, topo.nodes, buildRacks(topo.nodes), "", 0, 10, 4, rp) + + moved := 0 + for _, n := range []*Node{c1, d1} { + if info, ok := n.shards[volKey{"col1", 100}]; ok { + moved += info.shardBits.Count() + } + } + if moved == 0 { + t.Error("data shards were not moved to the viable machines boxC/boxD; the capped boxB blocked the move") + } +} + +// TestGlobalPrefersVolumeAbsentFromDestinationMachine: the global load phase must +// judge "volume already present" at the machine level. boxB's sibling node holds +// vol100, so draining boxA onto boxB's empty node should move a vol200 shard first +// (vol200 is absent from boxB) rather than piling a second vol100 copy onto boxB. +func TestGlobalPrefersVolumeAbsentFromDestinationMachine(t *testing.T) { + topo := NewTopology() + n1 := topo.AddNode("n1", "dc1", "dc1:rack1", 0) + n1.SetHost("boxA") + n1.AddShards(100, "col1", 0, bits(0, 1, 2, 3)) + n1.AddShards(200, "col1", 0, bits(0, 1, 2, 3)) + n2 := topo.AddNode("n2", "dc1", "dc1:rack1", 5) + n2.SetHost("boxB") + n2.AddShards(100, "col1", 0, bits(4)) // vol100 already on boxB (sibling node) + n3 := topo.AddNode("n3", "dc1", "dc1:rack1", 5) + n3.SetHost("boxB") // empty destination node, but its machine holds vol100 + + moves := detectGlobalImbalance(topo.nodes, buildRacks(topo.nodes), "", 0.01, nil, nil, 0, true) + if len(moves) == 0 { + t.Fatal("expected a global move from the full node") + } + if moves[0].volumeID != 200 { + t.Errorf("first global move is volume %d, want 200 (vol100 already on the destination machine)", moves[0].volumeID) + } +} diff --git a/weed/storage/erasure_coding/ecbalancer/place.go b/weed/storage/erasure_coding/ecbalancer/place.go index 9f98f18ba..c754995e2 100644 --- a/weed/storage/erasure_coding/ecbalancer/place.go +++ b/weed/storage/erasure_coding/ecbalancer/place.go @@ -388,9 +388,13 @@ func rackHasFreeDisk(r *rack, eligible func(*disk) bool) bool { // pickNodeInRackEligible is pickNodeInRack restricted to nodes that have a free // eligible disk. FromActiveTopology keeps all disk types/tags in the snapshot, so // without this a node with free volume slots but no eligible disk could be chosen. +// +// Among eligible nodes it prefers the one whose machine holds the fewest shards of +// the volume (tie-broken by the node's own count), spreading shards across machines. func pickNodeInRackEligible(r *rack, vk volKey, rp *super_block.ReplicaPlacement, eligible func(*disk) bool) *Node { + machineShards := countShardsByHost(vk, r.nodes) var best *Node - bestCount := -1 + bestMachineCount, bestNodeCount := -1, -1 for _, id := range sortedNodeKeys(r.nodes) { node := r.nodes[id] if node.freeSlots <= 0 { @@ -403,8 +407,9 @@ func pickNodeInRackEligible(r *rack, vk volKey, rp *super_block.ReplicaPlacement if rp != nil && rp.SameRackCount > 0 && count >= rp.SameRackCount { continue } - if best == nil || count < bestCount { - best, bestCount = node, count + mCount := machineShards[node.host] + if best == nil || mCount < bestMachineCount || (mCount == bestMachineCount && count < bestNodeCount) { + best, bestMachineCount, bestNodeCount = node, mCount, count } } return best diff --git a/weed/storage/erasure_coding/ecbalancer/place_test.go b/weed/storage/erasure_coding/ecbalancer/place_test.go index 6af01a331..4b2814270 100644 --- a/weed/storage/erasure_coding/ecbalancer/place_test.go +++ b/weed/storage/erasure_coding/ecbalancer/place_test.go @@ -73,6 +73,44 @@ func TestPlaceStrictSpreadAndCaps(t *testing.T) { } } +// TestPlaceSpreadsAcrossMachines: one rack runs two physical machines (each with +// four volume servers). Placing a 10+4 volume must spread its shards across both +// machines so no single machine holds more than ceil(14/2)=7, otherwise losing one +// box would take out more shards than EC can recover even though they look spread +// across distinct nodes. +func TestPlaceSpreadsAcrossMachines(t *testing.T) { + topo := NewTopology() + host := map[string]string{} + add := func(id, h string) { + n := topo.AddNode(id, "dc1", "dc1:rack0", 50) + n.SetHost(h) + n.AddDisk(0, "", 50, 0) + host[id] = h + } + for i := 0; i < 4; i++ { + add(fmt.Sprintf("a%d", i), "10.0.0.1") + add(fmt.Sprintf("b%d", i), "10.0.0.2") + } + + res, err := topo.Place(1, "c1", allShards(), Constraints{}, PlaceStrict) + if err != nil { + t.Fatalf("Place: %v", err) + } + perMachine := map[string]int{} + for _, d := range res.Destinations { + perMachine[host[d.Node]]++ + } + if len(perMachine) < 2 { + t.Fatalf("shards not spread across machines: %v", perMachine) + } + maxPerMachine := ceilDivide(erasure_coding.TotalShardsCount, 2) + for h, c := range perMachine { + if c > maxPerMachine { + t.Errorf("machine %s holds %d shards, want <=%d (ceil(14/2))", h, c, maxPerMachine) + } + } +} + // TestPlaceStrictFailsAndRollsBack: a single tiny disk cannot hold 14 shards, so // strict Place fails and leaves the snapshot untouched. func TestPlaceStrictFailsAndRollsBack(t *testing.T) { diff --git a/weed/storage/erasure_coding/ecbalancer/snapshot.go b/weed/storage/erasure_coding/ecbalancer/snapshot.go index 9d5c81103..a8380caac 100644 --- a/weed/storage/erasure_coding/ecbalancer/snapshot.go +++ b/weed/storage/erasure_coding/ecbalancer/snapshot.go @@ -2,6 +2,7 @@ package ecbalancer import ( "github.com/seaweedfs/seaweedfs/weed/admin/topology" + "github.com/seaweedfs/seaweedfs/weed/pb" "github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding" ) @@ -32,6 +33,7 @@ func FromActiveTopology(at *topology.ActiveTopology, dataShards int) *Topology { nodeFree := make(map[string]int) nodeDC := make(map[string]string) nodeRack := make(map[string]string) + nodeAddr := make(map[string]string) byNode := make(map[string][]*topology.DiskInfo) for _, d := range disks { if d == nil || d.DiskInfo == nil { @@ -42,11 +44,19 @@ func FromActiveTopology(at *topology.ActiveTopology, dataShards int) *Topology { } nodeDC[d.NodeID] = d.DataCenter nodeRack[d.NodeID] = d.DataCenter + ":" + d.Rack + nodeAddr[d.NodeID] = d.Address byNode[d.NodeID] = append(byNode[d.NodeID], d) } for nodeID, ds := range byNode { node := topo.AddNode(nodeID, nodeDC[nodeID], nodeRack[nodeID], nodeFree[nodeID]) + // Group by physical machine: derive the host from the address (NodeID may be + // an opaque id), falling back to the id when no address is recorded. + addr := nodeAddr[nodeID] + if addr == "" { + addr = nodeID + } + node.SetHost(pb.ServerAddress(addr).ToHost()) for _, d := range ds { free := perDiskFreeECSlots(at, d, dataShards) if free < 0 { diff --git a/weed/storage/erasure_coding/ecbalancer/snapshot_test.go b/weed/storage/erasure_coding/ecbalancer/snapshot_test.go index a7e54dbcf..4376beb17 100644 --- a/weed/storage/erasure_coding/ecbalancer/snapshot_test.go +++ b/weed/storage/erasure_coding/ecbalancer/snapshot_test.go @@ -102,6 +102,47 @@ func TestFromActiveTopology(t *testing.T) { } } +// 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 +// shards across boxes even when explicit node ids are configured. +func TestFromActiveTopologyGroupsByAddressHost(t *testing.T) { + at := topology.NewActiveTopology(10) + mk := func(id, addr string) *master_pb.DataNodeInfo { + return &master_pb.DataNodeInfo{ + Id: id, + Address: addr, + DiskInfos: map[string]*master_pb.DiskInfo{"hdd": {DiskId: 0, MaxVolumeCount: 100, VolumeCount: 0}}, + } + } + // vs-a and vs-b are two servers on the same physical host; vs-c is another box. + nodes := []*master_pb.DataNodeInfo{ + mk("vs-a", "10.0.0.9:8080"), + mk("vs-b", "10.0.0.9:8081"), + mk("vs-c", "10.0.0.10:8080"), + } + if err := at.UpdateTopology(&master_pb.TopologyInfo{ + DataCenterInfos: []*master_pb.DataCenterInfo{{ + Id: "dc1", + RackInfos: []*master_pb.RackInfo{{Id: "rack1", DataNodeInfos: nodes}}, + }}, + }); err != nil { + t.Fatalf("UpdateTopology: %v", err) + } + + topo := FromActiveTopology(at, 0) + if topo.nodes["vs-a"].host != topo.nodes["vs-b"].host { + t.Errorf("vs-a host %q != vs-b host %q; same-machine servers not grouped", + topo.nodes["vs-a"].host, topo.nodes["vs-b"].host) + } + if topo.nodes["vs-a"].host == topo.nodes["vs-c"].host { + t.Errorf("vs-a and vs-c share host %q but are different machines", topo.nodes["vs-a"].host) + } + if got := topo.nodes["vs-a"].host; got != "10.0.0.9" { + t.Errorf("vs-a host = %q, want 10.0.0.9", got) + } +} + // TestEcShardSlotsOnDiskRoundsUp covers the mixed-ratio (targetDataShards < // existingDataShards) conversion: an existing shard's fractional footprint must // round up so it is never floored to zero, which would overstate free capacity. diff --git a/weed/topology/node.go b/weed/topology/node.go index f6073326f..d1d121471 100644 --- a/weed/topology/node.go +++ b/weed/topology/node.go @@ -144,11 +144,11 @@ type Node interface { } type NodeImpl struct { - diskUsages *DiskUsages - id NodeId - parent Node + diskUsages *DiskUsages + id NodeId + parent Node sync.RWMutex // lock children - children map[NodeId]Node + children map[NodeId]Node // maxVolumeId uses atomic ops so UpAdjustMaxVolumeId (called from the // volume server heartbeat path) and GetMaxVolumeId (called from the // master's assign / warmup checks) can run concurrently without a @@ -167,6 +167,38 @@ func (n *NodeImpl) GetDiskUsages() *DiskUsages { return n.diskUsages } +// nodeHost returns the host a node runs on, or "" for non-data-node tiers (data +// centers, racks). +func nodeHost(node Node) string { + if dn, ok := node.(*DataNode); ok { + return dn.Ip + } + return "" +} + +// preferDistinctHosts reorders candidates so the first node of each not-yet-used +// host comes first (preserving weighted order), then the same-host leftovers, so a +// prefix covers the most distinct machines. usedHost seeds the set. No-op when +// hosts are empty (non-data-node tiers). +func preferDistinctHosts(usedHost string, candidates []Node) []Node { + used := map[string]bool{} + if usedHost != "" { + used[usedHost] = true + } + distinct := make([]Node, 0, len(candidates)) + dup := make([]Node, 0, len(candidates)) + for _, node := range candidates { + h := nodeHost(node) + if h != "" && !used[h] { + used[h] = true + distinct = append(distinct, node) + } else { + dup = append(dup, node) + } + } + return append(distinct, dup...) +} + // the first node must satisfy filterFirstNodeFn(), the rest nodes must have one free slot func (n *NodeImpl) PickNodesByWeight(numberOfNodes int, option *VolumeGrowOption, filterFirstNodeFn func(dn Node) error) (firstNode Node, restNodes []Node, err error) { var totalWeights int64 @@ -215,12 +247,17 @@ func (n *NodeImpl) PickNodesByWeight(numberOfNodes int, option *VolumeGrowOption for k, node := range sortedCandidates { if err := filterFirstNodeFn(node); err == nil { firstNode = node - if k >= numberOfNodes-1 { - restNodes = sortedCandidates[:numberOfNodes-1] - } else { - restNodes = append(restNodes, sortedCandidates[:k]...) - restNodes = append(restNodes, sortedCandidates[k+1:numberOfNodes]...) + // Fill the rest preferring not-yet-used hosts, so replicas spread across + // machines; falls back to same-host when too few. No-op for dc/rack tiers + // (empty host), which keep the weighted order. + pool := make([]Node, 0, len(sortedCandidates)-1) + pool = append(pool, sortedCandidates[:k]...) + pool = append(pool, sortedCandidates[k+1:]...) + pool = preferDistinctHosts(nodeHost(firstNode), pool) + if len(pool) > numberOfNodes-1 { + pool = pool[:numberOfNodes-1] } + restNodes = pool ret = true break } else { diff --git a/weed/topology/volume_growth_test.go b/weed/topology/volume_growth_test.go index d0b30c2ca..11e3ba082 100644 --- a/weed/topology/volume_growth_test.go +++ b/weed/topology/volume_growth_test.go @@ -475,3 +475,40 @@ func TestPickForWrite(t *testing.T) { } } } + +// TestPickNodesByWeightPrefersDistinctHosts: a rack runs two physical machines +// with two volume servers each. Picking two nodes (e.g. for a same-rack replica +// pair) must land them on distinct hosts so a single machine failure cannot take +// out both replicas, even though the four data nodes are independent. +func TestPickNodesByWeightPrefersDistinctHosts(t *testing.T) { + rack := NewRack("rack1") + mk := func(id, ip string, maxVol int64) *DataNode { + dn := NewDataNode(id) + dn.Ip = ip + rack.LinkChildNode(dn) + disk := dn.getOrCreateDisk("") + disk.UpAdjustDiskUsageDelta("", &DiskUsageCounts{maxVolumeCount: maxVol}) + return dn + } + mk("s1a", "10.0.0.1", 10) + mk("s1b", "10.0.0.1", 10) + mk("s2a", "10.0.0.2", 10) + mk("s2b", "10.0.0.2", 10) + + option := &VolumeGrowOption{DiskType: types.HardDriveType} + + // Selection is weighted-random, so exercise it repeatedly: with two machines + // available the chosen pair must always span both. + for i := 0; i < 50; i++ { + first, rest, err := rack.PickNodesByWeight(2, option, func(node Node) error { return nil }) + if err != nil { + t.Fatalf("PickNodesByWeight: %v", err) + } + if len(rest) != 1 { + t.Fatalf("got %d rest nodes, want 1", len(rest)) + } + if nodeHost(first) == nodeHost(rest[0]) { + t.Errorf("both picks on host %s; expected distinct machines", nodeHost(first)) + } + } +} diff --git a/weed/worker/tasks/balance/detection.go b/weed/worker/tasks/balance/detection.go index d0d73df72..3d5154fba 100644 --- a/weed/worker/tasks/balance/detection.go +++ b/weed/worker/tasks/balance/detection.go @@ -662,6 +662,7 @@ func planBalanceDestination(activeTopology *topology.ActiveTopology, selectedVol DataCenter: disk.DataCenter, Rack: disk.Rack, NodeID: disk.NodeID, + Host: hostFromAddress(disk.Address, disk.NodeID), } if !IsGoodMove(rp, replicas, selectedVolume.Server, target) { continue @@ -771,6 +772,7 @@ func isValidBalanceDestination(plan *topology.DestinationPlan, allowedServers ma DataCenter: plan.TargetDC, Rack: plan.TargetRack, NodeID: plan.TargetNode, + Host: hostFromAddress(plan.TargetAddress, plan.TargetNode), } return IsGoodMove(rp, replicas, sourceNodeID, target) } diff --git a/weed/worker/tasks/balance/replica_placement.go b/weed/worker/tasks/balance/replica_placement.go index c90ad6fb0..d84333ec3 100644 --- a/weed/worker/tasks/balance/replica_placement.go +++ b/weed/worker/tasks/balance/replica_placement.go @@ -3,10 +3,21 @@ package balance import ( "slices" + "github.com/seaweedfs/seaweedfs/weed/pb" "github.com/seaweedfs/seaweedfs/weed/storage/super_block" "github.com/seaweedfs/seaweedfs/weed/worker/types" ) +// hostFromAddress returns the physical machine (host/IP) of a server address, +// falling back to the node id (== ip:port in the common case) when no address is +// known. Servers sharing a host are one fault domain. +func hostFromAddress(address, nodeID string) string { + if address == "" { + address = nodeID + } + return pb.ServerAddress(address).ToHost() +} + // rackKey uniquely identifies a rack within a data center. type rackKey struct { DataCenter string @@ -44,6 +55,17 @@ func IsGoodMove(rp *super_block.ReplicaPlacement, existingReplicas []types.Repli return false } + // Best-effort machine anti-affinity: don't move a replica onto a host that + // already holds another replica of this volume, so a single machine failure + // can't take out two replicas. + if target.Host != "" { + for _, r := range afterMove { + if r.Host == target.Host { + return false + } + } + } + return satisfyReplicaPlacement(rp, afterMove, target) } diff --git a/weed/worker/tasks/balance/replica_placement_test.go b/weed/worker/tasks/balance/replica_placement_test.go index 303fb192b..baa8f1eb7 100644 --- a/weed/worker/tasks/balance/replica_placement_test.go +++ b/weed/worker/tasks/balance/replica_placement_test.go @@ -29,6 +29,24 @@ func TestIsGoodMove_NoReplication(t *testing.T) { } } +func TestIsGoodMove_MachineAntiAffinity(t *testing.T) { + // rep 001 allows two copies in one rack, so replica placement alone would permit + // moving onto another port of a host that already holds a replica; machine + // anti-affinity must reject that and allow a distinct host. + existing := []types.ReplicaLocation{ + {DataCenter: "dc1", Rack: "r1", NodeID: "10.0.0.1:8080", Host: "10.0.0.1"}, + {DataCenter: "dc1", Rack: "r1", NodeID: "10.0.0.2:8080", Host: "10.0.0.2"}, + } + onSameMachine := types.ReplicaLocation{DataCenter: "dc1", Rack: "r1", NodeID: "10.0.0.1:8081", Host: "10.0.0.1"} + if IsGoodMove(rp(t, "001"), existing, "10.0.0.2:8080", onSameMachine) { + t.Error("move onto a machine already holding a replica should be rejected") + } + onOtherMachine := types.ReplicaLocation{DataCenter: "dc1", Rack: "r1", NodeID: "10.0.0.3:8080", Host: "10.0.0.3"} + if !IsGoodMove(rp(t, "001"), existing, "10.0.0.2:8080", onOtherMachine) { + t.Error("move onto a distinct machine should be allowed") + } +} + func TestIsGoodMove_001_SameRack(t *testing.T) { // 001 = 1 replica on same rack (2 total on same rack) existing := []types.ReplicaLocation{ diff --git a/weed/worker/tasks/ec_balance/detection.go b/weed/worker/tasks/ec_balance/detection.go index d62d394db..4fe2e764b 100644 --- a/weed/worker/tasks/ec_balance/detection.go +++ b/weed/worker/tasks/ec_balance/detection.go @@ -6,6 +6,7 @@ import ( "time" "github.com/seaweedfs/seaweedfs/weed/glog" + "github.com/seaweedfs/seaweedfs/weed/pb" "github.com/seaweedfs/seaweedfs/weed/pb/master_pb" "github.com/seaweedfs/seaweedfs/weed/pb/worker_pb" "github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding" @@ -198,6 +199,9 @@ func buildBalancerTopology(topoInfo *master_pb.TopologyInfo, config *Config) (*e } node := topo.AddNode(dn.Id, dc.Id, rackKey, freeSlots) + // Group servers sharing a host so a volume's shards spread across + // machines, not just nodes (servers on one host are one fault domain). + node.SetHost(pb.NewServerAddressFromDataNode(dn).ToHost()) perDiskFree := 0 if diskCount := len(diskTypeOf); diskCount > 0 && freeSlots > 0 { diff --git a/weed/worker/tasks/ec_balance/detection_test.go b/weed/worker/tasks/ec_balance/detection_test.go index 66d473952..16a3431bc 100644 --- a/weed/worker/tasks/ec_balance/detection_test.go +++ b/weed/worker/tasks/ec_balance/detection_test.go @@ -2,6 +2,7 @@ package ec_balance import ( "context" + "net" "testing" "github.com/seaweedfs/seaweedfs/weed/pb/master_pb" @@ -50,6 +51,51 @@ func TestBuildBalancerTopology(t *testing.T) { } } +// TestBuildBalancerTopologyGroupsByHost: two volume servers on host 10.0.0.1 +// (different ports) plus three other hosts, a 10+4 volume concentrated on the +// 10.0.0.1 machine. Four machines is enough to spread within parity, so after +// planning the 10.0.0.1 machine must hold <=4 shards of the volume -- which only +// holds if its two ports are grouped into one machine (host wired into the build). +func TestBuildBalancerTopologyGroupsByHost(t *testing.T) { + mkNode := func(id string, bits uint32) *master_pb.DataNodeInfo { + di := &master_pb.DiskInfo{Type: "", MaxVolumeCount: 100} + if bits != 0 { + di.EcShardInfos = []*master_pb.VolumeEcShardInformationMessage{{Id: 100, Collection: "col1", DiskId: 0, EcIndexBits: bits}} + } + return &master_pb.DataNodeInfo{Id: id, DiskInfos: map[string]*master_pb.DiskInfo{"": di}} + } + topoInfo := &master_pb.TopologyInfo{ + DataCenterInfos: []*master_pb.DataCenterInfo{{ + Id: "dc1", + RackInfos: []*master_pb.RackInfo{{Id: "rack1", DataNodeInfos: []*master_pb.DataNodeInfo{ + mkNode("10.0.0.1:8080", 0x007F), // shards 0-6 on host 10.0.0.1 + mkNode("10.0.0.1:8081", 0x3F80), // shards 7-13 on host 10.0.0.1 + mkNode("10.0.0.2:8080", 0), + mkNode("10.0.0.3:8080", 0), + mkNode("10.0.0.4:8080", 0), + }}}, + }}, + } + + topo, _ := buildBalancerTopology(topoInfo, NewDefaultConfig()) + moves := ecbalancer.Plan(topo, ecbalancer.Options{ImbalanceThreshold: 0.01}) + + host := func(nodeID string) string { h, _, _ := net.SplitHostPort(nodeID); return h } + count := map[string]int{"10.0.0.1": 14} + for _, m := range moves { + if m.VolumeID != 100 { + continue + } + count[host(m.SourceNode)]-- + if m.SourceNode != m.TargetNode { // non-dedup move + count[host(m.TargetNode)]++ + } + } + if count["10.0.0.1"] > 4 { + t.Errorf("machine 10.0.0.1 holds %d shards of the volume after balancing, want <=4 (host grouping not applied)", count["10.0.0.1"]) + } +} + func TestBuildBalancerTopologyCollectionFilter(t *testing.T) { config := NewDefaultConfig() config.CollectionFilter = "other" // does not match the volume's collection diff --git a/weed/worker/types/data_types.go b/weed/worker/types/data_types.go index cd3040597..e42006163 100644 --- a/weed/worker/types/data_types.go +++ b/weed/worker/types/data_types.go @@ -12,6 +12,7 @@ type ReplicaLocation struct { DataCenter string Rack string NodeID string + Host string // physical machine (host/IP); servers sharing a host are one fault domain } // ClusterInfo contains cluster information for task detection