fix(topology): keep physical disk 0 distinct in SplitByPhysicalDisk (#10161)

* fix(topology): keep physical disk 0 distinct in SplitByPhysicalDisk

DiskId 0 doubles as the first physical disk (Locations[0]) and the
protobuf "unset" default. SplitByPhysicalDisk folded every DiskId-0
record onto the aggregate DiskId whenever that was non-zero, so on a
multi-disk node the first disk's volumes merged into whichever disk
held volumes[0]: the node reported one fewer disk, the sibling showed
~2x volumes, and per-disk max was smeared across the survivors. This
surfaced as cluster.status and volume.list undercounting disks.

Only treat 0 as unset when no record carries a non-zero DiskId; with a
mix, 0 is a real disk and keeps its own entry.

* fix(admin): resolve physical disk 0 in active-topology indexes

rebuildIndexes re-derived each volume/EC record's physical disk id with
the same "DiskId 0 means unset" heuristic SplitByPhysicalDisk used, so
the two agreed only by sharing the bug. Now that SplitByPhysicalDisk
keeps disk 0 distinct, the duplicated heuristic would fold disk-0 records
onto a sibling while at.disks kept them on disk 0; GetVolumeLocations and
GetECShardLocations then matched no record and silently dropped every
volume and EC shard on the first disk, starving balance and EC tasks.

Build the indexes from the same SplitByPhysicalDisk reconstruction that
builds at.disks, so the keys always resolve. One source of truth instead
of a parallel normalize.

* fix(ec): allow physical disk 0 as preferred EC shard target

pickBestDiskOnNode gated its result on bestDiskId != 0, but 0 is both a
valid physical disk and the uint32 zero value, so a best-scoring disk 0
was discarded and the non-matching fallback returned instead. Gate on
bestScore.

* test(admin): cover EC-shard index resolution for physical disk 0

rebuildIndexes builds ecShardIndex the same way as volumeIndex; pin the EC
path too so a shard on disk 0 keeps resolving via GetECShardLocations.

weed/admin/topology/active_topology_test.go

@@ -459,6 +459,83 @@ func TestECPlannerSeesEachPhysicalDisk(t *testing.T) {
 	}
 }
 
