diff --git a/weed/shell/command_volume_balance.go b/weed/shell/command_volume_balance.go index 8c259645c..ff8f521a0 100644 --- a/weed/shell/command_volume_balance.go +++ b/weed/shell/command_volume_balance.go @@ -71,11 +71,12 @@ func (c *commandVolumeBalance) Help() string { sizes are handled correctly. Set it to 0 (or >=100) to disable. Servers running an older build that does not report disk bytes are not gated, and balancing falls back to slot-only behavior for them. - The -byDiskUsage flag ranks servers by the actual data they hold (sum of volume sizes) instead of the - default slot-density metric. The default metric normalizes by maxVolumeCount, so a server whose - maxVolumeCount is configured too high for its disk looks nearly empty even when its disk is physically - full, and balancing can drain less-full servers onto it. Use -byDiskUsage to balance actual data - distribution instead. It assumes comparable disk sizes across servers of the same disk type. + The -byDiskUsage flag ranks servers by their reported physical disk used percentage instead of the + default slot-density metric. If any server does not report physical disk bytes (older build), ranking + falls back to the sum of volume sizes for all servers, since the two scales are not comparable. The + default metric normalizes by maxVolumeCount, so a server whose maxVolumeCount is configured too high + for its disk looks nearly empty even when its disk is physically full, and balancing can drain + less-full servers onto it. Use -byDiskUsage to balance actual disk usage instead. Algorithm: @@ -129,7 +130,7 @@ func (c *commandVolumeBalance) Do(args []string, commandEnv *CommandEnv, writer // TODO: remove this alias applyBalancingAlias := balanceCommand.Bool("force", false, "apply the balancing plan (alias for -apply)") volumesPerExec := balanceCommand.Int("volumesPerExec", 0, "how many volumes to move in one run (default is 0 for unlimited)") - byDiskUsage := balanceCommand.Bool("byDiskUsage", false, "rank servers by actual data held (sum of volume sizes) instead of slot density; use when maxVolumeCount is set too high for the disk. Assumes comparable disk sizes per disk type.") + byDiskUsage := balanceCommand.Bool("byDiskUsage", false, "rank servers by reported physical disk used percent instead of slot density; falls back to sum of volume sizes for all servers when any server does not report disk bytes. Use when maxVolumeCount is set too high for the disk.") maxDiskUsagePercent := balanceCommand.Int("maxDiskUsagePercent", balancer.DefaultMaxDiskUsagePercent, "skip a move target whose physical disk used%% is at/above this; judged per server against its own disk, so heterogeneous disk sizes are fine. 0 or >=100 disables. Auto-skipped for servers that do not report disk bytes.") balanceCommand.Func("volumeBy", "only apply the balancing for ALL volumes and ACTIVE or FULL", func(flagValue string) error { @@ -339,29 +340,45 @@ func capacityByMinVolumeDensity(diskType types.DiskType, volumeSizeLimitMb uint6 } } -// capacityByActualDataUsage ranks servers purely by how much actual data they -// hold (sum of volume sizes), ignoring MaxVolumeCount. The slot-density metric -// divides by MaxVolumeCount, so a server whose MaxVolumeCount was configured too -// high for its disk looks nearly empty even when its disk is physically full and -// gets picked as a move target. This function keeps the fullest-by-data server -// ranked as full so balancing drains it instead of piling onto it. It assumes -// comparable disk sizes across servers of the same disk type. Capacity is a -// uniform constant so the density ratio is proportional to actual data; the -// constant cancels out of every ratio comparison in balanceSelectedVolume. -func capacityByActualDataUsage(diskType types.DiskType, volumeSizeLimitMb uint64) DensityFunc { +// capacityByDiskUsage ranks servers by reported physical disk used percentage. +// This makes a physically full disk rank as a move source, even if the regular +// SeaweedFS volumes in topology do not make it look like the largest data holder. +// The percent scale is only comparable when every server reports disk bytes, so +// if any node lacks DiskTotalBytes (older build), all nodes fall back to the +// previous ranking by summed volume sizes with a uniform capacity: mixing the two +// scales would rank non-reporting servers as orders of magnitude fuller, and +// normalizing the fallback by MaxVolumeCount instead would reintroduce the +// over-configured-maxVolumeCount distortion this flag exists to avoid. +func capacityByDiskUsage(diskType types.DiskType, volumeSizeLimitMb uint64, nodes []*Node) DensityFunc { + if volumeSizeLimitMb == 0 { + volumeSizeLimitMb = util.VolumeSizeLimitGB * util.KiByte + } + volumeSizeLimitBytes := volumeSizeLimitMb * util.MiByte + allReportDiskBytes := true + for _, n := range nodes { + if diskInfo, found := n.info.DiskInfos[string(diskType)]; found && diskInfo != nil && diskInfo.DiskTotalBytes == 0 { + allReportDiskBytes = false + break + } + } return func(info *master_pb.DataNodeInfo) (float64, uint64) { diskInfo, found := info.DiskInfos[string(diskType)] if !found || diskInfo == nil { return 0, 0 } + if allReportDiskBytes && diskInfo.DiskTotalBytes > 0 { + usedBytes := uint64(0) + if diskInfo.DiskFreeBytes < diskInfo.DiskTotalBytes { + usedBytes = diskInfo.DiskTotalBytes - diskInfo.DiskFreeBytes + } + return float64(diskInfo.DiskTotalBytes) / float64(volumeSizeLimitBytes), + balancer.UsedVolumeEquivalents(usedBytes, volumeSizeLimitBytes) + } var volumeSizes uint64 for _, volumeInfo := range diskInfo.VolumeInfos { volumeSizes += volumeInfo.Size } - if volumeSizeLimitMb == 0 { - volumeSizeLimitMb = util.VolumeSizeLimitGB * util.KiByte - } - return 1, balancer.UsedVolumeEquivalents(volumeSizes, volumeSizeLimitMb*util.MiByte) + return 1, balancer.UsedVolumeEquivalents(volumeSizes, volumeSizeLimitBytes) } } @@ -486,7 +503,7 @@ func (c *commandVolumeBalance) balanceSelectedVolume(diskType types.DiskType, vo } capacityFunc := capacityByMinVolumeDensity(diskType, volumeSizeLimitMb) if c.byDiskUsage { - capacityFunc = capacityByActualDataUsage(diskType, volumeSizeLimitMb) + capacityFunc = capacityByDiskUsage(diskType, volumeSizeLimitMb, nodes) } for _, dn := range nodes { capacity, volumeCount := capacityFunc(dn.info) @@ -544,9 +561,9 @@ func (c *commandVolumeBalance) balanceSelectedVolume(diskType types.DiskType, vo } sortCandidatesFn(candidateVolumes) for _, emptyNode := range nodesWithCapacity[:fullNodeIndex] { - // In byte-usage mode capacity is a uniform constant, so a target's - // free volume slots aren't reflected in its ranking; skip targets that - // are already at MaxVolumeCount so balancing never exceeds the slot limit. + // In byte-usage mode the ranking ignores volume slots, so skip targets + // that are already at MaxVolumeCount so balancing never exceeds the + // slot limit. if c.byDiskUsage && !emptyNode.hasFreeVolumeSlot(diskType) { continue } diff --git a/weed/shell/command_volume_balance_test.go b/weed/shell/command_volume_balance_test.go index dd36bb926..149103334 100644 --- a/weed/shell/command_volume_balance_test.go +++ b/weed/shell/command_volume_balance_test.go @@ -444,6 +444,68 @@ func TestBalanceByDiskUsage(t *testing.T) { } } +// When volume servers report physical disk bytes, -byDiskUsage should use the +// filesystem fullness as the source ranking. This covers the production-shaped +// case where a 99%-used disk does not have the largest sum of regular +// VolumeInfos, so ranking by volume bytes alone drains the wrong server. +func TestBalanceByDiskUsageUsesPhysicalDiskUsageWhenReported(t *testing.T) { + const gb = 1024 * 1024 * 1024 + volumeSizeLimitMb := uint64(1024) + + physicalFull := makeByteNode("physical-full", 2000, 1000*gb, 10*gb, mkByteVolumes(1, 5, 1000)) // 99% used + dataHeavy := makeByteNode("data-heavy", 2000, 1000*gb, 530*gb, mkByteVolumes(101, 20, 1000)) // 47% used, more volume bytes + target := makeByteNode("target", 2000, 1000*gb, 900*gb, mkByteVolumes(201, 1, 1000)) // 10% used + beforePhysicalFull := volCount(physicalFull) + beforeDataHeavy := volCount(dataHeavy) + beforeTarget := volCount(target) + + c := &commandVolumeBalance{ + volumeSizeLimitMb: volumeSizeLimitMb, + byDiskUsage: true, + diskUsageHighWaterPercent: 90, + } + runBalance(t, c, []*Node{physicalFull, dataHeavy, target}) + + if got := volCount(physicalFull); got >= beforePhysicalFull { + t.Fatalf("-byDiskUsage should drain the physically fullest server, got %d volumes (was %d)", got, beforePhysicalFull) + } + if got := volCount(dataHeavy); got != beforeDataHeavy { + t.Fatalf("-byDiskUsage should not drain the lower physical-usage data-heavy server first, got %d volumes (was %d)", got, beforeDataHeavy) + } + if got := volCount(target); got <= beforeTarget { + t.Fatalf("expected the low-usage target to receive volumes, got %d volumes (was %d)", got, beforeTarget) + } +} + +// A fleet where only some servers report physical disk bytes must not mix the +// percent scale with the volume-bytes scale: a non-reporting server's ratio is +// whole volume-equivalents while reporting servers stay below 1.0, so the +// balancer would drain a nearly empty old-build server onto physically fuller +// ones. With mixed reporting, ranking falls back to data size for everyone. +func TestBalanceByDiskUsageMixedReportingFallsBackToDataSize(t *testing.T) { + const gb = 1024 * 1024 * 1024 + + oldEmpty := makeByteNode("old-empty", 2000, 0, 0, mkByteVolumes(1, 2, 1000)) // no disk bytes reported + newFull := makeByteNode("new-full", 2000, 1000*gb, 500*gb, mkByteVolumes(101, 20, 1000)) + newTarget := makeByteNode("new-target", 2000, 1000*gb, 990*gb, mkByteVolumes(201, 1, 1000)) + beforeOldEmpty := volCount(oldEmpty) + beforeNewFull := volCount(newFull) + + c := &commandVolumeBalance{ + volumeSizeLimitMb: 1024, + byDiskUsage: true, + diskUsageHighWaterPercent: 90, + } + runBalance(t, c, []*Node{oldEmpty, newFull, newTarget}) + + if got := volCount(oldEmpty); got < beforeOldEmpty { + t.Fatalf("the nearly empty non-reporting server should not be drained, got %d volumes (was %d)", got, beforeOldEmpty) + } + if got := volCount(newFull); got >= beforeNewFull { + t.Fatalf("the data-heavy server should drain under the fallback ranking, got %d volumes (was %d)", got, beforeNewFull) + } +} + // makeByteNode builds a single-disk Node carrying physical disk bytes, for the // disk-fullness gate tests. func makeByteNode(id string, maxVolumeCount int64, totalBytes, freeBytes uint64, volumes []*master_pb.VolumeInformationMessage) *Node {