From c2183566b6d7648e9a5fa2ea6f3d1890ae912bb2 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Wed, 29 Jul 2026 14:21:55 -0700 Subject: [PATCH] ec.encode: name the shard ids an aborted deletion found (#10486) Counting by node lost the per-node ShardsInfo, and with it the shard ids the old summary printed -- the message an operator gets when the pre-delete check refuses now says only how many shards each node holds. That is the wrong half. A set holding shards 0-9 and one holding 4-13 are both "10 shards", and only the ids say whether what survived can rebuild the volume, or which node to go looking at. Keep the count and list the ids beside it. --- weed/shell/command_ec_encode.go | 21 +++++++++++++++------ weed/shell/command_ec_encode_test.go | 16 ++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/weed/shell/command_ec_encode.go b/weed/shell/command_ec_encode.go index 7ce36ee4e..c9dcce0ea 100644 --- a/weed/shell/command_ec_encode.go +++ b/weed/shell/command_ec_encode.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "regexp" + "slices" "sort" "strconv" "strings" @@ -801,6 +802,19 @@ func ecShardsClumpedOnOneNode(topoInfo *master_pb.TopologyInfo, vid needle.Volum return "", false } +// ecShardSummaryByNode says where a volume's shards are, one entry per node, +// sorted so the message is stable. It names the ids and not just the count: a +// set holding shards 0-9 and one holding 4-13 are both "10 shards", and which +// ones survived is what says whether the set is recoverable and from where. +func ecShardSummaryByNode(byNode map[pb.ServerAddress]erasure_coding.ShardBits) []string { + summary := make([]string, 0, len(byNode)) + for node, bits := range byNode { + summary = append(summary, fmt.Sprintf("%s=%d shards %v", node, bits.Count(), slices.Collect(bits.All()))) + } + sort.Strings(summary) + return summary +} + func verifyEcShardsBeforeDelete(commandEnv *CommandEnv, volumeIds []needle.VolumeId, diskType types.DiskType, expectSpread bool) error { // Shard relocations from the preceding EC balance reach the master via // volume-server heartbeats, so freshly distributed shards may not all be @@ -847,12 +861,7 @@ func verifyEcShardsBeforeDelete(commandEnv *CommandEnv, volumeIds []needle.Volum totalShards := erasure_coding.TotalShardsCount degraded, err := erasure_coding.RequireRecoverableShardSet(uint32(vid), union, erasure_coding.DataShardsCount, totalShards) if err != nil { - summary := make([]string, 0, len(byNode)) - for node, bits := range byNode { - summary = append(summary, fmt.Sprintf("%s=%d shards", node, bits.Count())) - } - sort.Strings(summary) - lastErr = fmt.Errorf("volume %d: %w (observed: %v)", vid, err, summary) + lastErr = fmt.Errorf("volume %d: %w (observed: %v)", vid, err, ecShardSummaryByNode(byNode)) break } if expectSpread { diff --git a/weed/shell/command_ec_encode_test.go b/weed/shell/command_ec_encode_test.go index 7255c0eeb..ad8abe9e6 100644 --- a/weed/shell/command_ec_encode_test.go +++ b/weed/shell/command_ec_encode_test.go @@ -466,3 +466,19 @@ func TestEcShardCountIgnoresDiskTypeOfTheShards(t *testing.T) { scoped, _ := collectEcNodeShardsInfo(topo, needle.VolumeId(1), types.ToDiskType("")) assert.Empty(t, scoped, "the hdd-scoped view cannot see ssd shards") } + +// The message an aborted deletion leaves behind is all the operator has to go +// on, so it has to name which shards were found and not only how many: a set +// holding 0-9 and one holding 4-13 are both "10 shards", and only the ids say +// whether what survived can rebuild the volume. +func TestEcShardSummaryNamesTheShardIds(t *testing.T) { + byNode := map[pb.ServerAddress]erasure_coding.ShardBits{ + "node2:8080": erasure_coding.ShardBits(0).Set(1).Set(2), + "node1:8080": erasure_coding.ShardBits(0).Set(0).Set(12), + } + + assert.Equal(t, []string{ + "node1:8080=2 shards [0 12]", + "node2:8080=2 shards [1 2]", + }, ecShardSummaryByNode(byNode)) +}