+// TestVolumeIndexResolvesPhysicalDiskZero pins that a volume genuinely on
+// physical disk 0 resolves to its own disk via GetVolumeLocations even when the
+// aggregate DiskInfo.DiskId is seeded from a non-zero sibling (as ToDiskInfo
+// does from volumes[0]). Re-deriving the per-record disk id would fold disk 0
+// onto the sibling, then the lookup would find no matching volume and drop it.
+func TestVolumeIndexResolvesPhysicalDiskZero(t *testing.T) {
+	topology := NewActiveTopology(10)
+
+	require.NoError(t, topology.UpdateTopology(&master_pb.TopologyInfo{
+		DataCenterInfos: []*master_pb.DataCenterInfo{{
+			Id: "dc1",
+			RackInfos: []*master_pb.RackInfo{{
+				Id: "rack1",
+				DataNodeInfos: []*master_pb.DataNodeInfo{{
+					Id: "127.0.0.1:8080",
+					DiskInfos: map[string]*master_pb.DiskInfo{
+						"hdd": {
+							DiskId:         6, // seeded from volumes[0].DiskId
+							MaxVolumeCount: 200,
+							VolumeInfos: []*master_pb.VolumeInformationMessage{
+								{Id: 11, DiskId: 6, DiskType: "hdd"},
+								{Id: 10, DiskId: 0, DiskType: "hdd"},
+							},
+						},
+					},
+				}},
+			}},
+		}},
+	}))
+
+	loc10 := topology.GetVolumeLocations(10, "")
+	require.Len(t, loc10, 1, "volume on physical disk 0 must resolve to one disk")
+	assert.Equal(t, uint32(0), loc10[0].DiskID, "volume 10 lives on physical disk 0")
+
+	loc11 := topology.GetVolumeLocations(11, "")
+	require.Len(t, loc11, 1)
+	assert.Equal(t, uint32(6), loc11[0].DiskID, "volume 11 lives on physical disk 6")
+}
+
+// TestECShardIndexResolvesPhysicalDiskZero is the EC-shard twin of
+// TestVolumeIndexResolvesPhysicalDiskZero: rebuildIndexes builds ecShardIndex
+// the same way, so an EC shard on physical disk 0 must resolve via
+// GetECShardLocations rather than being folded onto a non-zero sibling.
+func TestECShardIndexResolvesPhysicalDiskZero(t *testing.T) {
+	topology := NewActiveTopology(10)
+
+	require.NoError(t, topology.UpdateTopology(&master_pb.TopologyInfo{
+		DataCenterInfos: []*master_pb.DataCenterInfo{{
+			Id: "dc1",
+			RackInfos: []*master_pb.RackInfo{{
+				Id: "rack1",
+				DataNodeInfos: []*master_pb.DataNodeInfo{{
+					Id: "127.0.0.1:8080",
+					DiskInfos: map[string]*master_pb.DiskInfo{
+						"hdd": {
+							DiskId:         6, // seeded from a non-zero sibling
+							MaxVolumeCount: 200,
+							EcShardInfos: []*master_pb.VolumeEcShardInformationMessage{
+								{Id: 21, DiskId: 6, Collection: "c1", EcIndexBits: 0b1},
+								{Id: 20, DiskId: 0, Collection: "c1", EcIndexBits: 0b1},
+							},
+						},
+					},
+				}},
+			}},
+		}},
+	}))
+
+	loc20 := topology.GetECShardLocations(20, "c1")
+	require.Len(t, loc20, 1, "EC shard on physical disk 0 must resolve to one disk")
+	assert.Equal(t, uint32(0), loc20[0].DiskID, "shard 20 lives on physical disk 0")
+
+	loc21 := topology.GetECShardLocations(21, "c1")
+	require.Len(t, loc21, 1)
+	assert.Equal(t, uint32(6), loc21[0].DiskID, "shard 21 lives on physical disk 6")
+}
+
 // TestPublicInterfaces tests the public interface methods
 func TestPublicInterfaces(t *testing.T) {
 	topology := NewActiveTopology(10)

weed/admin/topology/topology_management.go

@@ -216,27 +216,22 @@ func (at *ActiveTopology) rebuildIndexes() {
 	at.volumeIndex = make(map[uint32][]string)
 	at.ecShardIndex = make(map[uint32][]string)
 
-	// Index by the per-record DiskId (not the outer DiskInfo.DiskId) so the
-	// keys match the per-physical-disk activeDisk entries — see #9369.
+	// Derive index keys from the same SplitByPhysicalDisk reconstruction that
+	// builds at.disks, so every volume/EC lookup resolves to a real activeDisk
+	// entry. Re-deriving the per-record disk id here would diverge on physical
+	// disk 0, which the raw DiskId cannot tell apart from the "unset" default.
 	for _, dc := range at.topologyInfo.DataCenterInfos {
 		for _, rack := range dc.RackInfos {
 			for _, nodeInfo := range rack.DataNodeInfos {
 				for _, diskInfo := range nodeInfo.DiskInfos {
-					for _, volumeInfo := range diskInfo.VolumeInfos {
-						diskID := volumeInfo.DiskId
-						if diskID == 0 && diskInfo.DiskId != 0 {
-							diskID = diskInfo.DiskId
+					for _, perDisk := range diskInfo.SplitByPhysicalDisk() {
+						diskKey := fmt.Sprintf("%s:%d", nodeInfo.Id, perDisk.DiskId)
+						for _, volumeInfo := range perDisk.VolumeInfos {
+							at.volumeIndex[volumeInfo.Id] = append(at.volumeIndex[volumeInfo.Id], diskKey)
 						}
-						diskKey := fmt.Sprintf("%s:%d", nodeInfo.Id, diskID)
-						at.volumeIndex[volumeInfo.Id] = append(at.volumeIndex[volumeInfo.Id], diskKey)
-					}
-					for _, ecShardInfo := range diskInfo.EcShardInfos {
-						diskID := ecShardInfo.DiskId
-						if diskID == 0 && diskInfo.DiskId != 0 {
-							diskID = diskInfo.DiskId
+						for _, ecShardInfo := range perDisk.EcShardInfos {
+							at.ecShardIndex[ecShardInfo.Id] = append(at.ecShardIndex[ecShardInfo.Id], diskKey)
 						}
-						diskKey := fmt.Sprintf("%s:%d", nodeInfo.Id, diskID)
-						at.ecShardIndex[ecShardInfo.Id] = append(at.ecShardIndex[ecShardInfo.Id], diskKey)
 					}
 				}
 			}

weed/pb/master_pb/master_helper.go

@@ -24,8 +24,30 @@ func (d *DiskInfo) SplitByPhysicalDisk() []*DiskInfo {
 		return nil
 	}
 
+	// DiskId 0 is overloaded: it is both the first physical disk (Locations[0])
+	// and the protobuf default for "unset". Only treat 0 as unset when every
+	// record reports 0 (the legacy case where the volume server didn't populate
+	// DiskId). If any record carries a non-zero DiskId, the reporting is real and
+	// a 0 means physical disk 0 — keep it distinct instead of folding it onto
+	// d.DiskId, which would merge two physical disks and drop disk 0 from view.
+	hasNonZero := false
+	for _, vi := range d.VolumeInfos {
+		if vi.DiskId != 0 {
+			hasNonZero = true
+			break
+		}
+	}
+	if !hasNonZero {
+		for _, eci := range d.EcShardInfos {
+			if eci.DiskId != 0 {
+				hasNonZero = true
+				break
+			}
+		}
+	}
+
 	normalize := func(id uint32) uint32 {
-		if id == 0 && d.DiskId != 0 {
+		if id == 0 && !hasNonZero && d.DiskId != 0 {
 			return d.DiskId
 		}
 		return id

weed/pb/master_pb/master_helper_test.go

@@ -203,6 +203,56 @@ func TestDiskInfoSplitByPhysicalDisk_normalizesZeroToOuterDiskId(t *testing.T) {
 	}
 }
 
+// TestDiskInfoSplitByPhysicalDisk_keepsRealDiskZeroWhenMixed reproduces a
+// multi-disk node where physical disk 0 (Locations[0]) holds volumes and the
+// aggregate DiskId is seeded from a non-zero sibling (volumes[0] landed on
+// disk 6). DiskId 0 must stay its own physical disk, not get folded onto disk
+// 6 — folding drops a disk from the count and doubles the sibling's volumes.
+func TestDiskInfoSplitByPhysicalDisk_keepsRealDiskZeroWhenMixed(t *testing.T) {
+	d := &DiskInfo{
+		Type:           "hdd",
+		MaxVolumeCount: 30,
+		DiskId:         6, // seeded from volumes[0].DiskId in ToDiskInfo
+		VolumeInfos: []*VolumeInformationMessage{
+			{Id: 60, DiskId: 6},
+			{Id: 61, DiskId: 6},
+			{Id: 1, DiskId: 0}, // real disk 0, must not be remapped to 6
+			{Id: 2, DiskId: 0},
+			{Id: 30, DiskId: 3},
+		},
+	}
+
+	got := d.SplitByPhysicalDisk()
+	byID := map[uint32]*DiskInfo{}
+	for _, di := range got {
+		byID[di.DiskId] = di
+	}
+
+	if len(got) != 3 {
+		t.Fatalf("want 3 physical disks (0, 3, 6), got %d: %v", len(got), byID)
+	}
+	if _, ok := byID[0]; !ok {
+		t.Fatalf("physical disk 0 was dropped; got disks %v", byID)
+	}
+	if byID[0].VolumeCount != 2 {
+		t.Errorf("disk 0: want 2 volumes, got %d", byID[0].VolumeCount)
+	}
+	if byID[6].VolumeCount != 2 {
+		t.Errorf("disk 6: want 2 volumes (not merged with disk 0), got %d", byID[6].VolumeCount)
+	}
+	if byID[3].VolumeCount != 1 {
+		t.Errorf("disk 3: want 1 volume, got %d", byID[3].VolumeCount)
+	}
+
+	var sumMax int64
+	for _, di := range got {
+		sumMax += di.MaxVolumeCount
+	}
+	if sumMax != 30 {
+		t.Errorf("sum of MaxVolumeCount = %d, want 30 (lossless split)", sumMax)
+	}
+}
+
 func TestDiskInfoSplitByPhysicalDisk_nilSafe(t *testing.T) {
 	var d *DiskInfo
 	if got := d.SplitByPhysicalDisk(); got != nil {

weed/shell/command_ec_common.go

@@ -820,8 +820,10 @@ func pickBestDiskOnNode(ecNode *EcNode, vid needle.VolumeId, diskType types.Disk
 		}
 	}
 
-	// Return matching disk type if found, otherwise fallback
-	if bestDiskId != 0 {
+	// Return matching disk type if found, otherwise fallback. Gate on bestScore,
+	// not bestDiskId: physical disk 0 is a valid target and 0 is also the "no
+	// match" zero value, so testing bestDiskId would never select disk 0.
+	if bestScore != -1 {
 		return bestDiskId
 	}
 	return fallbackDiskId

weed/shell/command_ec_test.go

@@ -9,6 +9,24 @@ import (
 	"github.com/seaweedfs/seaweedfs/weed/storage/types"
 )
 
+// TestPickBestDiskOnNodeSelectsPhysicalDiskZero verifies physical disk 0 can be
+// chosen as the preferred shard target. Gating on bestDiskId != 0 discarded a
+// best-scoring disk 0 (0 is also the zero value) and returned the fallback.
+func TestPickBestDiskOnNodeSelectsPhysicalDiskZero(t *testing.T) {
+	ecNode := &EcNode{
+		disks: map[uint32]*EcDisk{
+			0: {diskId: 0, diskType: string(types.SsdType), freeEcSlots: 10, ecShards: map[needle.VolumeId]*erasure_coding.ShardsInfo{}},
+			5: {diskId: 5, diskType: string(types.HardDriveType), freeEcSlots: 10, ecShards: map[needle.VolumeId]*erasure_coding.ShardsInfo{}},
+		},
+	}
+
+	// Disk 0 matches the requested type; it must win over the non-matching
+	// fallback on disk 5 instead of being treated as "no match".
+	if got := pickBestDiskOnNode(ecNode, needle.VolumeId(42), types.SsdType, false, 0, 0); got != 0 {
+		t.Fatalf("want physical disk 0 (matching type), got %d", got)
+	}
+}
+
 func TestCommandEcBalanceSmall(t *testing.T) {
 	ecb := &ecBalancer{
 		ecNodes: []*EcNode{