fix(hop): skip failed nodes before priority short-circuit, last-resort fallback

Failed nodes (http.failCodes marker) were still selected when the
priority short-circuit fired or when FailFilter saw a single-node tier,
so failover never happened for a high-priority matcher node returning
429. Check IsFailed first in the candidate-pool scan, and when every
candidate is failed return the first node matching the request gates as
a best-effort last resort instead of nil.

Refactors the fail-code check into shared selector.IsFailed (cee264e)
wired through FailFilterSettingsOption (64ea518).
This commit is contained in:
ginuerzh
2026-08-18 23:42:08 +08:00
parent cee264e184
commit f8ca4908f3
2 changed files with 103 additions and 61 deletions
+59 -53
View File
@@ -35,9 +35,9 @@ type options struct {
bypass bypass.Bypass
selector selector.Selector[*chain.Node]
// failMaxFails / failTimeout mirror the FailFilter config used by the
// selector. The pre-selection dead-node filter (Stage 2.5) consults them
// to decide whether a node is currently marked failed (FailFilter itself
// no-ops on a single node, so the marker is checked directly here).
// selector. Node selection consults them via selector.IsFailed to skip
// nodes currently marked failed (FailFilter itself no-ops on a single
// node, so the marker is checked here instead).
failMaxFails int
failTimeout time.Duration
fileLoader loader.Loader
@@ -209,40 +209,26 @@ func (p *chainHop) Select(ctx context.Context, opts ...hop.SelectOption) *chain.
maxPriCount int
maxPriNode *chain.Node
hasBackup bool
sawFailed bool
)
for _, node := range p.Nodes() {
if node == nil {
continue
}
// node level bypass
if node.Options().Bypass != nil &&
node.Options().Bypass.Contains(ctx, options.Network, options.Addr, bypass.WithHostOption(options.Host)) {
// A node currently marked failed (http.failCodes match, probe failure)
// is never selected, regardless of its priority or matcher. Checked
// before the eligibility gates so failed nodes skip matcher evaluation
// (body matching reads a large request prefix) and never reach the
// priority short-circuit.
if xs.IsFailed(node, p.options.failMaxFails, p.options.failTimeout) {
sawFailed = true
continue
}
if !p.nodeMatches(ctx, node, &options) {
continue
}
if matcher := node.Options().Matcher; matcher != nil {
// Gate 2a: routing.Matcher — boolean expression on HTTP-level fields.
// A match assigns a non-zero Priority (from rule specificity).
req := routing.Request{
ClientIP: options.ClientIP,
Network: options.Network,
Host: options.Host,
Protocol: options.Protocol,
Method: options.Method,
Path: options.Path,
Query: options.Query,
Header: options.Header,
Body: options.Body,
}
if !matcher.Match(&req) {
continue
}
log.Debugf("node %s match request %s %s, priority %d", node.Name, req.Protocol, req.Host, node.Options().Priority)
} else {
// Gate 2b: fallback eligibility — simple host/protocol/path filters.
if !p.isEligible(node, &options) {
continue
}
log.Debugf("node %s match request %s %s, priority %d", node.Name, options.Protocol, options.Host, node.Options().Priority)
}
pri := node.Options().Priority
@@ -260,31 +246,23 @@ func (p *chainHop) Select(ctx context.Context, opts ...hop.SelectOption) *chain.
nodes = append(nodes, node)
}
// Stage 2.5: drop nodes currently marked failed (http.failCodes match,
// probe failure, etc.). A failed node must not win via the priority
// shortcut (Stage 3) or a lone-node tier, neither of which consults the
// marker — otherwise failCodes could never trigger failover. This mirrors
// FailFilter's per-node criteria; FailFilter still runs in Stage 4 but
// finds nothing left to exclude.
if len(nodes) > 0 {
alive := nodes[:0]
for _, n := range nodes {
if !xs.IsFailed(n, p.options.failMaxFails, p.options.failTimeout) {
alive = append(alive, n)
}
}
if len(alive) == 0 {
if len(nodes) == 1 {
alive = nodes // safety net: keep the only node (matches FailFilter's len(vs) <= 1 guard and the single-node early return below)
} else {
return nil
}
}
nodes = alive
}
if len(nodes) == 0 {
if sawFailed {
// All candidates are marked failed. Best-effort last resort: return
// the first candidate that matches this request, so a transient
// failure (e.g. a single 429) doesn't take the hop fully down — a
// successful dial in the caller resets the marker, so the node
// recovers as soon as it works again. Requests matching no node
// still yield nil (no route).
for _, node := range p.Nodes() {
if node != nil &&
xs.IsFailed(node, p.options.failMaxFails, p.options.failTimeout) &&
p.nodeMatches(ctx, node, &options) {
log.Debugf("last resort: node %s selected (all candidates failed)", node.Name)
return node
}
}
}
return nil
}
if len(nodes) == 1 {
@@ -382,6 +360,34 @@ func (p *chainHop) isEligible(node *chain.Node, opts *hop.SelectOptions) bool {
return true
}
// nodeMatches reports whether node passes the per-node bypass and
// matcher/eligible gates for the given selection options. Shared by the
// candidate-pool scan and the all-failed last-resort fallback.
func (p *chainHop) nodeMatches(ctx context.Context, node *chain.Node, opts *hop.SelectOptions) bool {
if node == nil {
return false
}
if node.Options().Bypass != nil &&
node.Options().Bypass.Contains(ctx, opts.Network, opts.Addr, bypass.WithHostOption(opts.Host)) {
return false
}
if matcher := node.Options().Matcher; matcher != nil {
req := routing.Request{
ClientIP: opts.ClientIP,
Network: opts.Network,
Host: opts.Host,
Protocol: opts.Protocol,
Method: opts.Method,
Path: opts.Path,
Query: opts.Query,
Header: opts.Header,
Body: opts.Body,
}
return matcher.Match(&req)
}
return p.isEligible(node, opts)
}
func (p *chainHop) checkHost(host string, node *chain.Node) bool {
var vhost string
if filter := node.Options().Filter; filter != nil {
+44 -8
View File
@@ -598,10 +598,11 @@ func TestSelect_FailCodesHighPriorityNode(t *testing.T) {
}
}
// TestSelect_SingleNodeKeptWhenFailed preserves the legacy safety net: a hop
// with a single node still returns it even when marked failed (FailFilter's
// len(vs) <= 1 guard / the single-node early return).
func TestSelect_SingleNodeKeptWhenFailed(t *testing.T) {
// TestSelect_SingleNodeFailedLastResort: with all candidates failed, Select
// falls back to the first matching node (best effort) instead of returning
// nil — a transient failure (e.g. one 429) must not take a lone-node hop
// fully down. A successful dial in the caller resets the marker.
func TestSelect_SingleNodeFailedLastResort(t *testing.T) {
only := chain.NewNode("only", "127.0.0.1:8080")
h := newTestHop(NodeOption(only))
defer h.Close()
@@ -609,11 +610,46 @@ func TestSelect_SingleNodeKeptWhenFailed(t *testing.T) {
only.Marker().Mark()
node := h.Select(context.Background())
if node == nil {
t.Fatal("expected the only node, got nil")
if node == nil || node.Name != "only" {
t.Fatalf("expected last-resort node 'only', got %v", node)
}
if node.Name != "only" {
t.Fatalf("expected 'only', got %q", node.Name)
}
// TestSelect_AllFailedLastResort: when every candidate is marked failed, the
// first matching node is still returned as a best-effort fallback.
func TestSelect_AllFailedLastResort(t *testing.T) {
n1 := chain.NewNode("n1", "127.0.0.1:8080")
n2 := chain.NewNode("n2", "127.0.0.1:9090")
h := newTestHop(NodeOption(n1, n2))
defer h.Close()
n1.Marker().Mark()
n2.Marker().Mark()
node := h.Select(context.Background())
if node == nil {
t.Fatal("expected a last-resort node, got nil")
}
if node.Name != "n1" && node.Name != "n2" {
t.Fatalf("expected n1 or n2, got %q", node.Name)
}
}
// TestSelect_AllFailedNoMatchReturnsNil: the last resort must still respect
// the matcher gates — a request matching no node yields nil even when nodes
// are marked failed (no route, not a fallback target).
func TestSelect_AllFailedNoMatchReturnsNil(t *testing.T) {
only := chain.NewNode("only", "127.0.0.1:8080",
chain.MatcherNodeOption(&testMatcher{match: false}),
)
h := newTestHop(NodeOption(only))
defer h.Close()
only.Marker().Mark()
node := h.Select(context.Background())
if node != nil {
t.Fatalf("expected nil (no node matches), got %q", node.Name)
}
}