mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-07-29 19:43:19 +00:00
Chris LuandGitHub
d221a64262
* fix(ec): skip re-encode when EC shards already exist for the volume (#9448) When an earlier EC encoding succeeded but the post-encode source-delete left a regular replica behind on one of the servers, the next detection cycle proposes the same volume again. The new encode tries to redistribute shards to targets that already have them mounted, the volume server returns `ec volume %d is mounted; refusing overwrite`, the task fails, and detection re-queues the volume. The cycle repeats forever — issue #9448. The existing `metric.IsECVolume` skip catches the case where the canonical metric is reported on the EC-shard side of the heartbeat, but when the master sees BOTH a regular replica AND its EC shards in the same volume list, the canonical metric we pick is the regular replica and IsECVolume is false. Add a second guard that checks the topology directly via `findExistingECShards` (already present and indexed) and skip the volume when any shards exist, logging a warning that points the admin at the stuck source. This breaks the loop. Auto-cleanup of the orphaned replica is left as follow-up work — deleting a source replica from inside the detector is only safe with a re-verification step right before the delete, plus a config opt-in, and is best done in its own change. * fix(ec): #9448 guard only fires when EC shard set is complete The first version of the #9448 guard tripped on `len(existingShards) > 0`, which is broader than necessary. The existing recovery branch in the encode arm (around the `existingECShards` block, ~line 216) is designed to fold partial leftover shards from a previously failed encode into the new task as cleanup sources. Skipping unconditionally on any existing shards made that branch dead code, regressing the recovery behavior Gemini flagged in the review ofaf09e1ec7. Two corrections: 1. New helper `countExistingEcShardsForVolume` walks each disk's `EcIndexBits` bitmap and ORs the results into a `ShardBits`, returning the distinct-shard popcount. This is the right unit: a single `VolumeEcShardInformationMessage` can carry several shards, so `len(EcShardInfos)` is not the same as the number of present shards. Per Gemini's "use helper functions that walk the actual shard bitmap" note. 2. The guard now fires only when `shardCount >= totalShards`. Partial shard sets fall through to the existing recovery branch, unchanged. Tests: - TestDetectionSkipsWhenECShardsAlreadyExist: complete shards → no proposal (the regression test for #9448 itself, unchanged intent, rewritten on top of new helpers). - TestDetectionAllowsRegularReplicaWhenShardsPartial: partial shards → guard does NOT swallow the volume; the encode arm still gets a chance. - TestCountExistingEcShardsForVolume: the helper walks the bitmap correctly even when one info entry packs multiple shards on one disk. The dangerous `volume.delete` hint in the warning is unchanged for now — it gets fixed in the next commit. * fix(ec): drop dangerous shell-command hint from #9448 warning The previous warning told operators to run `volume.delete -volumeId=%d` in the SeaweedFS shell to clean up the orphaned source replica. That command is cluster-wide — it deletes every replica of the volume, including the EC shards, which share the same volume id. Running it in the state the message describes would cause the data loss the guard exists to prevent. Replace it with explicit guidance that the cleanup must be a targeted VolumeDelete RPC against the source server only, and that the shell command is the exact wrong thing to use here. The next two commits add the plumbing and the auto-execution of that targeted delete so most operators never see this hint at all. Per Gemini comment onaf09e1ec7. * feat(worker): plumb grpc dial option through ClusterInfo Add ClusterInfo.GrpcDialOption (optional) and set it in the erasure_coding plugin handler. Lets the detector make targeted gRPC calls during detection — used by the follow-up commit to auto-clean orphan source replicas via VolumeDelete RPCs. Zero-value safe: existing detectors that don't need RPC access get a nil DialOption and ignore the field. * feat(ec): auto-clean orphan source replica via targeted VolumeDelete Builds on the previous commits: the guard now identifies the #9448 stuck-source state and a gRPC dial option is available on ClusterInfo. When both are true, detection auto-cleans the orphaned regular replica instead of just warning the operator. New helper `cleanupOrphanSourceReplicas`: 1. Re-verifies the EC shard set is still complete via `countExistingEcShardsForVolume` against the live topology snapshot. If the count dropped between detection start and the cleanup decision (a volume server going down mid-cycle), it aborts — the source replica is the only complete copy and deleting it without a healthy shard set would be data loss. 2. Issues targeted VolumeDelete RPCs to each regular-replica server via `operation.WithVolumeServerClient`. That RPC only touches the regular volume on the targeted server; EC shards live in a separate store path and are not affected. This is the safe alternative to the cluster-wide `volume.delete` shell command we previously warned against. If the cleanup partially fails (one replica delete errors, others succeed), detection logs the failure and continues to skip the volume. The next detection cycle will try again. We deliberately don't fall back to a re-encode because that would just collide with the mounted shards on the targets again. When no dial option is available the existing warning still points operators at the safe manual procedure.