fix(ec): treat a missing teardown ack as fatal, not as an unreachable node

isNodeUnreachable returned true for any non-gRPC-status error, so a reachable
pre-upgrade server's missing full_teardown_done ack (a plain error) was
classified unreachable and the unreported pair was silently skipped. Classify
only a real codes.Unavailable as unreachable, and wrap the missing ack in a
sentinel the sweep treats as fatal regardless. A genuinely down node still
surfaces as Unavailable from the RPC and stays best-effort.
This commit is contained in:
Chris Lu
2026-06-09 10:52:52 -07:00
parent 5e818e9640
commit 92bdcc71c5
2 changed files with 16 additions and 7 deletions
+8 -1
View File
@@ -2,6 +2,7 @@ package shell
import (
"context"
"errors"
"fmt"
"regexp"
"slices"
@@ -536,6 +537,12 @@ func sourceServerDeleteEcShards(grpcDialOption grpc.DialOption, collection strin
}
// errFullTeardownNotAcked marks a reachable server that completed the delete RPC
// but did not report full_teardown_done (a pre-upgrade volume server). The orphan
// sweep must treat this as fatal: the node may still hold an orphan that a later
// copy would re-stamp into the new generation.
var errFullTeardownNotAcked = errors.New("delete did not perform full teardown (pre-upgrade volume server?); a stale EC generation may remain")
// unmountAndDeleteEcShardsQuiet unmounts then deletes shards on one server in a
// single connection, without the per-call logging the interactive helpers emit.
// Used by the orphan sweep, which fans out to every node x volume and would
@@ -559,7 +566,7 @@ func unmountAndDeleteEcShardsQuiet(grpcDialOption grpc.DialOption, collection st
return fmt.Errorf("delete: %w", err)
}
if !resp.GetFullTeardownDone() {
return fmt.Errorf("delete on %s did not perform full teardown (pre-upgrade volume server?); a stale EC generation may remain", location)
return fmt.Errorf("delete on %s: %w", location, errFullTeardownNotAcked)
}
return nil
})
+8 -6
View File
@@ -2,6 +2,7 @@ package shell
import (
"context"
"errors"
"flag"
"fmt"
"io"
@@ -410,7 +411,7 @@ func clearPreexistingEcShards(commandEnv *CommandEnv, topologyInfo *master_pb.To
// still hold an orphan a later copy would re-stamp into the new generation.
// Stay best-effort only for an unreachable node, which cannot receive this
// new generation at all.
if fatal || !isNodeUnreachable(err) {
if fatal || errors.Is(err, errFullTeardownNotAcked) || !isNodeUnreachable(err) {
return fmt.Errorf("clear stale ec shards for volume %d on %s: %w", vid, addr, err)
}
glog.V(1).Infof("orphan sweep: volume %d on %s skipped: %v", vid, addr, err)
@@ -424,15 +425,16 @@ func clearPreexistingEcShards(commandEnv *CommandEnv, topologyInfo *master_pb.To
// isNodeUnreachable reports whether err means the volume server could not be
// reached at all, as opposed to an RPC that reached the node and failed. Only an
// unreachable node is safe to skip in the orphan sweep.
// unreachable node is safe to skip in the orphan sweep. A dead peer surfaces as
// a gRPC codes.Unavailable from the RPC (the dial is lazy, so it never fails at
// connect time); any non-status error reached node logic and is treated as
// reachable, so the sweep stays fatal rather than silently leaving stale state.
func isNodeUnreachable(err error) bool {
if err == nil {
return false
}
if st, ok := status.FromError(err); ok {
return st.Code() == codes.Unavailable
}
return true
st, ok := status.FromError(err)
return ok && st.Code() == codes.Unavailable
}
func verifyEcShardsBeforeDelete(commandEnv *CommandEnv, volumeIds []needle.VolumeId, diskType types.DiskType) error {