mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-11-07 20:47:46 +01:00
ec: confirm a surviving copy before deleting a duplicate EC shard (#10719)
* ec: confirm a surviving copy before deleting a duplicate EC shard The dedup phase of EC balancing removes a shard it believes exists elsewhere. It copies nothing first, so the shard surviving on another node is the only thing that makes the delete safe -- and it took the plan's word for that. The plan is built from the master's topology, which can name a location that holds nothing: such a server answers "CopyFile not found ec volume id N" when something later tries to read the shard there. A shard listed on a phantom location and on a real one looks duplicated, so dedup deletes one of them. When it picks the real one the last copy is gone, and the job reports success -- the loss only surfaces later, as a rebuild that cannot assemble enough shards. The move phase already refuses to work on trust: it verifies the shard registered on the destination before removing the source. Dedup now holds to the same standard. The planner records which node it chose to keep, and both executors -- the worker task and the shell's ec.balance -- confirm that node really holds the shard before deleting. A keep node that cannot be queried is unknown rather than confirmed, and blocks the delete. Tests drive the destructive path against an in-process volume server that tracks what is actually on disk separately from what the plan claims, which is the distinction the bug turns on. Without the guard, two of them fail by deleting the only copy and returning success. * ec: check the collection and bound the wait when confirming a survivor Two gaps in the dedup survivor check. The inventory RPC is keyed by volume id alone, so a server holding the same number for a different collection answers "yes, I have that shard" to a question about this one. Accepting that deletes the last real copy on the strength of an unrelated volume. The response already carries the collection, so verify against it rather than widening the RPC. The shell path also queried on a background context, so a keep node that accepts the connection but never answers would hang the whole balance run instead of reporting that the survivor could not be confirmed. Bound it. The check moves into VerifyShardsOnServer next to the existing helper, shared by both executors, so the two paths cannot drift.
This commit is contained in:
1 parent
5b519489c1
commit
a7d5443125
9 files changed
+402
-8
No files matched your search
@@ -1089,6 +1089,28 @@ func (ecb *ecBalancer) executePhase(byID map[string]*EcNode, moves []ecbalancer.
|
||||
return ewg.Wait()
|
||||
}
|
||||
|
||||
// verifyEcShardOnKeepNode confirms the node a dedup move chose to keep actually
|
||||
// holds the shard, so a duplicate is only removed when a real copy remains. An
|
||||
// unreachable keep node is unknown, not confirmed, and blocks the delete — as
|
||||
// does one that answers too slowly to be waited on, which is why this is
|
||||
// bounded rather than left to hang the whole balance run.
|
||||
// ecShardVerifyTimeout bounds the keep-node inventory query. A node that accepts
|
||||
// the connection but never answers must not stall the whole balance run.
|
||||
const ecShardVerifyTimeout = 30 * time.Second
|
||||
|
||||
func verifyEcShardOnKeepNode(grpcDialOption grpc.DialOption, collection string, vid needle.VolumeId, keepNode string, shardId erasure_coding.ShardId) error {
|
||||
if keepNode == "" {
|
||||
return fmt.Errorf("refusing dedup delete of %d.%d: no keep node recorded", vid, shardId)
|
||||
}
|
||||
ctx, cancel := context.WithTimeout(context.Background(), ecShardVerifyTimeout)
|
||||
defer cancel()
|
||||
if err := erasure_coding.VerifyShardsOnServer(ctx, collection, uint32(vid), keepNode,
|
||||
[]uint32{uint32(shardId)}, grpcDialOption); err != nil {
|
||||
return fmt.Errorf("refusing dedup delete: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (ecb *ecBalancer) executeMove(byID map[string]*EcNode, m ecbalancer.Move) error {
|
||||
src := byID[m.SourceNode]
|
||||
if src == nil {
|
||||
@@ -1105,6 +1127,13 @@ func (ecb *ecBalancer) executeMove(byID map[string]*EcNode, m ecbalancer.Move) e
|
||||
return nil
|
||||
}
|
||||
grpcDialOption := ecb.commandEnv.option.GrpcDialOption
|
||||
// Nothing is copied first, so the shard surviving elsewhere is the only
|
||||
// thing making this safe -- and the plan saying so is not evidence. A
|
||||
// topology entry can name a location holding nothing, and deleting on
|
||||
// that basis removes the last copy. Confirm the keep node has it.
|
||||
if err := verifyEcShardOnKeepNode(grpcDialOption, m.Collection, vid, m.KeepNode, shardId); err != nil {
|
||||
return err
|
||||
}
|
||||
addr := pb.NewServerAddressFromDataNode(src.info)
|
||||
if err := unmountEcShards(grpcDialOption, vid, addr, shardIds); err != nil {
|
||||
return err
|
||||
|
||||
Reference in new issue
Block a user