fix(ec placement): treat empty disk type as hdd and skip used racks on spill (#9423)

partitionByDiskType used raw string comparison, so a PreferredDiskType
of "hdd" never matched candidates whose DiskType is "" (the
HardDriveType sentinel that weed/storage/types uses). EC encoding of
an HDD source would spill onto any HDD reporting "" even when the
cluster has plenty of matching capacity. Normalize both sides
through normalizeDiskType, which lowercases and folds "" → "hdd",
mirroring types.ToDiskType without taking a dependency on it.

selectFromTier's rack-diversity pass also kept revisiting racks the
preferred tier had already used when running on the fallback tier,
which negated PreferDifferentRacks on spillover. Skip racks already
in usedRacks so fallback placements still spread onto new racks.
This commit is contained in:
Chris Lu 2026-05-11 18:16:32 -07:00
parent fabc141148
commit 57f211b398
2 changed files with 56 additions and 2 deletions

View File

@ -10,6 +10,7 @@ package placement
import (
"fmt"
"sort"
"strings"
)
// DiskCandidate represents a disk that can receive EC shards
@ -160,12 +161,19 @@ func SelectDestinations(disks []*DiskCandidate, config PlacementRequest) (*Place
// partitionByDiskType splits disks into (matching, fallback) based on the
// preferred disk type. If preferred is empty, everything goes into the
// matching tier and fallback is empty — i.e. existing single-pool behavior.
//
// Empty DiskCandidate.DiskType is treated as HardDriveType ("hdd") to
// mirror weed/storage/types.ToDiskType's normalization, so a
// PreferredDiskType of "hdd" matches disks reporting "" — otherwise EC
// shards from an HDD source would always spill onto disks that happen to
// report their type as "" (HardDriveType).
func partitionByDiskType(disks []*DiskCandidate, preferred string) (matching, fallback []*DiskCandidate) {
if preferred == "" {
return disks, nil
}
pref := normalizeDiskType(preferred)
for _, d := range disks {
if d.DiskType == preferred {
if normalizeDiskType(d.DiskType) == pref {
matching = append(matching, d)
} else {
fallback = append(fallback, d)
@ -174,6 +182,16 @@ func partitionByDiskType(disks []*DiskCandidate, preferred string) (matching, fa
return matching, fallback
}
// normalizeDiskType lower-cases the input and folds "" to "hdd" so the
// HardDriveType sentinel ("") and explicit "hdd"/"HDD" all compare equal.
func normalizeDiskType(t string) string {
t = strings.ToLower(t)
if t == "" {
return "hdd"
}
return t
}
// selectFromTier runs the three diversity passes against `tier`, mutating
// `result` and the used* maps in place. Passes stop as soon as ShardsNeeded
// is reached. The function is a no-op when the tier is empty or the result
@ -188,7 +206,10 @@ func selectFromTier(tier []*DiskCandidate, result *PlacementResult,
rackToDisks := groupDisksByRack(tier)
// Pass 1: Select one disk from each rack (maximize rack diversity)
// Pass 1: Select one disk from each rack (maximize rack diversity).
// When this is the fallback tier (preferred tier already populated
// usedRacks), skip those racks so the spillover still spreads onto
// new racks instead of doubling up on ones already picked.
if config.PreferDifferentRacks {
// Sort racks by number of available servers (descending) to prioritize racks with more options
sortedRacks := sortRacksByServerCount(rackToDisks)
@ -196,6 +217,9 @@ func selectFromTier(tier []*DiskCandidate, result *PlacementResult,
if len(result.SelectedDisks) >= config.ShardsNeeded {
break
}
if usedRacks[rackKey] {
continue
}
rackDisks := rackToDisks[rackKey]
// Select best disk from this rack, preferring a new server
disk := selectBestDiskFromRack(rackDisks, usedServers, usedDisks, config)

View File

@ -90,6 +90,36 @@ func TestSelectDestinations_SpillsWhenPreferredScarce(t *testing.T) {
}
}
// PreferredDiskType="hdd" must match disks whose DiskType is "" (the
// HardDriveType sentinel) — otherwise EC encoding of an HDD source would
// always spill onto HDDs that happen to report disk_type="" even though
// the cluster has plenty of matching capacity.
func TestSelectDestinations_PreferredHddMatchesEmptyDiskType(t *testing.T) {
disks := []*DiskCandidate{
makeDisk("hdd-0", "r0", "", 0), // HardDriveType sentinel
makeDisk("hdd-1", "r1", "", 0), // HardDriveType sentinel
makeDisk("ssd-0", "r2", "ssd", 0),
}
result, err := SelectDestinations(disks, newRequest(2, "hdd"))
if err != nil {
t.Fatalf("SelectDestinations: %v", err)
}
if got := len(result.SelectedDisks); got != 2 {
t.Fatalf("selected %d disks, want 2", got)
}
// Both selected disks must be HDD-reporting (i.e. DiskType == ""),
// and no spillover should have been required.
for _, d := range result.SelectedDisks {
if d.DiskType != "" {
t.Errorf("selected disk %s has DiskType=%q, want \"\" (HardDriveType)", d.NodeID, d.DiskType)
}
}
if result.SpilledToOtherDiskType {
t.Fatalf("SpilledToOtherDiskType should be false when HDD pool matches preferred=hdd")
}
}
// Empty PreferredDiskType: pre-#9423 behavior, single pool, no spillover
// flag regardless of disk-type mix.
func TestSelectDestinations_EmptyPreferredDiskTypeKeepsPriorBehavior(t *testing.T) {