From 3e55971e0e38b319703fca15dedc65aaaa442865 Mon Sep 17 00:00:00 2001
From: Chris Lu <chrislusf@users.noreply.github.com>
Date: Fri, 16 Oct 2026 23:45:10 -0700
Subject: [PATCH] ec.shard.unmount: treat same-node shard copies as one target
 (#11778)

A shard held on two disks of one server produced two identical live
ShardRefs, so resolution reported "ambiguous" even for a fully
node-qualified -shardId. Ambiguity now means the shard lives on more
than one node; duplicate refs on the same node resolve to it.
---
 weed/ec/ec_shard_unmount.go      |  5 +-
 weed/ec/ec_shard_unmount_test.go | 81 ++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+), 1 deletion(-)

diff --git a/weed/ec/ec_shard_unmount.go b/weed/ec/ec_shard_unmount.go
index e4cfee94b..791762a55 100644
--- a/weed/ec/ec_shard_unmount.go
+++ b/weed/ec/ec_shard_unmount.go
@@ -135,7 +135,10 @@ func (c *ecShardUnmounter) doShardsUnmount() error {
 		for _, ts := range liveShards {
 			if ts.ShardID == ps.ShardID {
 				if ps.NodeAddress == "" || ps.NodeAddress == ts.NodeAddress {
-					if result != nil {
+					// One node is one unmount target no matter how many of
+					// its disks hold the shard; ambiguity means the shard
+					// lives on more than one node.
+					if result != nil && result.NodeAddress != ts.NodeAddress {
 						return fmt.Errorf("shard %v is ambiguous", ps)
 					}
 					result = ts
diff --git a/weed/ec/ec_shard_unmount_test.go b/weed/ec/ec_shard_unmount_test.go
index 82b11fe58..a35e5cb1c 100644
--- a/weed/ec/ec_shard_unmount_test.go
+++ b/weed/ec/ec_shard_unmount_test.go
@@ -1,8 +1,10 @@
 package ec
 
 import (
+	"bytes"
 	"testing"
 
+	"github.com/seaweedfs/seaweedfs/weed/pb/master_pb"
 	"github.com/seaweedfs/seaweedfs/weed/storage/erasure_coding"
 	"github.com/stretchr/testify/assert"
 )
@@ -100,3 +102,82 @@ func TestEcShardStringRoundTrips(t *testing.T) {
 		assert.Equal(t, []*ShardRef{s}, got)
 	}
 }
+
+func unmountTestTopology(nodes map[string]map[string][]uint32) *master_pb.TopologyInfo {
+	var dataNodes []*master_pb.DataNodeInfo
+	for addr, disks := range nodes {
+		diskInfos := map[string]*master_pb.DiskInfo{}
+		for disk, shardIds := range disks {
+			si := erasure_coding.NewShardsInfo()
+			for _, sid := range shardIds {
+				si.Set(erasure_coding.NewShardInfo(erasure_coding.ShardId(sid), 1))
+			}
+			diskInfos[disk] = &master_pb.DiskInfo{
+				Type:           "hdd",
+				MaxVolumeCount: 100,
+				EcShardInfos: []*master_pb.VolumeEcShardInformationMessage{{
+					Id:          100,
+					Collection:  "c1",
+					EcIndexBits: si.Bitmap(),
+					ShardSizes:  si.SizesInt64(),
+					DiskType:    "hdd",
+				}},
+			}
+		}
+		dataNodes = append(dataNodes, &master_pb.DataNodeInfo{
+			Id:        addr,
+			Address:   addr,
+			DiskInfos: diskInfos,
+		})
+	}
+	return &master_pb.TopologyInfo{DataCenterInfos: []*master_pb.DataCenterInfo{{
+		Id:        "dc1",
+		RackInfos: []*master_pb.RackInfo{{Id: "r1", DataNodeInfos: dataNodes}},
+	}}}
+}
+
+// A shard held on two disks of one node is a single unmount target: the
+// unmount RPC addresses the node, not the disk.
+func TestUnmountResolvesShardDuplicatedOnOneNode(t *testing.T) {
+	topo := unmountTestTopology(map[string]map[string][]uint32{
+		"vs3:8080": {"hdd:0": {12}, "hdd:1": {12}},
+		"vs4:8080": {"hdd:0": {13}},
+	})
+
+	for _, shards := range [][]*ShardRef{
+		{{ShardID: 12}},
+		{{ShardID: 12, NodeAddress: "vs3:8080"}},
+	} {
+		var out bytes.Buffer
+		err := UnmountShards(&Env{}, &out, ShardUnmountRequest{
+			Topology: topo,
+			VolumeID: 100,
+			Shards:   shards,
+		})
+		assert.NoError(t, err, "shard %+v", shards)
+		assert.Contains(t, out.String(), "Will unmount 1 shard(s)")
+	}
+}
+
+func TestUnmountStillRejectsShardOnTwoNodes(t *testing.T) {
+	topo := unmountTestTopology(map[string]map[string][]uint32{
+		"vs3:8080": {"hdd:0": {12}},
+		"vs4:8080": {"hdd:0": {12}},
+	})
+
+	var out bytes.Buffer
+	err := UnmountShards(&Env{}, &out, ShardUnmountRequest{
+		Topology: topo,
+		VolumeID: 100,
+		Shards:   []*ShardRef{{ShardID: 12}},
+	})
+	assert.ErrorContains(t, err, "ambiguous")
+
+	out.Reset()
+	err = UnmountShards(&Env{}, &out, ShardUnmountRequest{
+		Topology: topo,
+		VolumeID: 100,
+		Shards:   []*ShardRef{{ShardID: 12, NodeAddress: "vs3:8080"}},
+	})
+	assert.NoError(t, err)
+}