mirror of
https://github.com/go-gost/x.git
synced 2026-10-21 04:47:31 +00:00
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:
@@ -15,6 +15,7 @@ import (
|
|||||||
selector_parser "github.com/go-gost/x/config/parsing/selector"
|
selector_parser "github.com/go-gost/x/config/parsing/selector"
|
||||||
xhop "github.com/go-gost/x/hop"
|
xhop "github.com/go-gost/x/hop"
|
||||||
hop_plugin "github.com/go-gost/x/hop/plugin"
|
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/loader"
|
||||||
"github.com/go-gost/x/internal/plugin"
|
"github.com/go-gost/x/internal/plugin"
|
||||||
"github.com/go-gost/x/metadata"
|
"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()
|
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{
|
opts := []xhop.Option{
|
||||||
xhop.NameOption(cfg.Name),
|
xhop.NameOption(cfg.Name),
|
||||||
xhop.NodeOption(nodes...),
|
xhop.NodeOption(nodes...),
|
||||||
xhop.SelectorOption(sel),
|
xhop.SelectorOption(sel),
|
||||||
|
xhop.FailFilterSettingsOption(maxFails, timeout),
|
||||||
xhop.BypassOption(xbypass.BypassGroup(bypass_parser.List(cfg.Bypass, cfg.Bypasses...)...)),
|
xhop.BypassOption(xbypass.BypassGroup(bypass_parser.List(cfg.Bypass, cfg.Bypasses...)...)),
|
||||||
xhop.ReloadPeriodOption(cfg.Reload),
|
xhop.ReloadPeriodOption(cfg.Reload),
|
||||||
xhop.LoggerOption(log.WithFields(map[string]any{
|
xhop.LoggerOption(log.WithFields(map[string]any{
|
||||||
|
|||||||
+59
@@ -22,6 +22,7 @@ import (
|
|||||||
"github.com/go-gost/core/routing"
|
"github.com/go-gost/core/routing"
|
||||||
"github.com/go-gost/core/selector"
|
"github.com/go-gost/core/selector"
|
||||||
"github.com/go-gost/x/config"
|
"github.com/go-gost/x/config"
|
||||||
|
xs "github.com/go-gost/x/selector"
|
||||||
mdutil "github.com/go-gost/x/metadata/util"
|
mdutil "github.com/go-gost/x/metadata/util"
|
||||||
node_parser "github.com/go-gost/x/config/parsing/node"
|
node_parser "github.com/go-gost/x/config/parsing/node"
|
||||||
"github.com/go-gost/x/internal/loader"
|
"github.com/go-gost/x/internal/loader"
|
||||||
@@ -33,6 +34,12 @@ type options struct {
|
|||||||
nodes []*chain.Node
|
nodes []*chain.Node
|
||||||
bypass bypass.Bypass
|
bypass bypass.Bypass
|
||||||
selector selector.Selector[*chain.Node]
|
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
|
fileLoader loader.Loader
|
||||||
redisLoader loader.Loader
|
redisLoader loader.Loader
|
||||||
httpLoader 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.
|
// ReloadPeriodOption sets the interval for periodic reloading of node lists.
|
||||||
func ReloadPeriodOption(period time.Duration) Option {
|
func ReloadPeriodOption(period time.Duration) Option {
|
||||||
return func(opts *options) {
|
return func(opts *options) {
|
||||||
@@ -243,6 +260,30 @@ func (p *chainHop) Select(ctx context.Context, opts ...hop.SelectOption) *chain.
|
|||||||
|
|
||||||
nodes = append(nodes, node)
|
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 {
|
if len(nodes) == 0 {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
@@ -327,6 +368,24 @@ func isAllBackup(nodes []*chain.Node) bool {
|
|||||||
return len(nodes) > 0
|
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 {
|
func (p *chainHop) isEligible(node *chain.Node, opts *hop.SelectOptions) bool {
|
||||||
if node == nil {
|
if node == nil {
|
||||||
return false
|
return false
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ import (
|
|||||||
"github.com/go-gost/core/routing"
|
"github.com/go-gost/core/routing"
|
||||||
xmd "github.com/go-gost/x/metadata"
|
xmd "github.com/go-gost/x/metadata"
|
||||||
xlogger "github.com/go-gost/x/logger"
|
xlogger "github.com/go-gost/x/logger"
|
||||||
|
xselector "github.com/go-gost/x/selector"
|
||||||
"github.com/go-gost/x/registry"
|
"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) {
|
func TestSelect_NilNodesSkipped(t *testing.T) {
|
||||||
h := newTestHop()
|
h := newTestHop()
|
||||||
defer h.Close()
|
defer h.Close()
|
||||||
|
|||||||
Reference in New Issue
Block a user