fix(hop): drop marked-failed nodes before priority selection

http.failCodes (and probe) marking had no effect when the failing node
had a strictly higher matcher priority: the priority short-circuit
returned the top node directly, and FailFilter no-ops on a single-node
tier (len(vs) <= 1 guard), so the marked node was always re-selected
and failover never happened.

Pre-filter nodes marked failed (same Count >= maxFails && within
failTimeout criteria as FailFilter) out of the candidate pool before
priority selection. Single-node hops keep their lone node (safety net),
and an all-failed pool returns nil as before.

Wired maxFails/failTimeout from the hop selector config into the hop
via FailFilterSettingsOption.
This commit is contained in:
ginuerzh
2026-08-18 23:15:59 +08:00
parent fc5fc052e4
commit 64ea5184f8
3 changed files with 140 additions and 0 deletions
+12
View File
@@ -15,6 +15,7 @@ import (
selector_parser "github.com/go-gost/x/config/parsing/selector"
xhop "github.com/go-gost/x/hop"
hop_plugin "github.com/go-gost/x/hop/plugin"
xs "github.com/go-gost/x/selector"
"github.com/go-gost/x/internal/loader"
"github.com/go-gost/x/internal/plugin"
"github.com/go-gost/x/metadata"
@@ -158,10 +159,21 @@ func ParseHop(cfg *config.HopConfig, log logger.Logger) (hop.Hop, error) {
sel = selector_parser.DefaultNodeSelector()
}
maxFails, timeout := xs.DefaultMaxFails, xs.DefaultFailTimeout
if cfg.Selector != nil {
if cfg.Selector.MaxFails > 0 {
maxFails = cfg.Selector.MaxFails
}
if cfg.Selector.FailTimeout > 0 {
timeout = cfg.Selector.FailTimeout
}
}
opts := []xhop.Option{
xhop.NameOption(cfg.Name),
xhop.NodeOption(nodes...),
xhop.SelectorOption(sel),
xhop.FailFilterSettingsOption(maxFails, timeout),
xhop.BypassOption(xbypass.BypassGroup(bypass_parser.List(cfg.Bypass, cfg.Bypasses...)...)),
xhop.ReloadPeriodOption(cfg.Reload),
xhop.LoggerOption(log.WithFields(map[string]any{
+59
View File
@@ -22,6 +22,7 @@ import (
"github.com/go-gost/core/routing"
"github.com/go-gost/core/selector"
"github.com/go-gost/x/config"
xs "github.com/go-gost/x/selector"
mdutil "github.com/go-gost/x/metadata/util"
node_parser "github.com/go-gost/x/config/parsing/node"
"github.com/go-gost/x/internal/loader"
@@ -33,6 +34,12 @@ type options struct {
nodes []*chain.Node
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).
failMaxFails int
failTimeout time.Duration
fileLoader loader.Loader
redisLoader loader.Loader
httpLoader loader.Loader
@@ -71,6 +78,16 @@ func SelectorOption(s selector.Selector[*chain.Node]) Option {
}
}
// FailFilterSettingsOption sets the maxFails/failTimeout used by the hop's
// FailFilter. The pre-selection dead-node filter consults these to decide
// whether a node is currently marked failed.
func FailFilterSettingsOption(maxFails int, timeout time.Duration) Option {
return func(o *options) {
o.failMaxFails = maxFails
o.failTimeout = timeout
}
}
// ReloadPeriodOption sets the interval for periodic reloading of node lists.
func ReloadPeriodOption(period time.Duration) Option {
return func(opts *options) {
@@ -243,6 +260,30 @@ 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 !nodeIsFailed(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 {
return nil
}
@@ -327,6 +368,24 @@ func isAllBackup(nodes []*chain.Node) bool {
return len(nodes) > 0
}
// nodeIsFailed reports whether node is currently marked failed per the same
// criteria FailFilter uses: failure count has reached maxFails and the last
// failure is still within failTimeout. FailFilter itself no-ops on a single
// node, so the priority short-circuit checks the marker directly here.
func nodeIsFailed(node *chain.Node, maxFails int, timeout time.Duration) bool {
marker := node.Marker()
if marker == nil {
return false
}
if maxFails <= 0 {
maxFails = xs.DefaultMaxFails
}
if timeout <= 0 {
timeout = xs.DefaultFailTimeout
}
return !(marker.Count() < int64(maxFails) || time.Since(marker.Time()) >= timeout)
}
func (p *chainHop) isEligible(node *chain.Node, opts *hop.SelectOptions) bool {
if node == nil {
return false
+69
View File
@@ -19,6 +19,7 @@ import (
"github.com/go-gost/core/routing"
xmd "github.com/go-gost/x/metadata"
xlogger "github.com/go-gost/x/logger"
xselector "github.com/go-gost/x/selector"
"github.com/go-gost/x/registry"
)
@@ -548,6 +549,74 @@ func TestSelect_EqualPriorityDoesNotShortcut(t *testing.T) {
}
}
// TestSelect_FailCodesHighPriorityNode reproduces the reverse-proxy failCodes
// bug: a node with a stricter (longer) matcher wins by auto-computed priority,
// so the priority shortcut selected it directly and bypassed the FailFilter —
// a node marked failed by http.failCodes could never be excluded, so failover
// never happened. After the fix, marking the high-priority node makes Select
// fall through to the healthy lower-priority node.
func TestSelect_FailCodesHighPriorityNode(t *testing.T) {
// highPriority: stricter matcher -> longer rule -> higher auto priority.
highPriority := chain.NewNode("high", "127.0.0.1:8080",
chain.MatcherNodeOption(&testMatcher{match: true}),
chain.PriorityNodeOption(100), // simulates len(longer-rule)
)
// healthy: less specific matcher -> lower priority.
healthy := chain.NewNode("healthy", "127.0.0.1:9090",
chain.MatcherNodeOption(&testMatcher{match: true}),
chain.PriorityNodeOption(10),
)
sel := xselector.NewSelector[*chain.Node](
xselector.FIFOStrategy[*chain.Node](),
xselector.FailFilter[*chain.Node](1, 60*time.Second),
)
h := newTestHop(NodeOption(highPriority, healthy), SelectorOption(sel),
FailFilterSettingsOption(1, 60*time.Second))
defer h.Close()
// Before marking: high-priority node wins via the shortcut.
node := h.Select(context.Background())
if node == nil {
t.Fatal("expected node, got nil")
}
if node.Name != "high" {
t.Fatalf("expected 'high' (higher priority) before marking, got %q", node.Name)
}
// Simulate http.failCodes marking the high-priority node failed (429/5xx).
highPriority.Marker().Mark()
// After marking: Select must fall through to the healthy node.
node = h.Select(context.Background())
if node == nil {
t.Fatal("expected a node after marking, got nil")
}
if node.Name != "healthy" {
t.Fatalf("expected 'healthy' (marked node excluded by FailFilter), got %q", node.Name)
}
}
// 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) {
only := chain.NewNode("only", "127.0.0.1:8080")
h := newTestHop(NodeOption(only))
defer h.Close()
only.Marker().Mark()
node := h.Select(context.Background())
if node == nil {
t.Fatal("expected the only node, got nil")
}
if node.Name != "only" {
t.Fatalf("expected 'only', got %q", node.Name)
}
}
func TestSelect_NilNodesSkipped(t *testing.T) {
h := newTestHop()
defer h.Close()