diff --git a/hop/hop.go b/hop/hop.go index 15797e0..4a288a7 100644 --- a/hop/hop.go +++ b/hop/hop.go @@ -270,7 +270,7 @@ func (p *chainHop) Select(ctx context.Context, opts ...hop.SelectOption) *chain. if len(nodes) > 0 { alive := nodes[:0] for _, n := range nodes { - if !nodeIsFailed(n, p.options.failMaxFails, p.options.failTimeout) { + if !xs.IsFailed(n, p.options.failMaxFails, p.options.failTimeout) { alive = append(alive, n) } } @@ -368,24 +368,6 @@ 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 diff --git a/selector/filter.go b/selector/filter.go index 465ba3f..304afdd 100644 --- a/selector/filter.go +++ b/selector/filter.go @@ -30,39 +30,50 @@ func (f *failFilter[T]) Filter(ctx context.Context, vs ...T) []T { } var l []T for _, v := range vs { - maxFails := f.maxFails - failTimeout := f.failTimeout - if mi, _ := any(v).(metadata.Metadatable); mi != nil { - if md := mi.Metadata(); md != nil { - if md.IsExists(labelMaxFails) { - maxFails = mdutil.GetInt(md, labelMaxFails) - } - if md.IsExists(labelFailTimeout) { - failTimeout = mdutil.GetDuration(md, labelFailTimeout) - } - } + if !IsFailed(v, f.maxFails, f.failTimeout) { + l = append(l, v) } - if maxFails <= 0 { - maxFails = 1 - } - if failTimeout <= 0 { - failTimeout = DefaultFailTimeout - } - - if mi, _ := any(v).(selector.Markable); mi != nil { - if marker := mi.Marker(); marker != nil { - if marker.Count() < int64(maxFails) || - time.Since(marker.Time()) >= failTimeout { - l = append(l, v) - } - continue - } - } - l = append(l, v) } return l } +// IsFailed reports whether v is currently marked failed: its failure count +// has reached maxFails and the last failure is still within failTimeout. +// Per-item metadata keys "maxFails" and "failTimeout" override the given +// defaults. Items without a failure marker are never considered failed. +func IsFailed[T any](v T, maxFails int, failTimeout time.Duration) bool { + maxFails, failTimeout = effectiveFailSettings(v, maxFails, failTimeout) + + mi, _ := any(v).(selector.Markable) + if mi == nil || mi.Marker() == nil { + return false + } + marker := mi.Marker() + return !(marker.Count() < int64(maxFails) || time.Since(marker.Time()) >= failTimeout) +} + +// effectiveFailSettings resolves the maxFails/failTimeout for v, honoring +// per-item metadata overrides and defaulting to 1 / DefaultFailTimeout. +func effectiveFailSettings[T any](v T, maxFails int, failTimeout time.Duration) (int, time.Duration) { + if mi, _ := any(v).(metadata.Metadatable); mi != nil { + if md := mi.Metadata(); md != nil { + if md.IsExists(labelMaxFails) { + maxFails = mdutil.GetInt(md, labelMaxFails) + } + if md.IsExists(labelFailTimeout) { + failTimeout = mdutil.GetDuration(md, labelFailTimeout) + } + } + } + if maxFails <= 0 { + maxFails = 1 + } + if failTimeout <= 0 { + failTimeout = DefaultFailTimeout + } + return maxFails, failTimeout +} + type backupFilter[T any] struct{} // BackupFilter filters the backup objects. diff --git a/selector/filter_test.go b/selector/filter_test.go index 98b3b6b..9bb4880 100644 --- a/selector/filter_test.go +++ b/selector/filter_test.go @@ -264,3 +264,33 @@ func TestFilterMetadataLabels(t *testing.T) { t.Fatalf("failTimeout: expected 30s, got %v", ft) } } + +// --- IsFailed shared helper tests --- + +func TestIsFailed(t *testing.T) { + item := newTestMarkable(nil) + + if IsFailed(item, 1, 10*time.Second) { + t.Fatal("unmarked item should not be failed") + } + + item.Marker().Mark() + if !IsFailed(item, 1, 10*time.Second) { + t.Fatal("marked item with maxFails=1 should be failed") + } + if IsFailed(item, 2, 10*time.Second) { + t.Fatal("single mark below maxFails=2 should not be failed") + } + + // Per-item metadata override beats the given defaults. + item2 := newTestMarkable(map[string]any{"maxFails": 1, "failTimeout": "60s"}) + item2.Marker().Mark() + if !IsFailed(item2, 5, 10*time.Second) { + t.Fatal("metadata maxFails override should take precedence") + } + + // Non-markable items are never failed. + if IsFailed(42, 1, time.Second) { + t.Fatal("non-markable item should not be failed") + } +}