diff --git a/.github/workflows/doctests.yaml b/.github/workflows/doctests.yaml index 980629f06b..c5fb19a5fe 100644 --- a/.github/workflows/doctests.yaml +++ b/.github/workflows/doctests.yaml @@ -4,7 +4,7 @@ on: push: branches: [master, examples] pull_request: - branches: [master, examples] + branches: [master, examples, 'feature/**'] permissions: contents: read diff --git a/internal/circuitbreaker/circuit_breaker.go b/internal/circuitbreaker/circuit_breaker.go new file mode 100644 index 0000000000..372d2cb3e6 --- /dev/null +++ b/internal/circuitbreaker/circuit_breaker.go @@ -0,0 +1,421 @@ +// Package circuitbreaker provides a circuit breaker implementation for fault tolerance. +package circuitbreaker + +import ( + "sync" + "sync/atomic" + "time" +) + +// State represents the state of a circuit breaker. +type State int32 + +const ( + // StateClosed indicates the circuit is closed and requests are allowed. + StateClosed State = iota + // StateOpen indicates the circuit is open and requests are blocked. + StateOpen + // StateHalfOpen indicates the circuit is testing if the service has recovered. + StateHalfOpen +) + +// String returns the string representation of the circuit state. +func (s State) String() string { + switch s { + case StateClosed: + return "closed" + case StateOpen: + return "open" + case StateHalfOpen: + return "half-open" + default: + return "unknown" + } +} + +// Config holds configuration for a circuit breaker. +type Config struct { + // FailureThreshold is the number of failures before opening the circuit. + // Default: 5 + FailureThreshold int + + // SuccessThreshold is the number of successes in half-open state before closing. + // Default: 2 + SuccessThreshold int + + // MaxHalfOpenRequests is the maximum number of requests allowed in half-open state. + // If 0, uses SuccessThreshold as the limit. + // Default: 0 (uses SuccessThreshold) + MaxHalfOpenRequests int + + // OpenTimeout is how long to wait before transitioning from open to half-open. + // This is the circuit "grace period" that gives a failed database time to + // self-heal before it is probed again. + // Default: 60 seconds + OpenTimeout time.Duration +} + +// DefaultConfig returns the default circuit breaker configuration. +func DefaultConfig() Config { + return Config{ + FailureThreshold: 5, + SuccessThreshold: 2, + MaxHalfOpenRequests: 0, + OpenTimeout: 60 * time.Second, + } +} + +// applyDefaults fills in zero values with defaults. +func (c *Config) applyDefaults() { + if c.FailureThreshold <= 0 { + c.FailureThreshold = 5 + } + if c.SuccessThreshold <= 0 { + c.SuccessThreshold = 2 + } + if c.MaxHalfOpenRequests <= 0 { + c.MaxHalfOpenRequests = c.SuccessThreshold + } + if c.OpenTimeout <= 0 { + c.OpenTimeout = 60 * time.Second + } +} + +// StateChangeCallback is called when the circuit breaker state changes. +type StateChangeCallback func(oldState, newState State) + +// CircuitBreaker implements the circuit breaker pattern. +type CircuitBreaker struct { + config Config + + state atomic.Int32 + failures atomic.Int32 + successes atomic.Int32 + requests atomic.Int32 // Request count in half-open state + lastFailure atomic.Int64 // Unix nano timestamp + + // transitionMu serializes the open -> half-open transition so the + // half-open counters can be cleared before the new state is published. + transitionMu sync.Mutex + + mu sync.RWMutex + callbacks []StateChangeCallback +} + +// New creates a new circuit breaker with the given configuration. +func New(config Config) *CircuitBreaker { + config.applyDefaults() + cb := &CircuitBreaker{ + config: config, + } + cb.state.Store(int32(StateClosed)) + return cb +} + +// State returns the current state without triggering any transitions. +func (cb *CircuitBreaker) State() State { + return State(cb.state.Load()) +} + +// CheckState returns the current state and may trigger state transitions. +// Use this when you need to check if requests should be allowed. +func (cb *CircuitBreaker) CheckState() State { + state := State(cb.state.Load()) + + if state == StateOpen { + // Check if we should transition to half-open. + // Guard against a zero timestamp (no failure recorded yet) so we don't + // treat the Unix epoch as the last failure and transition immediately. + lastFailure := cb.lastFailure.Load() + if lastFailure != 0 && time.Now().UnixNano()-lastFailure >= int64(cb.config.OpenTimeout) { + // Clear the half-open counters BEFORE half-open becomes visible: + // clearing after a CAS would erase reservations and successes + // recorded by requests that observe the new state in between. + // The mutex keeps a second (stale) transition attempt from + // re-clearing counters that live probes are already using; while + // the state is still Open no request touches these counters, so + // clearing here is race-free. + cb.transitionMu.Lock() + // Re-read lastFailure under the lock: a failure recorded after + // the check above (e.g. from a request admitted before the + // circuit opened) must restart the grace period — transitioning + // off the stale timestamp would probe the endpoint early. + lastFailure = cb.lastFailure.Load() + if State(cb.state.Load()) == StateOpen && + lastFailure != 0 && time.Now().UnixNano()-lastFailure >= int64(cb.config.OpenTimeout) { + cb.successes.Store(0) + cb.requests.Store(0) + // CAS, not Store: a concurrent Reset may have just published + // Closed, and overwriting it with HalfOpen would silently + // undo the reset. + if cb.state.CompareAndSwap(int32(StateOpen), int32(StateHalfOpen)) { + cb.transitionMu.Unlock() + cb.notifyCallbacks(StateOpen, StateHalfOpen) + return StateHalfOpen + } + } + cb.transitionMu.Unlock() + } + } + + return State(cb.state.Load()) +} + +// IsAllowed returns true if a request should be allowed through. +// This is a convenience method that combines CheckState with half-open request limiting. +func (cb *CircuitBreaker) IsAllowed() bool { + allowed, _ := cb.Allow() + return allowed +} + +// Allow reports whether a request may proceed and whether the admission +// reserved a bounded half-open probe slot. Closed-state admissions reserve +// nothing, so callers whose operation may outlive a later open -> half-open +// transition (for example a WATCH transaction) must consult reserved before +// calling ReleaseHalfOpen — an unconditional release would free a slot a +// real recovery probe is holding. +func (cb *CircuitBreaker) Allow() (allowed, reserved bool) { + state := cb.CheckState() + + switch state { + case StateClosed: + return true, false + case StateOpen: + return false, false + case StateHalfOpen: + // Limit requests in half-open state + requests := cb.requests.Add(1) + if int(requests) > cb.config.MaxHalfOpenRequests { + cb.requests.Add(-1) // Revert + return false, false + } + // Re-check after reserving: a probe failure may have re-opened the + // circuit in between (zeroing the counter), and admitting here would + // both send a request to the endpoint that just failed its recovery + // probe and leave a phantom reservation behind. + if State(cb.state.Load()) != StateHalfOpen { + if cb.requests.Add(-1) < 0 { + cb.requests.Store(0) + } + return false, false + } + return true, true + default: + return false, false + } +} + +// ReleaseHalfOpen returns a half-open request slot previously reserved by a +// successful IsAllowed call when the operation produced neither a recordable +// success nor failure (for example, it was aborted for an unrelated reason). +// Without this, a reserved-but-never-completed probe could permanently starve +// half-open recovery once MaxHalfOpenRequests slots are exhausted. It only has +// an effect while the breaker is half-open. +func (cb *CircuitBreaker) ReleaseHalfOpen() { + if State(cb.state.Load()) != StateHalfOpen { + return + } + if cb.requests.Add(-1) < 0 { + cb.requests.Store(0) + } +} + +// RecordSuccess records a successful operation that was admitted through +// IsAllowed. In half-open state the completed probe's admission slot is +// released when the circuit does not close. +func (cb *CircuitBreaker) RecordSuccess() { + cb.recordSuccess(true) +} + +// RecordExternalSuccess records a successful operation that was NOT admitted +// through IsAllowed (e.g. an out-of-band health check). It counts toward +// closing a half-open circuit but never releases an admission slot it did +// not hold — releasing one would let more than MaxHalfOpenRequests requests +// reach a recovering service. +func (cb *CircuitBreaker) RecordExternalSuccess() { + cb.recordSuccess(false) +} + +func (cb *CircuitBreaker) recordSuccess(heldSlot bool) { + state := State(cb.state.Load()) + + switch state { + case StateHalfOpen: + successes := cb.successes.Add(1) + // Re-check state after increment - another goroutine may have changed it + if State(cb.state.Load()) != StateHalfOpen { + return + } + if int(successes) >= cb.config.SuccessThreshold { + // Clear the failure counter BEFORE Closed becomes visible: it + // still holds the count that opened the circuit, and a failure + // recorded between the state swap and a later reset would + // immediately re-open the circuit off that stale count. + cb.failures.Store(0) + if cb.state.CompareAndSwap(int32(StateHalfOpen), int32(StateClosed)) { + // Notify callbacks before resetting the half-open counters so + // they observe the success count that triggered the transition. + cb.notifyCallbacks(StateHalfOpen, StateClosed) + cb.successes.Store(0) + cb.requests.Store(0) + } + return + } + if heldSlot { + // The probe completed but the circuit is still half-open: give + // its admission slot back, so MaxHalfOpenRequests bounds + // CONCURRENT probes rather than a lifetime budget. Without this, + // a MaxHalfOpenRequests lower than SuccessThreshold could never + // accumulate enough successes to close the circuit. + cb.ReleaseHalfOpen() + } + case StateClosed: + // Reset failure count on success + cb.failures.Store(0) + } +} + +// RecordFailure records a failed operation. +func (cb *CircuitBreaker) RecordFailure() { + cb.lastFailure.Store(time.Now().UnixNano()) + state := State(cb.state.Load()) + + switch state { + case StateClosed: + failures := cb.failures.Add(1) + if int(failures) >= cb.config.FailureThreshold { + if cb.state.CompareAndSwap(int32(StateClosed), int32(StateOpen)) { + // A Reset that completed between the timestamp store above and + // this CAS wiped lastFailure; repair it (CAS so a concurrent + // newer failure's timestamp is kept), or the zero-timestamp + // guard in CheckState would wedge the circuit open. A Reset + // that wipes after this repair also publishes Closed after, + // so the circuit does not stay Open with a zero timestamp. + cb.lastFailure.CompareAndSwap(0, time.Now().UnixNano()) + // Notify callbacks before clearing the half-open counters so + // observers see the failure count that triggered the + // transition, matching the half-open -> closed/open paths. + cb.notifyCallbacks(StateClosed, StateOpen) + // successes and requests should already be 0 in Closed (they + // are only incremented while half-open, and every half-open + // exit zeroes them). Reset defensively so the invariant + // "successes/requests are clean on entry to Open" is upheld + // consistently across all transitions into Open, even if a + // future change starts touching those counters in Closed. + cb.successes.Store(0) + cb.requests.Store(0) + } + } + case StateHalfOpen: + // Any failure in half-open state opens the circuit. + if cb.state.CompareAndSwap(int32(StateHalfOpen), int32(StateOpen)) { + // Same timestamp repair as the closed -> open transition above. + cb.lastFailure.CompareAndSwap(0, time.Now().UnixNano()) + // Notify callbacks before resetting counters so they observe the + // counts that triggered the transition, matching the half-open -> + // closed path in RecordSuccess. + cb.notifyCallbacks(StateHalfOpen, StateOpen) + cb.successes.Store(0) + cb.requests.Store(0) + } + } +} + +// OnStateChange registers a callback to be called when the state changes. +func (cb *CircuitBreaker) OnStateChange(callback StateChangeCallback) { + cb.mu.Lock() + defer cb.mu.Unlock() + cb.callbacks = append(cb.callbacks, callback) +} + +// notifyCallbacks notifies all registered callbacks of a state change. +func (cb *CircuitBreaker) notifyCallbacks(oldState, newState State) { + cb.mu.RLock() + callbacks := make([]StateChangeCallback, len(cb.callbacks)) + copy(callbacks, cb.callbacks) + cb.mu.RUnlock() + + for _, callback := range callbacks { + callback(oldState, newState) + } +} + +// Reset resets the circuit breaker to closed state. +// If the circuit was not already closed, callbacks are notified. +func (cb *CircuitBreaker) Reset() { + // Clear the counters BEFORE Closed becomes visible: a failure recorded + // right after the swap must count against a fresh counter — off the + // stale one it could immediately re-open the circuit, and the + // lastFailure wipe below would then wedge it open past the + // zero-timestamp guard in CheckState. + cb.failures.Store(0) + cb.successes.Store(0) + cb.requests.Store(0) + cb.lastFailure.Store(0) + oldState := State(cb.state.Swap(int32(StateClosed))) + if oldState != StateClosed { + cb.notifyCallbacks(oldState, StateClosed) + } +} + +// Stats returns current statistics for monitoring. +type Stats struct { + State State + Failures int32 + Successes int32 + Requests int32 + LastFailureTime time.Time +} + +// Stats returns current statistics. +func (cb *CircuitBreaker) Stats() Stats { + lastFailure := cb.lastFailure.Load() + var lastFailureTime time.Time + if lastFailure > 0 { + lastFailureTime = time.Unix(0, lastFailure) + } + + return Stats{ + State: cb.State(), + Failures: cb.failures.Load(), + Successes: cb.successes.Load(), + Requests: cb.requests.Load(), + LastFailureTime: lastFailureTime, + } +} + +// Execute runs the given function with circuit breaker protection. +// Returns ErrCircuitOpen if the circuit is open and not ready for testing. +func (cb *CircuitBreaker) Execute(fn func() error) error { + allowed, reserved := cb.Allow() + if !allowed { + return ErrCircuitOpen + } + + err := fn() + if err != nil { + cb.RecordFailure() + return err + } + + if reserved { + cb.RecordSuccess() + } else { + // Admitted while closed: no slot was reserved, so the success must + // not release one — fn can outlive a later open -> half-open + // transition, and RecordSuccess would free a slot a real recovery + // probe is holding. + cb.RecordExternalSuccess() + } + return nil +} + +// ErrCircuitOpen is returned when the circuit breaker is open. +var ErrCircuitOpen = &CircuitOpenError{} + +// CircuitOpenError indicates the circuit breaker is open. +type CircuitOpenError struct{} + +func (e *CircuitOpenError) Error() string { + return "circuit breaker is open" +} diff --git a/internal/circuitbreaker/circuit_breaker_test.go b/internal/circuitbreaker/circuit_breaker_test.go new file mode 100644 index 0000000000..21e0f0d340 --- /dev/null +++ b/internal/circuitbreaker/circuit_breaker_test.go @@ -0,0 +1,785 @@ +package circuitbreaker + +import ( + "sync" + "sync/atomic" + "testing" + "time" +) + +func TestCircuitBreaker_InitialState(t *testing.T) { + cb := New(DefaultConfig()) + + if cb.State() != StateClosed { + t.Errorf("expected initial state to be Closed, got %v", cb.State()) + } +} + +func TestCircuitBreaker_OpenAfterFailures(t *testing.T) { + config := Config{ + FailureThreshold: 3, + SuccessThreshold: 2, + OpenTimeout: 100 * time.Millisecond, + } + cb := New(config) + + // Record failures + for i := 0; i < 3; i++ { + cb.RecordFailure() + } + + if cb.State() != StateOpen { + t.Errorf("expected state to be Open after %d failures, got %v", 3, cb.State()) + } +} + +func TestCircuitBreaker_TransitionToHalfOpen(t *testing.T) { + config := Config{ + FailureThreshold: 2, + SuccessThreshold: 1, + OpenTimeout: 50 * time.Millisecond, + } + cb := New(config) + + // Open the circuit + cb.RecordFailure() + cb.RecordFailure() + + if cb.State() != StateOpen { + t.Fatalf("expected state to be Open, got %v", cb.State()) + } + + // Wait for timeout + time.Sleep(60 * time.Millisecond) + + // CheckState should transition to half-open + state := cb.CheckState() + if state != StateHalfOpen { + t.Errorf("expected state to be HalfOpen after timeout, got %v", state) + } +} + +func TestCircuitBreaker_CloseAfterSuccesses(t *testing.T) { + config := Config{ + FailureThreshold: 2, + SuccessThreshold: 2, + OpenTimeout: 50 * time.Millisecond, + } + cb := New(config) + + // Open the circuit + cb.RecordFailure() + cb.RecordFailure() + + // Wait for timeout and transition to half-open + time.Sleep(60 * time.Millisecond) + cb.CheckState() + + // Record successes + cb.RecordSuccess() + cb.RecordSuccess() + + if cb.State() != StateClosed { + t.Errorf("expected state to be Closed after successes, got %v", cb.State()) + } +} + +func TestCircuitBreaker_ReopenOnFailureInHalfOpen(t *testing.T) { + config := Config{ + FailureThreshold: 2, + SuccessThreshold: 2, + OpenTimeout: 50 * time.Millisecond, + } + cb := New(config) + + // Open the circuit + cb.RecordFailure() + cb.RecordFailure() + + // Wait for timeout and transition to half-open + time.Sleep(60 * time.Millisecond) + cb.CheckState() + + if cb.State() != StateHalfOpen { + t.Fatalf("expected state to be HalfOpen, got %v", cb.State()) + } + + // Record a failure - should reopen + cb.RecordFailure() + + if cb.State() != StateOpen { + t.Errorf("expected state to be Open after failure in half-open, got %v", cb.State()) + } +} + +func TestCircuitBreaker_IsAllowed(t *testing.T) { + config := Config{ + FailureThreshold: 2, + SuccessThreshold: 2, + OpenTimeout: 50 * time.Millisecond, + } + cb := New(config) + + // Should be allowed when closed + if !cb.IsAllowed() { + t.Error("expected IsAllowed to return true when closed") + } + + // Open the circuit + cb.RecordFailure() + cb.RecordFailure() + + // Should not be allowed when open + if cb.IsAllowed() { + t.Error("expected IsAllowed to return false when open") + } +} + +func TestCircuitBreaker_MaxHalfOpenRequests(t *testing.T) { + config := Config{ + FailureThreshold: 2, + SuccessThreshold: 3, + MaxHalfOpenRequests: 2, + OpenTimeout: 50 * time.Millisecond, + } + cb := New(config) + + // Open the circuit + cb.RecordFailure() + cb.RecordFailure() + + // Wait for timeout + time.Sleep(60 * time.Millisecond) + + // First two requests should be allowed + if !cb.IsAllowed() { + t.Error("first request should be allowed in half-open") + } + if !cb.IsAllowed() { + t.Error("second request should be allowed in half-open") + } + + // Third request should be rejected + if cb.IsAllowed() { + t.Error("third request should be rejected (max half-open requests)") + } +} + +func TestCircuitBreaker_ReleaseHalfOpen(t *testing.T) { + config := Config{ + FailureThreshold: 2, + SuccessThreshold: 3, + MaxHalfOpenRequests: 2, + OpenTimeout: 50 * time.Millisecond, + } + cb := New(config) + + // Open the circuit, then wait for the half-open window. + cb.RecordFailure() + cb.RecordFailure() + time.Sleep(60 * time.Millisecond) + + // Reserve both half-open slots. + if !cb.IsAllowed() { + t.Fatal("first request should be allowed in half-open") + } + if !cb.IsAllowed() { + t.Fatal("second request should be allowed in half-open") + } + if cb.IsAllowed() { + t.Fatal("third request should be rejected before release") + } + + // Releasing a reserved slot should let a subsequent probe through. + cb.ReleaseHalfOpen() + if !cb.IsAllowed() { + t.Error("request should be allowed after ReleaseHalfOpen") + } + + // Release must not drive the counter negative or admit extra probes. + cb.ReleaseHalfOpen() + cb.ReleaseHalfOpen() + if cb.requests.Load() < 0 { + t.Errorf("requests counter must not go negative, got %d", cb.requests.Load()) + } + + // ReleaseHalfOpen is a no-op outside the half-open state. + cb.Reset() + cb.ReleaseHalfOpen() + if cb.requests.Load() != 0 { + t.Errorf("expected requests to remain 0 when closed, got %d", cb.requests.Load()) + } +} + +func TestCircuitBreaker_OnStateChange(t *testing.T) { + config := Config{ + FailureThreshold: 2, + SuccessThreshold: 1, + OpenTimeout: 50 * time.Millisecond, + } + cb := New(config) + + var transitions []struct{ old, new State } + cb.OnStateChange(func(oldState, newState State) { + transitions = append(transitions, struct{ old, new State }{oldState, newState}) + }) + + // Open the circuit + cb.RecordFailure() + cb.RecordFailure() + + // Wait and transition to half-open + time.Sleep(60 * time.Millisecond) + cb.CheckState() + + // Close the circuit + cb.RecordSuccess() + + if len(transitions) != 3 { + t.Fatalf("expected 3 transitions, got %d", len(transitions)) + } + + // Closed -> Open + if transitions[0].old != StateClosed || transitions[0].new != StateOpen { + t.Errorf("expected Closed->Open, got %v->%v", transitions[0].old, transitions[0].new) + } + + // Open -> HalfOpen + if transitions[1].old != StateOpen || transitions[1].new != StateHalfOpen { + t.Errorf("expected Open->HalfOpen, got %v->%v", transitions[1].old, transitions[1].new) + } + + // HalfOpen -> Closed + if transitions[2].old != StateHalfOpen || transitions[2].new != StateClosed { + t.Errorf("expected HalfOpen->Closed, got %v->%v", transitions[2].old, transitions[2].new) + } +} + +func TestCircuitBreaker_CallbackObservesSuccessCountOnClose(t *testing.T) { + config := Config{ + FailureThreshold: 2, + SuccessThreshold: 2, + OpenTimeout: 50 * time.Millisecond, + } + cb := New(config) + + var closeSuccesses int32 = -1 + cb.OnStateChange(func(oldState, newState State) { + if oldState == StateHalfOpen && newState == StateClosed { + closeSuccesses = cb.Stats().Successes + } + }) + + // Open the circuit, wait for the timeout, then transition to half-open. + cb.RecordFailure() + cb.RecordFailure() + time.Sleep(60 * time.Millisecond) + cb.CheckState() + + // Record enough successes to close the circuit. + cb.RecordSuccess() + cb.RecordSuccess() + + if cb.State() != StateClosed { + t.Fatalf("expected state to be Closed, got %v", cb.State()) + } + // The callback must see the success count that triggered the close, not the + // post-reset value of 0. + if closeSuccesses != int32(config.SuccessThreshold) { + t.Errorf("expected callback to observe %d successes, got %d", + config.SuccessThreshold, closeSuccesses) + } +} + +func TestCircuitBreaker_Reset(t *testing.T) { + config := Config{ + FailureThreshold: 2, + SuccessThreshold: 1, + OpenTimeout: 1 * time.Hour, // Long timeout + } + cb := New(config) + + // Open the circuit + cb.RecordFailure() + cb.RecordFailure() + + if cb.State() != StateOpen { + t.Fatalf("expected state to be Open, got %v", cb.State()) + } + + // Reset + cb.Reset() + + if cb.State() != StateClosed { + t.Errorf("expected state to be Closed after reset, got %v", cb.State()) + } + + stats := cb.Stats() + if stats.Failures != 0 || stats.Successes != 0 { + t.Errorf("expected counters to be reset, got failures=%d, successes=%d", + stats.Failures, stats.Successes) + } +} + +func TestCircuitBreaker_ResetNotifiesCallbacks(t *testing.T) { + config := Config{ + FailureThreshold: 2, + SuccessThreshold: 1, + OpenTimeout: 1 * time.Hour, + } + cb := New(config) + + var transitions []struct{ old, new State } + cb.OnStateChange(func(oldState, newState State) { + transitions = append(transitions, struct{ old, new State }{oldState, newState}) + }) + + // Open the circuit + cb.RecordFailure() + cb.RecordFailure() + + if len(transitions) != 1 { + t.Fatalf("expected 1 transition (Closed->Open), got %d", len(transitions)) + } + + // Reset should notify callback + cb.Reset() + + if len(transitions) != 2 { + t.Fatalf("expected 2 transitions after reset, got %d", len(transitions)) + } + + // Verify Open -> Closed transition + if transitions[1].old != StateOpen || transitions[1].new != StateClosed { + t.Errorf("expected Open->Closed, got %v->%v", transitions[1].old, transitions[1].new) + } + + // Reset when already closed should NOT notify + cb.Reset() + if len(transitions) != 2 { + t.Errorf("expected no callback when resetting already-closed circuit, got %d transitions", len(transitions)) + } +} + +func TestCircuitBreaker_ConcurrentAccess(t *testing.T) { + config := Config{ + FailureThreshold: 100, + SuccessThreshold: 50, + OpenTimeout: 100 * time.Millisecond, + } + cb := New(config) + + var wg sync.WaitGroup + for i := 0; i < 100; i++ { + wg.Add(2) + go func() { + defer wg.Done() + cb.RecordFailure() + }() + go func() { + defer wg.Done() + cb.RecordSuccess() + }() + } + wg.Wait() + + // Should not panic and state should be valid + state := cb.State() + if state != StateClosed && state != StateOpen && state != StateHalfOpen { + t.Errorf("invalid state: %v", state) + } +} + +func TestCircuitBreaker_Stats(t *testing.T) { + cb := New(DefaultConfig()) + + cb.RecordFailure() + cb.RecordFailure() + + stats := cb.Stats() + if stats.Failures != 2 { + t.Errorf("expected 2 failures, got %d", stats.Failures) + } + if stats.LastFailureTime.IsZero() { + t.Error("expected LastFailureTime to be set") + } +} + +// TestCircuitBreaker_HalfOpenCountersAreCleanOnReentry asserts the invariant +// that successes and requests start at 0 every time the breaker enters the +// HalfOpen state, regardless of what activity preceded it. The transitions +// out of HalfOpen and out of Open each zero those counters; this test guards +// against a future change that lets them carry over from a previous cycle. +func TestCircuitBreaker_HalfOpenCountersAreCleanOnReentry(t *testing.T) { + config := Config{ + FailureThreshold: 2, + SuccessThreshold: 2, + OpenTimeout: 50 * time.Millisecond, + } + cb := New(config) + + // Cycle 1: drive the breaker through Open -> HalfOpen -> Closed so the + // counters have non-trivial values before the next failure burst. + cb.RecordFailure() + cb.RecordFailure() + time.Sleep(60 * time.Millisecond) + cb.CheckState() // -> HalfOpen + cb.RecordSuccess() + cb.RecordSuccess() // -> Closed (zeroes failures, successes, requests) + + if cb.State() != StateClosed { + t.Fatalf("setup: expected Closed after first cycle, got %v", cb.State()) + } + + // Run plenty of successful traffic in Closed; this must not leak into + // the successes counter (which is only meaningful in HalfOpen). + for i := 0; i < 50; i++ { + cb.RecordSuccess() + } + if s := cb.Stats().Successes; s != 0 { + t.Errorf("successes must remain 0 in Closed, got %d", s) + } + + // Cycle 2: drive Closed -> Open. After the transition both half-open + // counters must be 0 so the next HalfOpen cycle starts clean. + cb.RecordFailure() + cb.RecordFailure() // -> Open + if cb.State() != StateOpen { + t.Fatalf("expected Open after threshold, got %v", cb.State()) + } + stats := cb.Stats() + if stats.Successes != 0 { + t.Errorf("successes must be 0 on entry to Open, got %d", stats.Successes) + } + if stats.Requests != 0 { + t.Errorf("requests must be 0 on entry to Open, got %d", stats.Requests) + } + + // And the first success after Open -> HalfOpen must count as 1, not as + // "1 + whatever leaked from before". + time.Sleep(60 * time.Millisecond) + cb.CheckState() // -> HalfOpen + cb.RecordSuccess() + if s := cb.Stats().Successes; s != 1 { + t.Errorf("first success in HalfOpen must be 1, got %d", s) + } +} + +func TestCircuitBreaker_FailureCounterClearedBeforeCloseIsPublished(t *testing.T) { + config := Config{ + FailureThreshold: 2, + SuccessThreshold: 1, + OpenTimeout: 50 * time.Millisecond, + } + cb := New(config) + + // A failure that lands the instant Closed becomes visible (here: from + // the HalfOpen -> Closed callback, which runs while the state is already + // Closed) must count against a fresh failure counter. If the counter + // still holds the count that opened the circuit, this single failure + // re-opens it immediately. + cb.OnStateChange(func(oldState, newState State) { + if oldState == StateHalfOpen && newState == StateClosed { + cb.RecordFailure() + } + }) + + cb.RecordFailure() + cb.RecordFailure() // -> Open, failures == FailureThreshold + time.Sleep(60 * time.Millisecond) + cb.CheckState() // -> HalfOpen + cb.RecordSuccess() // -> Closed; callback records one failure + + if got := cb.State(); got != StateClosed { + t.Errorf("one failure right after recovery re-opened the circuit: state = %v", got) + } +} + +func TestCircuitBreaker_HalfOpenAdmissionNotErasedByTransition(t *testing.T) { + config := Config{ + FailureThreshold: 1, + SuccessThreshold: 2, + MaxHalfOpenRequests: 1, + OpenTimeout: time.Nanosecond, + } + cb := New(config) + + // Reservations taken the moment HalfOpen becomes visible must not be + // erased by the transition's own counter maintenance: that would admit + // more than MaxHalfOpenRequests concurrent probes. Hammer the + // open -> half-open edge and watch the concurrent-admission gauge. + const rounds = 5000 + workers := 8 + var overrun atomic.Bool + + for r := 0; r < rounds && !overrun.Load(); r++ { + cb.RecordFailure() // (re-)open; OpenTimeout of 1ns has already elapsed + + var inFlight atomic.Int32 + start := make(chan struct{}) + var wg sync.WaitGroup + for w := 0; w < workers; w++ { + wg.Add(1) + go func() { + defer wg.Done() + <-start + if cb.IsAllowed() { + if inFlight.Add(1) > int32(config.MaxHalfOpenRequests) { + overrun.Store(true) + } + inFlight.Add(-1) + cb.ReleaseHalfOpen() + } + }() + } + close(start) + wg.Wait() + } + + if overrun.Load() { + t.Error("more concurrent requests admitted than MaxHalfOpenRequests") + } +} + +func TestCircuitBreaker_ResetNotLostDuringHalfOpenTransition(t *testing.T) { + config := Config{ + FailureThreshold: 1, + SuccessThreshold: 1, + OpenTimeout: time.Nanosecond, + } + + // Reset racing the open -> half-open transition must never be + // overwritten: whatever the interleaving, the breaker must not end up + // half-open after a Reset (either the Reset lands last, or the + // transition saw Closed and did nothing). + for i := 0; i < 5000; i++ { + cb := New(config) + cb.RecordFailure() // -> Open; the 1ns timeout has already elapsed + + start := make(chan struct{}) + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); <-start; cb.CheckState() }() + go func() { defer wg.Done(); <-start; cb.Reset() }() + close(start) + wg.Wait() + + if s := cb.State(); s == StateHalfOpen { + t.Fatalf("round %d: Reset lost — breaker half-open after Reset", i) + } + } +} + +func TestCircuitBreaker_ExternalSuccessDoesNotReleaseCommandSlots(t *testing.T) { + config := Config{ + FailureThreshold: 1, + SuccessThreshold: 3, + MaxHalfOpenRequests: 1, + OpenTimeout: time.Nanosecond, + } + cb := New(config) + cb.RecordFailure() + time.Sleep(time.Millisecond) // let the 1ns OpenTimeout provably elapse + cb.CheckState() // -> HalfOpen + + if !cb.IsAllowed() { + t.Fatal("setup: expected to reserve the only half-open slot") + } + + // An out-of-band success (e.g. a background health check) never held an + // admission slot, so it must not release the one a real command probe is + // still using — that would let more than MaxHalfOpenRequests hit a + // recovering database. + cb.RecordExternalSuccess() + if cb.IsAllowed() { + t.Error("external success released a command probe's half-open slot") + } + + // It still counts toward closing the circuit. + cb.RecordExternalSuccess() + cb.RecordExternalSuccess() // successes reach SuccessThreshold + if got := cb.State(); got != StateClosed { + t.Errorf("state = %v after SuccessThreshold external successes, want Closed", got) + } +} + +func TestCircuitBreaker_ResetClearsCountersBeforePublishingClosed(t *testing.T) { + config := Config{ + FailureThreshold: 2, + SuccessThreshold: 1, + OpenTimeout: time.Hour, + } + + // A failure racing Reset must count against a fresh counter: if Closed + // becomes visible while the counter still holds the count that opened + // the circuit, one failure re-opens it — and Reset then zeroes + // lastFailure, wedging the breaker open past its timeout guard. + const threshold = 20 + config.FailureThreshold = threshold + for i := 0; i < 5000; i++ { + cb := New(config) + for f := 0; f < threshold; f++ { + cb.RecordFailure() // -> Open, failures == FailureThreshold + } + + start := make(chan struct{}) + var wg sync.WaitGroup + wg.Add(2) + go func() { defer wg.Done(); <-start; cb.Reset() }() + go func() { + defer wg.Done() + <-start + // Far fewer failures than the threshold: only the stale + // pre-reset count can push the breaker over the edge. + for f := 0; f < threshold/2; f++ { + cb.RecordFailure() + } + }() + close(start) + wg.Wait() + + if s := cb.State(); s != StateClosed { + t.Fatalf("round %d: %d post-Reset failures (threshold %d) re-opened the circuit (state %v)", + i, threshold/2, threshold, s) + } + } +} + +func TestCircuitBreaker_NoReservationSurvivesHalfOpenReopen(t *testing.T) { + config := Config{ + FailureThreshold: 1, + SuccessThreshold: 2, + MaxHalfOpenRequests: 2, + OpenTimeout: time.Nanosecond, + } + + // A reservation racing the half-open -> open transition must not stick: + // the transition zeroes the counter, and a late Add would both admit a + // request to the endpoint that just failed its probe and pollute the + // counter for the next half-open epoch. + for i := 0; i < 5000; i++ { + cb := New(config) + cb.RecordFailure() + cb.CheckState() // -> HalfOpen + + start := make(chan struct{}) + var wg sync.WaitGroup + for g := 0; g < 4; g++ { + wg.Add(1) + go func() { + defer wg.Done() + <-start + cb.IsAllowed() + }() + } + wg.Add(1) + go func() { defer wg.Done(); <-start; cb.RecordFailure() }() // re-opens + close(start) + wg.Wait() + + if State(cb.state.Load()) == StateOpen { + if r := cb.Stats().Requests; r > 0 { + t.Fatalf("round %d: reservation survived the half-open -> open transition (requests = %d)", i, r) + } + } + } +} + +// TestAllowReportsReservation pins the Allow contract: a closed-state +// admission reserves nothing, a half-open admission reserves one bounded +// probe slot, and a denied request reserves nothing — callers use the +// reserved flag to decide whether a later ReleaseHalfOpen is theirs to call, +// so a closed-state admission that outlives a later open -> half-open +// transition cannot free a slot a real recovery probe is holding. +func TestAllowReportsReservation(t *testing.T) { + cb := New(Config{ + FailureThreshold: 1, + SuccessThreshold: 1, + MaxHalfOpenRequests: 1, + OpenTimeout: 30 * time.Millisecond, + }) + + if allowed, reserved := cb.Allow(); !allowed || reserved { + t.Fatalf("closed: Allow() = (%v, %v), want (true, false)", allowed, reserved) + } + + cb.RecordFailure() + if allowed, reserved := cb.Allow(); allowed || reserved { + t.Fatalf("open: Allow() = (%v, %v), want (false, false)", allowed, reserved) + } + + time.Sleep(50 * time.Millisecond) + if allowed, reserved := cb.Allow(); !allowed || !reserved { + t.Fatalf("half-open: Allow() = (%v, %v), want (true, true)", allowed, reserved) + } + if allowed, reserved := cb.Allow(); allowed || reserved { + t.Fatalf("half-open budget exhausted: Allow() = (%v, %v), want (false, false)", allowed, reserved) + } +} + +// TestExecuteClosedAdmissionDoesNotFreeProbeSlot pins Execute's slot +// accounting: work admitted while the breaker is CLOSED reserves no +// half-open slot, so when it finishes after other failures moved the +// breaker to half-open, its success must count toward closing WITHOUT +// releasing the slot a real recovery probe is holding. +func TestExecuteClosedAdmissionDoesNotFreeProbeSlot(t *testing.T) { + cb := New(Config{ + FailureThreshold: 1, + SuccessThreshold: 2, // one success does not close the circuit + MaxHalfOpenRequests: 1, + OpenTimeout: 30 * time.Millisecond, + }) + + err := cb.Execute(func() error { + // While the closed-admitted work runs: a failure opens the circuit, + // the grace period elapses, and a recovery probe reserves the only + // half-open slot. + cb.RecordFailure() + time.Sleep(50 * time.Millisecond) + if allowed, reserved := cb.Allow(); !allowed || !reserved { + t.Fatal("setup: expected to reserve the half-open probe slot") + } + return nil + }) + if err != nil { + t.Fatalf("Execute: %v", err) + } + + if cb.IsAllowed() { + t.Error("a second half-open admission succeeded — Execute released a slot it never reserved") + } +} + +// TestRecordFailureResetRaceKeepsTimestamp hammers RecordFailure against +// Reset: when Reset fully completes between RecordFailure's timestamp store +// and its CAS into Open, the circuit must not end up Open with a zero +// lastFailure — CheckState's zero-timestamp guard would then never allow the +// open -> half-open transition, wedging the breaker open for callers with no +// out-of-band probe traffic. +func TestRecordFailureResetRaceKeepsTimestamp(t *testing.T) { + for round := 0; round < 5000; round++ { + cb := New(Config{ + FailureThreshold: 1, + SuccessThreshold: 1, + MaxHalfOpenRequests: 1, + OpenTimeout: time.Minute, + }) + done := make(chan struct{}) + go func() { + defer close(done) + for i := 0; i < 200; i++ { + cb.Reset() + } + }() + for i := 0; i < 200; i++ { + cb.RecordFailure() + } + <-done + + // Quiescent: whatever interleaving happened, an Open circuit must + // carry a non-zero timestamp or it can never leave Open. + if State(cb.state.Load()) == StateOpen && cb.lastFailure.Load() == 0 { + t.Fatalf("round %d: circuit open with lastFailure == 0 — wedged past the zero-timestamp guard", round) + } + } +} diff --git a/internal/failuredetector/failure_detector.go b/internal/failuredetector/failure_detector.go new file mode 100644 index 0000000000..cb0e44a600 --- /dev/null +++ b/internal/failuredetector/failure_detector.go @@ -0,0 +1,338 @@ +// Package failuredetector provides primitives for deciding when a Redis +// database is unhealthy enough that the multi-database client should trigger +// a failover. +package failuredetector + +import ( + "context" + "errors" + "strings" + "sync/atomic" + "time" + + "github.com/redis/go-redis/v9/internal/pool" + "github.com/redis/go-redis/v9/internal/proto" +) + +// txFailedErr mirrors redis.TxFailedErr (the root package cannot be imported +// from internal packages): the sentinel Exec returns when WATCH detected a +// concurrent write. +const txFailedErr = proto.RedisError("redis: transaction failed") + +// FailureDetector decides when failover should be triggered based on a stream +// of command outcomes observed by the caller. +type FailureDetector interface { + // RecordSuccess records a successful command outcome. + RecordSuccess() + // RecordFailure records a failed command outcome. Implementations may + // ignore errors that are not health signals (for example client-side + // context cancellation). + RecordFailure(err error) + // ShouldFailover returns true when the recent outcomes indicate that + // failover should be triggered. + ShouldFailover() bool + // Reset discards all observed outcomes and starts fresh. + Reset() +} + +// CommandFailureDetectorConfig configures CommandFailureDetector. Every +// field has a documented default that NewCommandFailureDetector applies when +// the field is left at its zero value, so a zero-valued config is a valid +// way to ask for the recommended defaults. +type CommandFailureDetectorConfig struct { + // MinNumFailures is the minimum number of failed commands that must be + // observed within the detection window before failover is considered. + // Ignored when IgnoreMinNumFailures is true. + // Default: 1000. + MinNumFailures uint64 + + // IgnoreMinNumFailures disables the MinNumFailures check, so ShouldFailover + // considers only FailureRateThreshold. Use this when the rate alone is the + // signal you trust (typically combined with a small FailureRateThreshold). + IgnoreMinNumFailures bool + + // FailureRateThreshold is the failure rate (0.0-1.0] that, together with + // MinNumFailures, triggers failover. For example, 0.1 means failover when + // 10% or more of the commands in the window fail. + // Ignored when IgnoreFailureRateThreshold is true. + // Default: 0.1. (A zero value means "use the default".) + FailureRateThreshold float64 + + // IgnoreFailureRateThreshold disables the FailureRateThreshold check, so + // ShouldFailover considers only MinNumFailures. Use this when the absolute + // number of failures is the signal you trust regardless of traffic volume. + IgnoreFailureRateThreshold bool + + // FailureDetectionWindow is the sliding time window over which command + // outcomes are considered. Outcomes older than FailureDetectionWindow + // from now are no longer counted by ShouldFailover. + // Default: 2 seconds. + FailureDetectionWindow time.Duration + + // NumBuckets controls the time resolution of the sliding window. The + // window is divided into NumBuckets sub-buckets, each of width + // FailureDetectionWindow / NumBuckets, and outcomes age out one bucket + // at a time. Larger values give finer-grained ageing at the cost of + // O(NumBuckets) work per ShouldFailover call. + // Default: 10. + NumBuckets int +} + +// DefaultCommandFailureDetectorConfig returns the default configuration. +// NewCommandFailureDetector applies the same defaults to any zero-valued +// field, so this is mainly useful as a starting point for tuning. +func DefaultCommandFailureDetectorConfig() CommandFailureDetectorConfig { + return CommandFailureDetectorConfig{ + MinNumFailures: defaultMinNumFailures, + FailureRateThreshold: defaultFailureRateThreshold, + FailureDetectionWindow: defaultFailureDetectionWindow, + NumBuckets: defaultNumBuckets, + } +} + +const ( + defaultMinNumFailures = 1000 + defaultFailureRateThreshold = 0.1 + defaultFailureDetectionWindow = 2 * time.Second + defaultNumBuckets = 10 +) + +// applyDefaults fills zero-valued fields with their documented defaults so +// the rest of the detector can assume every threshold is set. +func (c *CommandFailureDetectorConfig) applyDefaults() { + if c.MinNumFailures == 0 { + c.MinNumFailures = defaultMinNumFailures + } + if c.FailureRateThreshold <= 0 { + c.FailureRateThreshold = defaultFailureRateThreshold + } + if c.FailureDetectionWindow <= 0 { + c.FailureDetectionWindow = defaultFailureDetectionWindow + } + if c.NumBuckets <= 0 { + c.NumBuckets = defaultNumBuckets + } +} + +// bucket holds the outcomes recorded inside a single sub-bucket of the +// sliding window. All fields are accessed atomically so the detector is +// lock-free on the hot path. +// +// Each slot holds a pointer to an immutable-epoch bucketState. When the ring +// wraps around and a writer revisits a slot whose state belongs to a previous +// lap, the writer installs a fresh zeroed bucketState via CompareAndSwap on +// the pointer. Readers ignore any state whose epoch falls outside the current +// window. +type bucket struct { + state atomic.Pointer[bucketState] +} + +// bucketState is one lap of a ring slot: a fixed epoch plus the counters +// recorded during that lap. Lap transitions swap the whole state pointer, so +// a writer that obtained a previous lap's state can only increment that stale +// lap (which readers already ignore) — no increment is ever zeroed away, as +// could happen with the earlier claim-then-zero in-place design. +type bucketState struct { + epochNano int64 + successes atomic.Uint64 + failures atomic.Uint64 +} + +// CommandFailureDetector observes command outcomes inside a sliding time +// window and reports when failover should be triggered. The implementation +// uses a fixed-size ring of buckets and only sync/atomic operations on the +// hot path, so RecordSuccess and RecordFailure scale across many goroutines +// without contention. +type CommandFailureDetector struct { + config CommandFailureDetectorConfig + buckets []bucket + bucketWidthNano int64 + windowNano int64 + now func() time.Time // injectable for tests +} + +// NewCommandFailureDetector creates a new sliding-window failure detector +// with the given configuration. Any zero-valued field in config is replaced +// with its documented default, so passing the zero value is equivalent to +// passing DefaultCommandFailureDetectorConfig(). +func NewCommandFailureDetector(config CommandFailureDetectorConfig) *CommandFailureDetector { + config.applyDefaults() + // Clamp to at least one nanosecond so bucketFor never divides by zero + // when a caller picks a window shorter than NumBuckets nanoseconds. + bucketWidthNano := int64(config.FailureDetectionWindow) / int64(config.NumBuckets) + if bucketWidthNano < 1 { + bucketWidthNano = 1 + } + return &CommandFailureDetector{ + config: config, + buckets: make([]bucket, config.NumBuckets), + bucketWidthNano: bucketWidthNano, + windowNano: int64(config.FailureDetectionWindow), + now: time.Now, + } +} + +// RecordSuccess records a successful command outcome. +func (d *CommandFailureDetector) RecordSuccess() { + d.bucketFor(d.now().UnixNano()).successes.Add(1) +} + +// RecordFailure records a failed command outcome. A nil error is treated as +// a no-op (so callers that forward errors unconditionally do not accumulate +// phantom failures); context cancellation and deadline-exceeded errors are +// also ignored because they originate on the client side and are not a +// signal about the database's health. +func (d *CommandFailureDetector) RecordFailure(err error) { + if err == nil { + return + } + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + return + } + if errors.Is(err, pool.ErrPoolTimeout) || errors.Is(err, pool.ErrPoolExhausted) { + // Local pool saturation: the command never reached the database, so + // this is no verdict on its health — an undersized pool on a busy + // client must not drive failover. + return + } + if errors.Is(err, proto.Nil) || errors.Is(err, txFailedErr) { + // redis.Nil (key missing) and an optimistic-locking transaction + // abort are well-formed server replies — proof of a healthy + // database. Count them as successes so miss-heavy or contended + // workloads cannot trip the failure rate. + d.bucketFor(d.now().UnixNano()).successes.Add(1) + return + } + var reply redisReply + if errors.As(err, &reply) && !isAvailabilityReply(reply.Error()) { + // Any other well-formed server reply (WRONGTYPE, BUSYGROUP, + // NOSCRIPT, ...) is an application-level error from a database that + // processed the command: proof of health, not a failure — except + // the availability replies (LOADING, CLUSTERDOWN, ...) that signal + // a database unable to serve. + d.bucketFor(d.now().UnixNano()).successes.Add(1) + return + } + d.bucketFor(d.now().UnixNano()).failures.Add(1) +} + +// redisReply matches any well-formed server error reply. The concrete +// proto.RedisError string only covers replies the reader does not recognize: +// known prefixes are parsed into typed structs (*proto.LoadingError, +// *proto.AuthError, *proto.MovedError, ...) that share just the RedisError() +// marker — matching the concrete string type alone would misclassify every +// typed reply as a transport failure. +type redisReply interface { + error + RedisError() +} + +// isAvailabilityReply matches server replies that indicate the database +// cannot currently serve traffic (mirroring the root package's retryable +// reply classification, which cannot be imported from here). +func isAvailabilityReply(s string) bool { + for _, prefix := range []string{ + "LOADING ", "READONLY ", "CLUSTERDOWN ", "TRYAGAIN ", + "MASTERDOWN ", "NOREPLICAS ", "ERR max number of clients", + } { + if strings.HasPrefix(s, prefix) { + return true + } + } + // A write script hitting a read-only replica embeds READONLY inside the + // script error instead of at the prefix; mirror the root classifier's + // substring match so EVAL-heavy workloads see the same verdict. + return strings.Contains(s, "-READONLY You can't write against a read only replica") +} + +// ShouldFailover returns true when the outcomes observed within the trailing +// FailureDetectionWindow indicate that failover should be triggered. A +// database is considered faulty when at least MinNumFailures commands have +// failed AND the observed failure rate is at least FailureRateThreshold. +// Either half of the check can be disabled by setting IgnoreMinNumFailures +// or IgnoreFailureRateThreshold; when both are disabled, any single failure +// in the window triggers failover. +// At least one failure must have been observed for failover to be considered. +func (d *CommandFailureDetector) ShouldFailover() bool { + successes, failures := d.snapshot() + + if failures == 0 { + return false + } + if !d.config.IgnoreMinNumFailures && failures < d.config.MinNumFailures { + return false + } + if d.config.IgnoreFailureRateThreshold { + return true + } + + total := successes + failures + failureRate := float64(failures) / float64(total) + return failureRate >= d.config.FailureRateThreshold +} + +// Reset discards all recorded outcomes. Concurrent recorders may race with +// Reset; in the worst case a small number of in-flight increments survive +// the reset, which is acceptable for a failure detector. +func (d *CommandFailureDetector) Reset() { + for i := range d.buckets { + d.buckets[i].state.Store(nil) + } +} + +// Stats returns a read-only snapshot of the outcomes observed within the +// current sliding window. The returned counts are aggregated across the +// bucket ring and reflect the same view of state used by ShouldFailover. +func (d *CommandFailureDetector) Stats() (successes, failures uint64) { + return d.snapshot() +} + +// bucketFor returns the bucket that owns the supplied nanosecond timestamp, +// initialising it (resetting counters and stamping the new epoch) if a +// previous lap of the ring left stale data in that slot. +func (d *CommandFailureDetector) bucketFor(nowNano int64) *bucketState { + bucketStart := nowNano - (nowNano % d.bucketWidthNano) + idx := (bucketStart / d.bucketWidthNano) % int64(len(d.buckets)) + b := &d.buckets[idx] + + for { + st := b.state.Load() + if st != nil { + if st.epochNano == bucketStart { + return st + } + if st.epochNano > bucketStart { + // Clock skew or a concurrent writer already moved this slot + // past the current instant; tolerate it. + return st + } + } + fresh := &bucketState{epochNano: bucketStart} + if b.state.CompareAndSwap(st, fresh) { + return fresh + } + // Another writer won the race; reload and decide again. + } +} + +// snapshot sums the outcomes across every bucket whose time slot overlaps +// the trailing window. A bucket spans [epoch, epoch + bucketWidth) and is +// included when (epoch + bucketWidth) > (now - window), i.e. when +// epoch > now - window - bucketWidth. The cutoff is precomputed below. +// +// The sum is not atomic across buckets, which is acceptable for a failure +// detector: a snapshot can interleave with concurrent writers, but the +// aggregated counts only ever undercount the true value by at most the +// in-flight writes. +func (d *CommandFailureDetector) snapshot() (successes, failures uint64) { + nowNano := d.now().UnixNano() + cutoff := nowNano - d.windowNano - d.bucketWidthNano + for i := range d.buckets { + st := d.buckets[i].state.Load() + if st != nil && st.epochNano > cutoff { + successes += st.successes.Load() + failures += st.failures.Load() + } + } + return successes, failures +} diff --git a/internal/failuredetector/failure_detector_test.go b/internal/failuredetector/failure_detector_test.go new file mode 100644 index 0000000000..41746dcb07 --- /dev/null +++ b/internal/failuredetector/failure_detector_test.go @@ -0,0 +1,697 @@ +package failuredetector + +import ( + "context" + "errors" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/redis/go-redis/v9/internal/pool" + "github.com/redis/go-redis/v9/internal/proto" +) + +// withFakeClock returns the detector with a hand-driven clock so tests can +// advance time without sleeping. The returned closure shifts the clock by +// the given duration. Use this for any test that depends on window expiry. +func withFakeClock(fd *CommandFailureDetector, start time.Time) (advance func(time.Duration)) { + cur := start + fd.now = func() time.Time { return cur } + return func(d time.Duration) { cur = cur.Add(d) } +} + +func TestCommandFailureDetector_ShouldFailover(t *testing.T) { + config := CommandFailureDetectorConfig{ + MinNumFailures: 10, + FailureRateThreshold: 0.5, + FailureDetectionWindow: time.Hour, + } + fd := NewCommandFailureDetector(config) + + // Not enough failures + for i := 0; i < 5; i++ { + fd.RecordFailure(errors.New("error")) + } + if fd.ShouldFailover() { + t.Error("should not failover with insufficient failures") + } + + // Add more failures to reach threshold + for i := 0; i < 5; i++ { + fd.RecordFailure(errors.New("error")) + } + if !fd.ShouldFailover() { + t.Error("should failover with 100% failure rate") + } +} + +func TestCommandFailureDetector_SuccessResetsRate(t *testing.T) { + config := CommandFailureDetectorConfig{ + MinNumFailures: 10, + FailureRateThreshold: 0.5, + FailureDetectionWindow: time.Hour, + } + fd := NewCommandFailureDetector(config) + + // Record failures + for i := 0; i < 10; i++ { + fd.RecordFailure(errors.New("error")) + } + + // Record successes to bring rate below threshold + for i := 0; i < 15; i++ { + fd.RecordSuccess() + } + + // 10 failures / 25 total = 40% < 50% + if fd.ShouldFailover() { + t.Error("should not failover when failure rate is below threshold") + } +} + +func TestCommandFailureDetector_IgnoresContextErrors(t *testing.T) { + config := CommandFailureDetectorConfig{ + MinNumFailures: 5, + FailureRateThreshold: 0.5, + FailureDetectionWindow: time.Hour, + } + fd := NewCommandFailureDetector(config) + + // Record context errors - should be ignored + for i := 0; i < 10; i++ { + fd.RecordFailure(context.Canceled) + fd.RecordFailure(context.DeadlineExceeded) + } + + // Record some successes + for i := 0; i < 10; i++ { + fd.RecordSuccess() + } + + // Only successes should be counted + successes, failures := fd.Stats() + if failures != 0 { + t.Errorf("expected 0 failures, got %d", failures) + } + if successes != 10 { + t.Errorf("expected 10 successes, got %d", successes) + } +} + +func TestCommandFailureDetector_IgnoresNilError(t *testing.T) { + config := CommandFailureDetectorConfig{ + MinNumFailures: 1, + FailureRateThreshold: 0.0, + FailureDetectionWindow: time.Hour, + } + fd := NewCommandFailureDetector(config) + + // A nil error represents success and must not be counted as a failure. + for i := 0; i < 10; i++ { + fd.RecordFailure(nil) + } + + _, failures := fd.Stats() + if failures != 0 { + t.Errorf("expected 0 failures for nil errors, got %d", failures) + } + if fd.ShouldFailover() { + t.Error("should not failover when only nil errors were recorded") + } +} + +func TestCommandFailureDetector_IgnoresPoolSaturation(t *testing.T) { + config := CommandFailureDetectorConfig{ + MinNumFailures: 1, + FailureRateThreshold: 0.0, + FailureDetectionWindow: time.Hour, + } + fd := NewCommandFailureDetector(config) + + // Pool timeouts/exhaustion are client-side saturation, not a verdict on + // the database: a busy client with an undersized pool must not trigger + // failover away from a healthy member. + for i := 0; i < 10; i++ { + fd.RecordFailure(pool.ErrPoolTimeout) + fd.RecordFailure(pool.ErrPoolExhausted) + } + + if _, failures := fd.Stats(); failures != 0 { + t.Errorf("expected 0 failures for pool saturation errors, got %d", failures) + } + if fd.ShouldFailover() { + t.Error("should not failover on local pool saturation") + } +} + +func TestCommandFailureDetector_TreatsReplyErrorsAsSuccess(t *testing.T) { + config := CommandFailureDetectorConfig{ + MinNumFailures: 1, + FailureRateThreshold: 0.0, + FailureDetectionWindow: time.Hour, + } + fd := NewCommandFailureDetector(config) + + // Application-level replies prove the database processed the command; + // availability replies prove it cannot serve. Only the latter count as + // failures. + for _, e := range []string{"WRONGTYPE Operation against a key", "BUSYGROUP Consumer Group name already exists", "NOSCRIPT No matching script"} { + fd.RecordFailure(proto.RedisError(e)) + } + if _, failures := fd.Stats(); failures != 0 { + t.Errorf("expected 0 failures for application replies, got %d", failures) + } + if fd.ShouldFailover() { + t.Error("should not failover on application-level reply errors") + } + + fd.RecordFailure(proto.RedisError("LOADING Redis is loading the dataset in memory")) + if _, failures := fd.Stats(); failures != 1 { + t.Errorf("expected 1 failure for the LOADING reply, got %d", failures) + } +} + +func TestCommandFailureDetector_TreatsTypedReplyErrorsAsSuccess(t *testing.T) { + config := CommandFailureDetectorConfig{ + MinNumFailures: 1, + FailureRateThreshold: 0.0, + FailureDetectionWindow: time.Hour, + } + fd := NewCommandFailureDetector(config) + + // The proto reader parses recognized reply prefixes into typed structs + // (*proto.AuthError, *proto.MovedError, ...) rather than the concrete + // proto.RedisError string — they are still well-formed server replies and + // must classify exactly like their string forms: application-level + // replies as successes, availability replies as failures. + for _, e := range []error{ + proto.NewAuthError("NOAUTH Authentication required"), + proto.NewPermissionError("NOPERM this user has no permissions"), + proto.NewExecAbortError("EXECABORT Transaction discarded because of previous errors"), + proto.NewMovedError("MOVED 3999 127.0.0.1:6381", "127.0.0.1:6381"), + } { + fd.RecordFailure(e) + } + successes, failures := fd.Stats() + if failures != 0 { + t.Errorf("expected 0 failures for typed application replies, got %d", failures) + } + if successes != 4 { + t.Errorf("expected typed application replies to count as successes, got %d", successes) + } + + fd.RecordFailure(proto.NewLoadingError("LOADING Redis is loading the dataset in memory")) + fd.RecordFailure(proto.NewClusterDownError("CLUSTERDOWN The cluster is down")) + if _, failures := fd.Stats(); failures != 2 { + t.Errorf("expected 2 failures for typed availability replies, got %d", failures) + } +} + +func TestCommandFailureDetector_TreatsLuaReadOnlyAsFailure(t *testing.T) { + config := CommandFailureDetectorConfig{ + MinNumFailures: 1, + FailureRateThreshold: 0.0, + FailureDetectionWindow: time.Hour, + } + fd := NewCommandFailureDetector(config) + + // A write script hitting a read-only replica embeds -READONLY inside the + // script error rather than at the prefix; the root retry classifier + // matches that substring, and the detector must agree — a member serving + // EVAL writes from a replica is an availability problem, not proof of + // health. + fd.RecordFailure(proto.RedisError( + "ERR Error running script (call to f_6b1bf486c81ceb7edf3c093f0fb1291d6438a1cc): " + + "@user_script:1: @user_script: 1: -READONLY You can't write against a read only replica.", + )) + if _, failures := fd.Stats(); failures != 1 { + t.Errorf("expected 1 failure for the Lua-embedded READONLY reply, got %d", failures) + } +} + +func TestCommandFailureDetector_TreatsTxFailedAsSuccess(t *testing.T) { + config := CommandFailureDetectorConfig{ + MinNumFailures: 1, + FailureRateThreshold: 0.0, + FailureDetectionWindow: time.Hour, + } + fd := NewCommandFailureDetector(config) + + // An optimistic-locking abort (WATCH saw a concurrent write) is a + // well-formed server reply: contended workloads must not trip failover. + txFailed := proto.RedisError("redis: transaction failed") + for i := 0; i < 10; i++ { + fd.RecordFailure(txFailed) + } + + successes, failures := fd.Stats() + if failures != 0 { + t.Errorf("expected 0 failures for tx aborts, got %d", failures) + } + if successes != 10 { + t.Errorf("expected tx aborts to count as success, got %d", successes) + } + if fd.ShouldFailover() { + t.Error("should not failover on optimistic-locking contention") + } +} + +func TestCommandFailureDetector_TreatsRedisNilAsSuccess(t *testing.T) { + config := CommandFailureDetectorConfig{ + MinNumFailures: 1, + FailureRateThreshold: 0.0, + FailureDetectionWindow: time.Hour, + } + fd := NewCommandFailureDetector(config) + + // redis.Nil (proto.Nil) is a well-formed server reply — a cache miss is + // proof of a healthy database. A miss-heavy workload must not trip the + // detector. + for i := 0; i < 10; i++ { + fd.RecordFailure(proto.Nil) + } + + successes, failures := fd.Stats() + if failures != 0 { + t.Errorf("expected 0 failures for redis.Nil, got %d", failures) + } + if successes != 10 { + t.Errorf("expected redis.Nil to count as success, got %d", successes) + } + if fd.ShouldFailover() { + t.Error("should not failover on a miss-heavy workload") + } +} + +func TestCommandFailureDetector_DefaultsWindowWhenUnset(t *testing.T) { + // A zero window must fall back to the documented default so the counters + // are not reset on every call (which would prevent failover entirely). + config := CommandFailureDetectorConfig{ + MinNumFailures: 1, + FailureRateThreshold: 0.0, + } + fd := NewCommandFailureDetector(config) + + fd.RecordFailure(errors.New("error")) + if !fd.ShouldFailover() { + t.Error("should failover after a failure with the defaulted window") + } + + _, failures := fd.Stats() + if failures != 1 { + t.Errorf("expected 1 failure to be retained within the window, got %d", failures) + } +} + +func TestCommandFailureDetector_WindowExpiry(t *testing.T) { + // Outcomes older than the window must not be counted. The sliding + // implementation drops them one bucket at a time, so by advancing time + // past the full window every previously-recorded outcome should age out. + config := CommandFailureDetectorConfig{ + MinNumFailures: 5, + FailureRateThreshold: 0.5, + FailureDetectionWindow: time.Second, + NumBuckets: 10, + } + fd := NewCommandFailureDetector(config) + advance := withFakeClock(fd, time.Unix(1_700_000_000, 0)) + + for i := 0; i < 10; i++ { + fd.RecordFailure(errors.New("error")) + } + if _, failures := fd.Stats(); failures != 10 { + t.Fatalf("expected 10 failures before expiry, got %d", failures) + } + + // Step past the window by one full bucket width so every bucket falls + // outside the trailing window. + advance(config.FailureDetectionWindow + config.FailureDetectionWindow/time.Duration(config.NumBuckets)) + + fd.RecordSuccess() + + successes, failures := fd.Stats() + if failures != 0 { + t.Errorf("expected 0 failures after window expiry, got %d", failures) + } + if successes != 1 { + t.Errorf("expected 1 success after window expiry, got %d", successes) + } +} + +func TestCommandFailureDetector_Reset(t *testing.T) { + config := CommandFailureDetectorConfig{ + MinNumFailures: 5, + FailureRateThreshold: 0.5, + FailureDetectionWindow: time.Hour, + } + fd := NewCommandFailureDetector(config) + + // Record some activity + for i := 0; i < 10; i++ { + fd.RecordFailure(errors.New("error")) + fd.RecordSuccess() + } + + fd.Reset() + + successes, failures := fd.Stats() + if failures != 0 || successes != 0 { + t.Errorf("expected 0/0 after reset, got %d/%d", successes, failures) + } +} + +func TestCommandFailureDetector_AppliesDefaultsForZeroFields(t *testing.T) { + // A zero-valued config must be equivalent to passing the documented + // defaults; reviewers flagged that the field comments promised defaults + // the constructor did not apply. + fd := NewCommandFailureDetector(CommandFailureDetectorConfig{}) + defaults := DefaultCommandFailureDetectorConfig() + + if fd.config.MinNumFailures != defaults.MinNumFailures { + t.Errorf("MinNumFailures: got %d, want %d (default)", + fd.config.MinNumFailures, defaults.MinNumFailures) + } + if fd.config.FailureRateThreshold != defaults.FailureRateThreshold { + t.Errorf("FailureRateThreshold: got %v, want %v (default)", + fd.config.FailureRateThreshold, defaults.FailureRateThreshold) + } + if fd.config.FailureDetectionWindow != defaults.FailureDetectionWindow { + t.Errorf("FailureDetectionWindow: got %v, want %v (default)", + fd.config.FailureDetectionWindow, defaults.FailureDetectionWindow) + } + if fd.config.NumBuckets != defaults.NumBuckets { + t.Errorf("NumBuckets: got %d, want %d (default)", + fd.config.NumBuckets, defaults.NumBuckets) + } +} + +func TestCommandFailureDetector_PreservesExplicitValues(t *testing.T) { + // Defaults must not overwrite explicit non-zero settings, otherwise the + // detector would be impossible to tune away from the defaults. + config := CommandFailureDetectorConfig{ + MinNumFailures: 5, + FailureRateThreshold: 0.25, + FailureDetectionWindow: 500 * time.Millisecond, + NumBuckets: 4, + } + fd := NewCommandFailureDetector(config) + if fd.config != config { + t.Errorf("explicit config was overwritten: got %+v, want %+v", fd.config, config) + } +} + +func TestCommandFailureDetector_DoesNotPanicOnTinyWindow(t *testing.T) { + // A FailureDetectionWindow shorter than NumBuckets nanoseconds would + // previously produce bucketWidthNano=0 and panic on the first record + // via "%". The constructor must clamp the bucket width. + fd := NewCommandFailureDetector(CommandFailureDetectorConfig{ + FailureDetectionWindow: 5 * time.Nanosecond, + NumBuckets: 10, + }) + // Both hot-path entry points must not panic. + fd.RecordSuccess() + fd.RecordFailure(errors.New("error")) + _ = fd.ShouldFailover() +} + +func TestCommandFailureDetector_IgnoreMinNumFailures(t *testing.T) { + // With IgnoreMinNumFailures set, ShouldFailover should ignore the count + // threshold and decide purely on the failure rate. + fd := NewCommandFailureDetector(CommandFailureDetectorConfig{ + MinNumFailures: 10_000, // would otherwise gate every reasonable burst + IgnoreMinNumFailures: true, + FailureRateThreshold: 0.5, + FailureDetectionWindow: time.Hour, + }) + + // 1 failure out of 1 command => 100% rate, well above the threshold. + fd.RecordFailure(errors.New("error")) + if !fd.ShouldFailover() { + t.Error("should failover when rate exceeds threshold and count is ignored") + } + + // Drive the rate below the threshold with successes; the count threshold + // is still ignored, but the rate check must now reject. + for i := 0; i < 10; i++ { + fd.RecordSuccess() + } + if fd.ShouldFailover() { + t.Error("should not failover when rate drops below threshold") + } +} + +func TestCommandFailureDetector_IgnoreFailureRateThreshold(t *testing.T) { + // With IgnoreFailureRateThreshold set, ShouldFailover should ignore the + // rate threshold and decide purely on the absolute failure count. + fd := NewCommandFailureDetector(CommandFailureDetectorConfig{ + MinNumFailures: 3, + FailureRateThreshold: 0.99, // would otherwise be impossible to meet + IgnoreFailureRateThreshold: true, + FailureDetectionWindow: time.Hour, + }) + + // A small failure rate but below the count threshold => no failover. + fd.RecordFailure(errors.New("error")) + fd.RecordFailure(errors.New("error")) + for i := 0; i < 100; i++ { + fd.RecordSuccess() + } + if fd.ShouldFailover() { + t.Error("should not failover before reaching the failure count") + } + + // Reaching the count threshold triggers failover regardless of the rate. + fd.RecordFailure(errors.New("error")) + if !fd.ShouldFailover() { + t.Error("should failover once the failure count is reached") + } +} + +func TestCommandFailureDetector_IgnoreBothThresholds(t *testing.T) { + // With both thresholds ignored, any single failure in the window should + // be enough to trigger failover. + fd := NewCommandFailureDetector(CommandFailureDetectorConfig{ + MinNumFailures: 10_000, + IgnoreMinNumFailures: true, + FailureRateThreshold: 0.99, + IgnoreFailureRateThreshold: true, + FailureDetectionWindow: time.Hour, + }) + + if fd.ShouldFailover() { + t.Error("should not failover before any failure is recorded") + } + + fd.RecordFailure(errors.New("error")) + if !fd.ShouldFailover() { + t.Error("any single failure should trigger when both thresholds are ignored") + } +} + +func TestCommandFailureDetector_RequiresBothThresholds(t *testing.T) { + // Both thresholds must be met when both are configured. + config := CommandFailureDetectorConfig{ + MinNumFailures: 5, + FailureRateThreshold: 0.5, + FailureDetectionWindow: time.Hour, + } + fd := NewCommandFailureDetector(config) + + // 5 failures reaches the count, but 5/20 = 25% is below the rate. + for i := 0; i < 5; i++ { + fd.RecordFailure(errors.New("error")) + } + for i := 0; i < 15; i++ { + fd.RecordSuccess() + } + if fd.ShouldFailover() { + t.Error("should not failover when only the count threshold is met") + } +} + +// TestCommandFailureDetector_SlidingWindow exercises the property that +// distinguishes a sliding window from a tumbling one: outcomes age out one +// bucket at a time as time advances, rather than disappearing en masse at a +// fixed window boundary. +func TestCommandFailureDetector_SlidingWindow(t *testing.T) { + config := CommandFailureDetectorConfig{ + MinNumFailures: 1, + FailureRateThreshold: 0.0, + FailureDetectionWindow: time.Second, + NumBuckets: 10, + } + fd := NewCommandFailureDetector(config) + advance := withFakeClock(fd, time.Unix(1_700_000_000, 0)) + + bucket := config.FailureDetectionWindow / time.Duration(config.NumBuckets) + + // Spread one failure across each of the first three buckets. + fd.RecordFailure(errors.New("e")) + advance(bucket) + fd.RecordFailure(errors.New("e")) + advance(bucket) + fd.RecordFailure(errors.New("e")) + + if _, failures := fd.Stats(); failures != 3 { + t.Fatalf("expected 3 failures in window, got %d", failures) + } + + // Step time so the very first bucket falls outside the trailing window. + // In a tumbling implementation, advancing by < window would change + // nothing; in a sliding implementation, the oldest failure must have + // aged out. + advance(config.FailureDetectionWindow - bucket) + if _, failures := fd.Stats(); failures != 2 { + t.Fatalf("expected 2 failures after oldest bucket aged out, got %d", failures) + } + + // One more bucket-width drops the next oldest failure. + advance(bucket) + if _, failures := fd.Stats(); failures != 1 { + t.Fatalf("expected 1 failure after second bucket aged out, got %d", failures) + } + + // And once we walk past the full window, everything is gone. + advance(config.FailureDetectionWindow) + if _, failures := fd.Stats(); failures != 0 { + t.Fatalf("expected 0 failures after full window elapsed, got %d", failures) + } +} + +// TestCommandFailureDetector_ConcurrentRecord checks that the lock-free +// hot path doesn't lose counts under contention. We fan out N goroutines +// each recording a fixed number of successes and failures and assert the +// totals match. The window is long enough that no bucket rotates during +// the test, so the count must be exact. +func TestCommandFailureDetector_ConcurrentRecord(t *testing.T) { + const ( + numGoroutines = 32 + opsPerGoroutine = 5_000 + expectedSuccesses = numGoroutines * opsPerGoroutine + expectedFailures = numGoroutines * opsPerGoroutine + ) + + fd := NewCommandFailureDetector(CommandFailureDetectorConfig{ + FailureDetectionWindow: time.Hour, + NumBuckets: 10, + }) + + var wg sync.WaitGroup + wg.Add(numGoroutines * 2) + errFail := errors.New("failure") + + for i := 0; i < numGoroutines; i++ { + go func() { + defer wg.Done() + for j := 0; j < opsPerGoroutine; j++ { + fd.RecordSuccess() + } + }() + go func() { + defer wg.Done() + for j := 0; j < opsPerGoroutine; j++ { + fd.RecordFailure(errFail) + } + }() + } + wg.Wait() + + successes, failures := fd.Stats() + if successes != uint64(expectedSuccesses) { + t.Errorf("successes: got %d, want %d", successes, expectedSuccesses) + } + if failures != uint64(expectedFailures) { + t.Errorf("failures: got %d, want %d", failures, expectedFailures) + } +} + +// TestCommandFailureDetector_ConcurrentRecordWithReaders pairs the +// concurrent recorders above with a flock of concurrent readers calling +// ShouldFailover. The test passes if no race is reported (-race) and no +// panic is observed. +func TestCommandFailureDetector_ConcurrentRecordWithReaders(t *testing.T) { + fd := NewCommandFailureDetector(CommandFailureDetectorConfig{ + MinNumFailures: 50, + FailureRateThreshold: 0.5, + FailureDetectionWindow: time.Hour, + NumBuckets: 10, + }) + + var wg sync.WaitGroup + var stop atomic.Bool + errFail := errors.New("failure") + + for i := 0; i < 8; i++ { + wg.Add(1) + go func() { + defer wg.Done() + for !stop.Load() { + fd.RecordSuccess() + fd.RecordFailure(errFail) + } + }() + wg.Add(1) + go func() { + defer wg.Done() + for !stop.Load() { + _ = fd.ShouldFailover() + } + }() + } + + time.Sleep(20 * time.Millisecond) + stop.Store(true) + wg.Wait() +} + +// BenchmarkCommandFailureDetector_RecordSuccess measures the cost of the +// hot path under contention so we can compare future implementations. +func BenchmarkCommandFailureDetector_RecordSuccess(b *testing.B) { + fd := NewCommandFailureDetector(CommandFailureDetectorConfig{ + FailureDetectionWindow: time.Hour, + NumBuckets: 10, + }) + b.ResetTimer() + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + fd.RecordSuccess() + } + }) +} + +// BenchmarkCommandFailureDetector_RecordFailure mirrors RecordSuccess for +// the failure path (same hot-path code, includes the error filter). +func BenchmarkCommandFailureDetector_RecordFailure(b *testing.B) { + fd := NewCommandFailureDetector(CommandFailureDetectorConfig{ + FailureDetectionWindow: time.Hour, + NumBuckets: 10, + }) + err := errors.New("failure") + b.ResetTimer() + b.RunParallel(func(pb *testing.PB) { + for pb.Next() { + fd.RecordFailure(err) + } + }) +} + +// BenchmarkCommandFailureDetector_ShouldFailover measures the read path, +// which sums NumBuckets atomically-loaded counters. +func BenchmarkCommandFailureDetector_ShouldFailover(b *testing.B) { + fd := NewCommandFailureDetector(CommandFailureDetectorConfig{ + MinNumFailures: 100, + FailureRateThreshold: 0.5, + FailureDetectionWindow: time.Hour, + NumBuckets: 10, + }) + // Populate so the rate check branch runs. + for i := 0; i < 1000; i++ { + fd.RecordSuccess() + } + fd.RecordFailure(errors.New("failure")) + b.ResetTimer() + for i := 0; i < b.N; i++ { + _ = fd.ShouldFailover() + } +} diff --git a/internal/multidb/circuit_breaker.go b/internal/multidb/circuit_breaker.go new file mode 100644 index 0000000000..87ff78e82d --- /dev/null +++ b/internal/multidb/circuit_breaker.go @@ -0,0 +1,88 @@ +// Package multidb provides internal components for multi-database support. +package multidb + +import ( + "time" + + "github.com/redis/go-redis/v9/internal/circuitbreaker" +) + +// CircuitState represents the state of a circuit breaker. +// This is an alias to the internal circuitbreaker package. +type CircuitState = circuitbreaker.State + +const ( + // CircuitClosed indicates the circuit is closed and requests are allowed. + CircuitClosed = circuitbreaker.StateClosed + // CircuitOpen indicates the circuit is open and requests are blocked. + CircuitOpen = circuitbreaker.StateOpen + // CircuitHalfOpen indicates the circuit is testing if the service has recovered. + CircuitHalfOpen = circuitbreaker.StateHalfOpen +) + +// CircuitBreakerConfig holds configuration for a circuit breaker. +type CircuitBreakerConfig struct { + // FailureThreshold is the number of failures before opening the circuit. + FailureThreshold int + // SuccessThreshold is the number of successes in half-open state before closing. + SuccessThreshold int + // GracePeriod is how long to wait before transitioning from open to half-open. + // This grace period gives a failed database time to self-heal before it is + // probed again. Default: 60 seconds. + GracePeriod time.Duration +} + +// DefaultCircuitBreakerConfig returns the default circuit breaker configuration. +func DefaultCircuitBreakerConfig() CircuitBreakerConfig { + return CircuitBreakerConfig{ + FailureThreshold: 5, + SuccessThreshold: 2, + GracePeriod: 60 * time.Second, + } +} + +// CircuitBreakerCallback is called when the circuit breaker state changes. +type CircuitBreakerCallback func(oldState, newState CircuitState) + +// CircuitBreaker wraps the internal circuitbreaker.CircuitBreaker. +type CircuitBreaker struct { + *circuitbreaker.CircuitBreaker + config CircuitBreakerConfig +} + +// NewCircuitBreaker creates a new circuit breaker with the given configuration. +// Zero-valued fields are replaced with their defaults before the config is +// stored, so Config() always reports the values the breaker actually runs with. +func NewCircuitBreaker(config CircuitBreakerConfig) *CircuitBreaker { + def := DefaultCircuitBreakerConfig() + if config.FailureThreshold <= 0 { + config.FailureThreshold = def.FailureThreshold + } + if config.SuccessThreshold <= 0 { + config.SuccessThreshold = def.SuccessThreshold + } + if config.GracePeriod <= 0 { + config.GracePeriod = def.GracePeriod + } + return &CircuitBreaker{ + CircuitBreaker: circuitbreaker.New(circuitbreaker.Config{ + FailureThreshold: config.FailureThreshold, + SuccessThreshold: config.SuccessThreshold, + MaxHalfOpenRequests: config.SuccessThreshold, + OpenTimeout: config.GracePeriod, + }), + config: config, + } +} + +// OnStateChange registers a callback to be called when the state changes. +func (cb *CircuitBreaker) OnStateChange(callback CircuitBreakerCallback) { + cb.CircuitBreaker.OnStateChange(func(oldState, newState circuitbreaker.State) { + callback(oldState, newState) + }) +} + +// Config returns the circuit breaker configuration. +func (cb *CircuitBreaker) Config() CircuitBreakerConfig { + return cb.config +} diff --git a/internal/multidb/circuit_breaker_test.go b/internal/multidb/circuit_breaker_test.go new file mode 100644 index 0000000000..1072654f81 --- /dev/null +++ b/internal/multidb/circuit_breaker_test.go @@ -0,0 +1,217 @@ +package multidb + +import ( + "sync" + "testing" + "time" +) + +func TestCircuitBreaker_InitialState(t *testing.T) { + cb := NewCircuitBreaker(DefaultCircuitBreakerConfig()) + + if cb.State() != CircuitClosed { + t.Errorf("expected initial state to be Closed, got %v", cb.State()) + } +} + +func TestCircuitBreaker_OpenAfterFailures(t *testing.T) { + config := CircuitBreakerConfig{ + FailureThreshold: 3, + SuccessThreshold: 2, + GracePeriod: 100 * time.Millisecond, + } + cb := NewCircuitBreaker(config) + + // Record failures + for i := 0; i < 3; i++ { + cb.RecordFailure() + } + + if cb.State() != CircuitOpen { + t.Errorf("expected state to be Open after %d failures, got %v", 3, cb.State()) + } +} + +func TestCircuitBreaker_HalfOpenAfterTimeout(t *testing.T) { + config := CircuitBreakerConfig{ + FailureThreshold: 2, + SuccessThreshold: 1, + GracePeriod: 50 * time.Millisecond, + } + cb := NewCircuitBreaker(config) + + // Open the circuit + cb.RecordFailure() + cb.RecordFailure() + + if cb.State() != CircuitOpen { + t.Fatalf("expected state to be Open, got %v", cb.State()) + } + + // Wait for timeout + time.Sleep(60 * time.Millisecond) + + // CheckState should transition to HalfOpen + state := cb.CheckState() + if state != CircuitHalfOpen { + t.Errorf("expected state to be HalfOpen after timeout, got %v", state) + } +} + +func TestCircuitBreaker_CloseAfterSuccesses(t *testing.T) { + config := CircuitBreakerConfig{ + FailureThreshold: 2, + SuccessThreshold: 2, + GracePeriod: 10 * time.Millisecond, + } + cb := NewCircuitBreaker(config) + + // Open the circuit + cb.RecordFailure() + cb.RecordFailure() + + // Wait for timeout and transition to half-open + time.Sleep(20 * time.Millisecond) + cb.CheckState() + + // Record successes + cb.RecordSuccess() + cb.RecordSuccess() + + if cb.State() != CircuitClosed { + t.Errorf("expected state to be Closed after successes, got %v", cb.State()) + } +} + +func TestCircuitBreaker_ReopenOnFailureInHalfOpen(t *testing.T) { + config := CircuitBreakerConfig{ + FailureThreshold: 2, + SuccessThreshold: 2, + GracePeriod: 10 * time.Millisecond, + } + cb := NewCircuitBreaker(config) + + // Open the circuit + cb.RecordFailure() + cb.RecordFailure() + + // Wait for timeout and transition to half-open + time.Sleep(20 * time.Millisecond) + cb.CheckState() + + // Record failure in half-open + cb.RecordFailure() + + if cb.State() != CircuitOpen { + t.Errorf("expected state to be Open after failure in HalfOpen, got %v", cb.State()) + } +} + +func TestCircuitBreaker_Reset(t *testing.T) { + config := CircuitBreakerConfig{ + FailureThreshold: 2, + SuccessThreshold: 2, + GracePeriod: 1 * time.Hour, + } + cb := NewCircuitBreaker(config) + + // Open the circuit + cb.RecordFailure() + cb.RecordFailure() + + if cb.State() != CircuitOpen { + t.Fatalf("expected state to be Open, got %v", cb.State()) + } + + // Reset + cb.Reset() + + if cb.State() != CircuitClosed { + t.Errorf("expected state to be Closed after reset, got %v", cb.State()) + } +} + +func TestCircuitBreaker_Callbacks(t *testing.T) { + config := CircuitBreakerConfig{ + FailureThreshold: 2, + SuccessThreshold: 1, + GracePeriod: 10 * time.Millisecond, + } + cb := NewCircuitBreaker(config) + + var transitions []struct { + old, new CircuitState + } + var mu sync.Mutex + + cb.OnStateChange(func(old, new CircuitState) { + mu.Lock() + transitions = append(transitions, struct{ old, new CircuitState }{old, new}) + mu.Unlock() + }) + + // Open the circuit + cb.RecordFailure() + cb.RecordFailure() + + // Wait and transition to half-open + time.Sleep(20 * time.Millisecond) + cb.CheckState() + + // Close the circuit + cb.RecordSuccess() + + mu.Lock() + defer mu.Unlock() + + if len(transitions) != 3 { + t.Errorf("expected 3 transitions, got %d", len(transitions)) + } +} + +func TestCircuitBreaker_Concurrent(t *testing.T) { + config := CircuitBreakerConfig{ + FailureThreshold: 100, + SuccessThreshold: 10, + GracePeriod: 1 * time.Hour, + } + cb := NewCircuitBreaker(config) + + var wg sync.WaitGroup + for i := 0; i < 100; i++ { + wg.Add(2) + go func() { + defer wg.Done() + cb.RecordSuccess() + }() + go func() { + defer wg.Done() + cb.RecordFailure() + }() + } + wg.Wait() + + // Should not panic and state should be valid + state := cb.State() + if state != CircuitClosed && state != CircuitOpen { + t.Errorf("unexpected state: %v", state) + } +} + +func TestCircuitState_String(t *testing.T) { + tests := []struct { + state CircuitState + expected string + }{ + {CircuitClosed, "closed"}, + {CircuitOpen, "open"}, + {CircuitHalfOpen, "half-open"}, + {CircuitState(99), "unknown"}, + } + + for _, tt := range tests { + if got := tt.state.String(); got != tt.expected { + t.Errorf("CircuitState(%d).String() = %q, want %q", tt.state, got, tt.expected) + } + } +} diff --git a/internal/otel/metrics.go b/internal/otel/metrics.go index 2e234ba8be..fa6323c8ad 100644 --- a/internal/otel/metrics.go +++ b/internal/otel/metrics.go @@ -86,6 +86,25 @@ type Recorder interface { // consumerName: name of the consumer RecordStreamLag(ctx context.Context, lag time.Duration, cn *pool.Conn, streamName, consumerGroup, consumerName string) + // RecordMultiDBFailover records a MultiDB (geo-failover) failover from one + // member database to another. fromFQDN/toFQDN are host-only database FQDNs, + // reason is "automatic" or "manual", and duration is the wall time of the + // failover. + RecordMultiDBFailover(ctx context.Context, fromFQDN, toFQDN, reason string, duration time.Duration) + + // RecordMultiDBActiveDatabaseChange records a change of the active MultiDB + // member database (failover, fallback, or manual selection). fromFQDN/toFQDN + // are host-only database FQDNs. + RecordMultiDBActiveDatabaseChange(ctx context.Context, fromFQDN, toFQDN string) + + // RecordMultiDBCircuitStateChange records a MultiDB circuit breaker state + // transition for the database identified by dbFQDN. + RecordMultiDBCircuitStateChange(ctx context.Context, dbFQDN, fromState, toState string) + + // RecordMultiDBHealthCheck records the result of a MultiDB health-check pass + // for the database identified by dbFQDN. duration is the wall time of the check. + RecordMultiDBHealthCheck(ctx context.Context, dbFQDN string, success bool, duration time.Duration) + // RecordConnectionCount records a change in connection count (UpDownCounter) // delta: +1 when connection added, -1 when connection removed // state: connection state (e.g., "idle", "used") @@ -252,6 +271,26 @@ func RecordStreamLag(ctx context.Context, lag time.Duration, cn *pool.Conn, stre getRecorder().RecordStreamLag(ctx, lag, cn, streamName, consumerGroup, consumerName) } +// RecordMultiDBFailover records a MultiDB failover from one database to another. +func RecordMultiDBFailover(ctx context.Context, fromFQDN, toFQDN, reason string, duration time.Duration) { + getRecorder().RecordMultiDBFailover(ctx, fromFQDN, toFQDN, reason, duration) +} + +// RecordMultiDBActiveDatabaseChange records a change of the active MultiDB database. +func RecordMultiDBActiveDatabaseChange(ctx context.Context, fromFQDN, toFQDN string) { + getRecorder().RecordMultiDBActiveDatabaseChange(ctx, fromFQDN, toFQDN) +} + +// RecordMultiDBCircuitStateChange records a MultiDB circuit breaker state transition. +func RecordMultiDBCircuitStateChange(ctx context.Context, dbFQDN, fromState, toState string) { + getRecorder().RecordMultiDBCircuitStateChange(ctx, dbFQDN, fromState, toState) +} + +// RecordMultiDBHealthCheck records the result of a MultiDB health-check pass. +func RecordMultiDBHealthCheck(ctx context.Context, dbFQDN string, success bool, duration time.Duration) { + getRecorder().RecordMultiDBHealthCheck(ctx, dbFQDN, success, duration) +} + type noopRecorder struct{} func (noopRecorder) RecordOperationDuration(context.Context, time.Duration, Cmder, int, error, *pool.Conn, int) { @@ -272,6 +311,12 @@ func (noopRecorder) RecordPubSubMessage(context.Context, *pool.Conn, string, str func (noopRecorder) RecordStreamLag(context.Context, time.Duration, *pool.Conn, string, string, string) { } + +func (noopRecorder) RecordMultiDBFailover(context.Context, string, string, string, time.Duration) {} +func (noopRecorder) RecordMultiDBActiveDatabaseChange(context.Context, string, string) {} +func (noopRecorder) RecordMultiDBCircuitStateChange(context.Context, string, string, string) {} +func (noopRecorder) RecordMultiDBHealthCheck(context.Context, string, bool, time.Duration) {} + func (noopRecorder) RecordConnectionCount(context.Context, int, *pool.Conn, string, bool) {} func (noopRecorder) RecordPendingRequests(context.Context, int, *pool.Conn, string) {} diff --git a/maintnotifications/circuit_breaker.go b/maintnotifications/circuit_breaker.go index cb76b6447f..727f838f9f 100644 --- a/maintnotifications/circuit_breaker.go +++ b/maintnotifications/circuit_breaker.go @@ -2,57 +2,37 @@ package maintnotifications import ( "context" + "errors" "sync" "sync/atomic" "time" "github.com/redis/go-redis/v9/internal" + "github.com/redis/go-redis/v9/internal/circuitbreaker" "github.com/redis/go-redis/v9/internal/maintnotifications/logs" ) // CircuitBreakerState represents the state of a circuit breaker -type CircuitBreakerState int32 +type CircuitBreakerState = circuitbreaker.State const ( // CircuitBreakerClosed - normal operation, requests allowed - CircuitBreakerClosed CircuitBreakerState = iota + CircuitBreakerClosed = circuitbreaker.StateClosed // CircuitBreakerOpen - failing fast, requests rejected - CircuitBreakerOpen + CircuitBreakerOpen = circuitbreaker.StateOpen // CircuitBreakerHalfOpen - testing if service recovered - CircuitBreakerHalfOpen + CircuitBreakerHalfOpen = circuitbreaker.StateHalfOpen ) -func (s CircuitBreakerState) String() string { - switch s { - case CircuitBreakerClosed: - return "closed" - case CircuitBreakerOpen: - return "open" - case CircuitBreakerHalfOpen: - return "half-open" - default: - return "unknown" - } -} - -// CircuitBreaker implements the circuit breaker pattern for endpoint-specific failure handling +// CircuitBreaker wraps the internal circuit breaker with endpoint-specific +// logging. The inner breaker is held as an unexported field (rather than +// embedded) so its exported methods are not promoted onto this type; that keeps +// callers on the wrapper's API and preserves wrapper invariants such as the +// lastSuccessTime bookkeeping. type CircuitBreaker struct { - // Configuration - failureThreshold int // Number of failures before opening - resetTimeout time.Duration // How long to stay open before testing - maxRequests int // Max requests allowed in half-open state - - // State tracking (atomic for lock-free access) - state atomic.Int32 // CircuitBreakerState - failures atomic.Int64 // Current failure count - successes atomic.Int64 // Success count in half-open state - requests atomic.Int64 // Request count in half-open state - lastFailureTime atomic.Int64 // Unix timestamp of last failure + inner *circuitbreaker.CircuitBreaker + endpoint string lastSuccessTime atomic.Int64 // Unix timestamp of last success - - // Endpoint identification - endpoint string - config *Config } // newCircuitBreaker creates a new circuit breaker for an endpoint @@ -68,136 +48,119 @@ func newCircuitBreaker(endpoint string, config *Config) *CircuitBreaker { maxRequests = config.CircuitBreakerMaxRequests } - return &CircuitBreaker{ - failureThreshold: failureThreshold, - resetTimeout: resetTimeout, - maxRequests: maxRequests, - endpoint: endpoint, - config: config, - state: atomic.Int32{}, // Defaults to CircuitBreakerClosed (0) + cb := &CircuitBreaker{ + inner: circuitbreaker.New(circuitbreaker.Config{ + FailureThreshold: failureThreshold, + SuccessThreshold: maxRequests, // Use maxRequests as success threshold + MaxHalfOpenRequests: maxRequests, + OpenTimeout: resetTimeout, + }), + endpoint: endpoint, } + + // Register logging callback for state changes + cb.inner.OnStateChange(func(oldState, newState circuitbreaker.State) { + switch { + case oldState == circuitbreaker.StateClosed && newState == circuitbreaker.StateOpen: + if internal.LogLevel.WarnOrAbove() { + stats := cb.inner.Stats() + internal.Logger.Printf(context.Background(), logs.CircuitBreakerOpened(endpoint, int64(stats.Failures))) + } + case oldState == circuitbreaker.StateOpen && newState == circuitbreaker.StateHalfOpen: + if internal.LogLevel.InfoOrAbove() { + internal.Logger.Printf(context.Background(), logs.CircuitBreakerTransitioningToHalfOpen(endpoint)) + } + case oldState == circuitbreaker.StateHalfOpen && newState == circuitbreaker.StateClosed: + if internal.LogLevel.InfoOrAbove() { + stats := cb.inner.Stats() + internal.Logger.Printf(context.Background(), logs.CircuitBreakerClosed(endpoint, int64(stats.Successes))) + } + case oldState == circuitbreaker.StateHalfOpen && newState == circuitbreaker.StateOpen: + if internal.LogLevel.WarnOrAbove() { + internal.Logger.Printf(context.Background(), logs.CircuitBreakerReopened(endpoint)) + } + } + }) + + return cb } -// IsOpen returns true if the circuit breaker is open (rejecting requests) +// IsOpen returns true if the circuit breaker is open (rejecting requests). +// It uses CheckState so the open->half-open transition is honored once +// OpenTimeout has elapsed, rather than reporting a stale open state. +// +// IsOpen does not reserve a half-open request slot; callers that gate work on +// the breaker should use allowRequest so MaxHalfOpenRequests is respected. func (cb *CircuitBreaker) IsOpen() bool { - state := CircuitBreakerState(cb.state.Load()) - return state == CircuitBreakerOpen + return cb.inner.CheckState() == circuitbreaker.StateOpen +} + +// allowRequest reports whether a request should be allowed through and +// whether the admission reserved a half-open probe slot (half-open +// admissions are bounded by MaxHalfOpenRequests; closed admissions reserve +// nothing). A reserved slot must be settled with a subsequent +// recordSuccess/recordFailure, or released via releaseRequest when the +// operation produced neither outcome — callers must consult reserved first, +// or a handoff admitted while closed that outlives a later open -> half-open +// transition would free a slot a real recovery probe is holding. +func (cb *CircuitBreaker) allowRequest() (allowed, reserved bool) { + return cb.inner.Allow() } -// shouldAttemptReset checks if enough time has passed to attempt reset -func (cb *CircuitBreaker) shouldAttemptReset() bool { - lastFailure := time.Unix(cb.lastFailureTime.Load(), 0) - return time.Since(lastFailure) >= cb.resetTimeout +// releaseRequest returns a half-open slot reserved by allowRequest when the +// operation completed without a recordable success or failure. +func (cb *CircuitBreaker) releaseRequest() { + cb.inner.ReleaseHalfOpen() } // Execute runs the given function with circuit breaker protection func (cb *CircuitBreaker) Execute(fn func() error) error { - // Single atomic state load for consistency - state := CircuitBreakerState(cb.state.Load()) - - switch state { - case CircuitBreakerOpen: - if cb.shouldAttemptReset() { - // Attempt transition to half-open - if cb.state.CompareAndSwap(int32(CircuitBreakerOpen), int32(CircuitBreakerHalfOpen)) { - cb.requests.Store(0) - cb.successes.Store(0) - if internal.LogLevel.InfoOrAbove() { - internal.Logger.Printf(context.Background(), logs.CircuitBreakerTransitioningToHalfOpen(cb.endpoint)) - } - // Fall through to half-open logic - } else { - return ErrCircuitBreakerOpen - } - } else { - return ErrCircuitBreakerOpen - } - fallthrough - case CircuitBreakerHalfOpen: - requests := cb.requests.Add(1) - if requests > int64(cb.maxRequests) { - cb.requests.Add(-1) // Revert the increment - return ErrCircuitBreakerOpen - } + err := cb.inner.Execute(fn) + if err == nil { + cb.lastSuccessTime.Store(time.Now().Unix()) + return nil } - - // Execute the function with consistent state - err := fn() - - if err != nil { - cb.recordFailure() - return err + // Convert internal circuit open error to our package's error. Use errors.Is + // so a future wrapped ErrCircuitOpen is still translated to the public error. + if errors.Is(err, circuitbreaker.ErrCircuitOpen) { + return ErrCircuitBreakerOpen } - - cb.recordSuccess() - return nil + return err } -// recordFailure records a failure and potentially opens the circuit +// recordFailure records a failure (for external use when not using Execute) func (cb *CircuitBreaker) recordFailure() { - cb.lastFailureTime.Store(time.Now().Unix()) - failures := cb.failures.Add(1) - - state := CircuitBreakerState(cb.state.Load()) - - switch state { - case CircuitBreakerClosed: - if failures >= int64(cb.failureThreshold) { - if cb.state.CompareAndSwap(int32(CircuitBreakerClosed), int32(CircuitBreakerOpen)) { - if internal.LogLevel.WarnOrAbove() { - internal.Logger.Printf(context.Background(), logs.CircuitBreakerOpened(cb.endpoint, failures)) - } - } - } - case CircuitBreakerHalfOpen: - // Any failure in half-open state immediately opens the circuit - if cb.state.CompareAndSwap(int32(CircuitBreakerHalfOpen), int32(CircuitBreakerOpen)) { - if internal.LogLevel.WarnOrAbove() { - internal.Logger.Printf(context.Background(), logs.CircuitBreakerReopened(cb.endpoint)) - } - } - } + cb.inner.RecordFailure() } -// recordSuccess records a success and potentially closes the circuit -func (cb *CircuitBreaker) recordSuccess() { +// recordSuccess records a success (for external use when not using Execute). +// reserved must be the flag allowRequest returned for this operation's +// admission: a closed-state admission holds no half-open slot, so its +// success must count toward closing without releasing one. +func (cb *CircuitBreaker) recordSuccess(reserved bool) { cb.lastSuccessTime.Store(time.Now().Unix()) - - state := CircuitBreakerState(cb.state.Load()) - - switch state { - case CircuitBreakerClosed: - // Reset failure count on success in closed state - cb.failures.Store(0) - case CircuitBreakerHalfOpen: - successes := cb.successes.Add(1) - - // If we've had enough successful requests, close the circuit - if successes >= int64(cb.maxRequests) { - if cb.state.CompareAndSwap(int32(CircuitBreakerHalfOpen), int32(CircuitBreakerClosed)) { - cb.failures.Store(0) - if internal.LogLevel.InfoOrAbove() { - internal.Logger.Printf(context.Background(), logs.CircuitBreakerClosed(cb.endpoint, successes)) - } - } - } + if reserved { + cb.inner.RecordSuccess() + } else { + cb.inner.RecordExternalSuccess() } } // GetState returns the current state of the circuit breaker func (cb *CircuitBreaker) GetState() CircuitBreakerState { - return CircuitBreakerState(cb.state.Load()) + return cb.inner.State() } // GetStats returns current statistics for monitoring func (cb *CircuitBreaker) GetStats() CircuitBreakerStats { + stats := cb.inner.Stats() return CircuitBreakerStats{ Endpoint: cb.endpoint, - State: cb.GetState(), - Failures: cb.failures.Load(), - Successes: cb.successes.Load(), - Requests: cb.requests.Load(), - LastFailureTime: time.Unix(cb.lastFailureTime.Load(), 0), + State: stats.State, + Failures: int64(stats.Failures), + Successes: int64(stats.Successes), + Requests: int64(stats.Requests), + LastFailureTime: stats.LastFailureTime, LastSuccessTime: time.Unix(cb.lastSuccessTime.Load(), 0), } } @@ -341,13 +304,8 @@ func (cbm *CircuitBreakerManager) Shutdown() { func (cbm *CircuitBreakerManager) Reset() { cbm.breakers.Range(func(key, value interface{}) bool { entry := value.(*CircuitBreakerEntry) - breaker := entry.breaker - breaker.state.Store(int32(CircuitBreakerClosed)) - breaker.failures.Store(0) - breaker.successes.Store(0) - breaker.requests.Store(0) - breaker.lastFailureTime.Store(0) - breaker.lastSuccessTime.Store(0) + entry.breaker.inner.Reset() + entry.breaker.lastSuccessTime.Store(0) return true }) } diff --git a/maintnotifications/circuit_breaker_test.go b/maintnotifications/circuit_breaker_test.go index 523558dd3f..23c4c87be1 100644 --- a/maintnotifications/circuit_breaker_test.go +++ b/maintnotifications/circuit_breaker_test.go @@ -225,6 +225,42 @@ func TestCircuitBreaker(t *testing.T) { }) } +// TestAllowRequestReportsReservation pins the handoff gate's slot +// accounting: a handoff admitted while the breaker is CLOSED reserves no +// half-open probe slot, so the worker's later releaseRequest (init-error +// path) must be skipped for it — an unconditional release would free the +// slot a real recovery probe is holding and let more than +// CircuitBreakerMaxRequests concurrent handoffs reach a recovering endpoint. +func TestAllowRequestReportsReservation(t *testing.T) { + config := &Config{ + CircuitBreakerFailureThreshold: 1, + CircuitBreakerResetTimeout: 30 * time.Millisecond, + CircuitBreakerMaxRequests: 1, + } + cb := newCircuitBreaker("test-endpoint:6379", config) + + // Closed admission: allowed, nothing reserved. + allowed, reserved := cb.allowRequest() + if !allowed || reserved { + t.Fatalf("closed: allowRequest() = (%v, %v), want (true, false)", allowed, reserved) + } + + // While that handoff runs: a failure opens the breaker, the reset + // timeout elapses, and a recovery probe reserves the only slot. + cb.recordFailure() + time.Sleep(50 * time.Millisecond) + if allowed, reserved := cb.allowRequest(); !allowed || !reserved { + t.Fatalf("half-open: allowRequest() = (%v, %v), want (true, true)", allowed, reserved) + } + + // The closed-admitted handoff finishes with an init error; the worker + // skips releaseRequest because reserved was false. The probe's slot + // must still be held. + if allowed, _ := cb.allowRequest(); allowed { + t.Error("a second half-open admission succeeded — the probe slot was freed") + } +} + func TestCircuitBreakerManager(t *testing.T) { config := &Config{ CircuitBreakerFailureThreshold: 5, @@ -298,8 +334,10 @@ func TestCircuitBreakerManager(t *testing.T) { t.Error("Circuit should be closed after reset") } - if cb.failures.Load() != 0 { - t.Error("Failure count should be reset to 0") + // Verify reset worked by checking state (can't access internal counters) + stats := cb.GetStats() + if stats.State != CircuitBreakerClosed { + t.Error("Circuit state should be closed after reset") } }) @@ -312,16 +350,8 @@ func TestCircuitBreakerManager(t *testing.T) { cb := newCircuitBreaker("test-endpoint:6379", config) - // Test that configuration values are used - if cb.failureThreshold != 10 { - t.Errorf("Expected failureThreshold=10, got %d", cb.failureThreshold) - } - if cb.resetTimeout != 30*time.Second { - t.Errorf("Expected resetTimeout=30s, got %v", cb.resetTimeout) - } - if cb.maxRequests != 5 { - t.Errorf("Expected maxRequests=5, got %d", cb.maxRequests) - } + // Test that configuration values are used by verifying behavior + // (can't access internal fields directly) // Test that circuit opens after configured threshold testError := errors.New("test error") diff --git a/maintnotifications/handoff_worker.go b/maintnotifications/handoff_worker.go index d66542ffc4..d8539a3b12 100644 --- a/maintnotifications/handoff_worker.go +++ b/maintnotifications/handoff_worker.go @@ -367,8 +367,13 @@ func (hwm *handoffWorkerManager) performConnectionHandoff(ctx context.Context, c // Use circuit breaker to protect against failing endpoints circuitBreaker := hwm.circuitBreakerManager.GetCircuitBreaker(newEndpoint) - // Check if circuit breaker is open before attempting handoff - if circuitBreaker.IsOpen() { + // Gate the handoff on the circuit breaker. A half-open admission reserves + // a probe slot so that, once OpenTimeout elapses, concurrent handoffs to a + // recovering endpoint stay bounded by MaxHalfOpenRequests instead of all + // being admitted at once; a closed admission reserves nothing, and the + // settle paths below must not release a slot this handoff never held. + allowed, reserved := circuitBreaker.allowRequest() + if !allowed { internal.Logger.Printf(ctx, logs.CircuitBreakerOpen(connID, newEndpoint)) return false, ErrCircuitBreakerOpen // Don't retry when circuit breaker is open } @@ -378,15 +383,20 @@ func (hwm *handoffWorkerManager) performConnectionHandoff(ctx context.Context, c // Update circuit breaker based on result if err != nil { - // Only track dial/network errors in circuit breaker, not initialization errors + // Only track dial/network errors in circuit breaker, not initialization errors. if shouldRetry { circuitBreaker.recordFailure() + } else if reserved { + // Initialization error: not a dial/network failure, so it neither + // opens nor closes the breaker. Release the reserved half-open slot + // so it does not starve future recovery probes. + circuitBreaker.releaseRequest() } return shouldRetry, err } // Success - record in circuit breaker - circuitBreaker.recordSuccess() + circuitBreaker.recordSuccess(reserved) return false, nil } diff --git a/multidb/healthcheck.go b/multidb/healthcheck.go new file mode 100644 index 0000000000..c9500bd9cd --- /dev/null +++ b/multidb/healthcheck.go @@ -0,0 +1,321 @@ +// Package multidb provides health check and failover strategy implementations +// for use with redis.MultiDBClient. +package multidb + +import ( + "context" + "sync" + "time" + + "github.com/redis/go-redis/v9" +) + +// Default values for health check configuration +const ( + DefaultHealthCheckProbes = 3 + DefaultHealthCheckDelay = 500 * time.Millisecond + DefaultHealthCheckTimeout = 3 * time.Second +) + +// HealthCheckConfig holds configuration for health checks. +type HealthCheckConfig struct { + Probes int + Delay time.Duration + Timeout time.Duration +} + +// DefaultHealthCheckConfig returns the default health check configuration. +func DefaultHealthCheckConfig() HealthCheckConfig { + return HealthCheckConfig{ + Probes: DefaultHealthCheckProbes, + Delay: DefaultHealthCheckDelay, + Timeout: DefaultHealthCheckTimeout, + } +} + +// HealthCheckOption is a functional option for configuring health checks. +type HealthCheckOption func(*HealthCheckConfig) + +// WithProbes sets the number of probes. +func WithProbes(probes int) HealthCheckOption { + return func(c *HealthCheckConfig) { + if probes > 0 { + c.Probes = probes + } + } +} + +// WithDelay sets the delay between probes. +func WithDelay(delay time.Duration) HealthCheckOption { + return func(c *HealthCheckConfig) { + if delay >= 0 { + c.Delay = delay + } + } +} + +// WithTimeout sets the timeout for the health check. +func WithTimeout(timeout time.Duration) HealthCheckOption { + return func(c *HealthCheckConfig) { + if timeout > 0 { + c.Timeout = timeout + } + } +} + +func applyOptions(opts []HealthCheckOption) HealthCheckConfig { + cfg := DefaultHealthCheckConfig() + for _, opt := range opts { + opt(&cfg) + } + return cfg +} + +// HealthCheckPolicy defines how health check probes are evaluated. +type HealthCheckPolicy interface { + Execute(ctx context.Context, checks []redis.MultiDBHealthCheck, client *redis.Client) bool + ExecuteCluster(ctx context.Context, checks []redis.MultiDBHealthCheck, client *redis.ClusterClient) bool +} + +// Compile-time interface compliance checks +var ( + _ redis.MultiDBHealthCheck = (*PingHealthCheck)(nil) + _ redis.MultiDBHealthCheck = (*LagAwareHealthCheck)(nil) + _ HealthCheckPolicy = (*HealthyAllPolicy)(nil) + _ HealthCheckPolicy = (*HealthyMajorityPolicy)(nil) + _ HealthCheckPolicy = (*HealthyAnyPolicy)(nil) +) + +// ConfigurableHealthCheck is an interface for health checks that have configuration. +type ConfigurableHealthCheck interface { + redis.MultiDBHealthCheck + Config() HealthCheckConfig +} + +// probeFunc runs one probe of a health check and reports whether it passed +// along with any error that caused a failure. +type probeFunc func(context.Context, redis.MultiDBHealthCheck) (bool, error) + +// checkRunner interprets the probes of a single health check (all / majority / +// any) and reports whether that check is healthy. +type checkRunner func(ctx context.Context, hc redis.MultiDBHealthCheck, probe probeFunc) bool + +// runChecks executes every health check concurrently, each with its own +// timeout, and returns true only if all of them pass (AND across checks). The +// per-check probe interpretation is delegated to runner. It is the single +// implementation shared by every policy; the policies differ only in which +// runner they pass in. +func runChecks(ctx context.Context, checks []redis.MultiDBHealthCheck, probe probeFunc, runner checkRunner) bool { + if len(checks) == 0 { + return true + } + // Cancelable so an early failure stops the remaining workers instead of + // letting a slow probe run to its own timeout. + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + // Buffered to the number of checks so a worker never blocks on send even + // when we return early, preventing goroutine leaks. + results := make(chan bool, len(checks)) + var wg sync.WaitGroup + for _, hc := range checks { + wg.Add(1) + go func(check redis.MultiDBHealthCheck) { + defer wg.Done() + results <- safeRunner(ctx, check, probe, runner) + }(hc) + } + go func() { wg.Wait(); close(results) }() + + // Every health check must pass (AND across checks). + for result := range results { + if !result { + return false + } + } + return true +} + +// safeRunner invokes runner and recovers from any panic, reporting the check +// as unhealthy in that case. This guarantees every worker in runChecks sends +// exactly one result: a panicking runner must not be silently dropped, which +// would let the consumer return true after fewer than len(checks) results. +func safeRunner(ctx context.Context, hc redis.MultiDBHealthCheck, probe probeFunc, runner checkRunner) (ok bool) { + defer func() { + if r := recover(); r != nil { + ok = false + } + }() + return runner(ctx, hc, probe) +} + +func standaloneProbe(client *redis.Client) probeFunc { + return func(ctx context.Context, hc redis.MultiDBHealthCheck) (bool, error) { + return hc.CheckHealth(ctx, client) + } +} + +func clusterProbe(client *redis.ClusterClient) probeFunc { + return func(ctx context.Context, hc redis.MultiDBHealthCheck) (bool, error) { + return hc.CheckClusterHealth(ctx, client) + } +} + +// HealthyAllPolicy returns true if ALL probes succeed for each health check. +// For each health check, it runs the configured number of probes with delays. +// ALL probes must succeed for the health check to be considered healthy. +// +// Note: The policy applies to probes within each individual health check. +// When multiple health checks are provided (e.g. Ping + LagAware), ALL +// health checks must pass — the policy does not control the relationship +// between different health checks, only between probes of the same check. +type HealthyAllPolicy struct{} + +func NewHealthyAllPolicy() *HealthyAllPolicy { return &HealthyAllPolicy{} } + +func (p *HealthyAllPolicy) Execute(ctx context.Context, checks []redis.MultiDBHealthCheck, client *redis.Client) bool { + return runChecks(ctx, checks, standaloneProbe(client), runProbesAllMustPass) +} + +func (p *HealthyAllPolicy) ExecuteCluster(ctx context.Context, checks []redis.MultiDBHealthCheck, client *redis.ClusterClient) bool { + return runChecks(ctx, checks, clusterProbe(client), runProbesAllMustPass) +} + +// HealthyMajorityPolicy returns true if a MAJORITY of probes succeed. +// For each health check, it runs the configured number of probes with delays. +// More than half of the probes must succeed. +// +// Note: The policy applies to probes within each individual health check. +// When multiple health checks are provided, ALL health checks must pass +// (each needing a majority of its probes to succeed). +type HealthyMajorityPolicy struct{} + +func NewHealthyMajorityPolicy() *HealthyMajorityPolicy { return &HealthyMajorityPolicy{} } + +func (p *HealthyMajorityPolicy) Execute(ctx context.Context, checks []redis.MultiDBHealthCheck, client *redis.Client) bool { + return runChecks(ctx, checks, standaloneProbe(client), runProbesMajority) +} + +func (p *HealthyMajorityPolicy) ExecuteCluster(ctx context.Context, checks []redis.MultiDBHealthCheck, client *redis.ClusterClient) bool { + return runChecks(ctx, checks, clusterProbe(client), runProbesMajority) +} + +// HealthyAnyPolicy returns true if AT LEAST ONE probe succeeds. +// For each health check, it runs probes until one succeeds or all fail. +// +// Note: The policy applies to probes within each individual health check. +// When multiple health checks are provided, ALL health checks must pass +// (each needing at least one successful probe). +type HealthyAnyPolicy struct{} + +func NewHealthyAnyPolicy() *HealthyAnyPolicy { return &HealthyAnyPolicy{} } + +func (p *HealthyAnyPolicy) Execute(ctx context.Context, checks []redis.MultiDBHealthCheck, client *redis.Client) bool { + return runChecks(ctx, checks, standaloneProbe(client), runProbesAny) +} + +func (p *HealthyAnyPolicy) ExecuteCluster(ctx context.Context, checks []redis.MultiDBHealthCheck, client *redis.ClusterClient) bool { + return runChecks(ctx, checks, clusterProbe(client), runProbesAny) +} + +// getConfig returns the health check config, using defaults if not +// configurable. Invalid values from a configurable check are clamped to their +// defaults so the probe runners stay robust: a non-positive Probes would +// otherwise make a check trivially pass (zero iterations), and a non-positive +// Timeout would expire the context immediately. +func getConfig(hc redis.MultiDBHealthCheck) HealthCheckConfig { + cfg := DefaultHealthCheckConfig() + if chc, ok := hc.(ConfigurableHealthCheck); ok { + cfg = chc.Config() + } + if cfg.Probes <= 0 { + cfg.Probes = DefaultHealthCheckProbes + } + if cfg.Timeout <= 0 { + cfg.Timeout = DefaultHealthCheckTimeout + } + if cfg.Delay < 0 { + cfg.Delay = DefaultHealthCheckDelay + } + return cfg +} + +// runProbesAllMustPass runs probes where ALL must succeed. +func runProbesAllMustPass(ctx context.Context, hc redis.MultiDBHealthCheck, probe probeFunc) bool { + cfg := getConfig(hc) + ctx, cancel := context.WithTimeout(ctx, cfg.Timeout) + defer cancel() + + for i := 0; i < cfg.Probes; i++ { + if ok, _ := probe(ctx, hc); !ok { + return false + } + if i < cfg.Probes-1 && cfg.Delay > 0 { + select { + case <-ctx.Done(): + return false + case <-time.After(cfg.Delay): + } + } + } + return true +} + +// runProbesMajority runs probes where MAJORITY must succeed. +func runProbesMajority(ctx context.Context, hc redis.MultiDBHealthCheck, probe probeFunc) bool { + cfg := getConfig(hc) + ctx, cancel := context.WithTimeout(ctx, cfg.Timeout) + defer cancel() + + // Strict majority: more than half must pass + // (probes - 1) // 2 gives the max allowed failures + allowedFailures := (cfg.Probes - 1) / 2 + // requiredSuccesses is the number of successful probes that guarantees a + // strict majority, allowing an early exit once it is reached. + requiredSuccesses := cfg.Probes - allowedFailures + failures := 0 + successes := 0 + + for i := 0; i < cfg.Probes; i++ { + if ok, _ := probe(ctx, hc); ok { + successes++ + if successes >= requiredSuccesses { + return true + } + } else { + failures++ + if failures > allowedFailures { + return false + } + } + if i < cfg.Probes-1 && cfg.Delay > 0 { + select { + case <-ctx.Done(): + return false + case <-time.After(cfg.Delay): + } + } + } + return true +} + +// runProbesAny runs probes where ANY success is enough. +func runProbesAny(ctx context.Context, hc redis.MultiDBHealthCheck, probe probeFunc) bool { + cfg := getConfig(hc) + ctx, cancel := context.WithTimeout(ctx, cfg.Timeout) + defer cancel() + + for i := 0; i < cfg.Probes; i++ { + if ok, _ := probe(ctx, hc); ok { + return true + } + if i < cfg.Probes-1 && cfg.Delay > 0 { + select { + case <-ctx.Done(): + return false + case <-time.After(cfg.Delay): + } + } + } + return false +} diff --git a/multidb/healthcheck_lag_aware.go b/multidb/healthcheck_lag_aware.go new file mode 100644 index 0000000000..61cce2a8dd --- /dev/null +++ b/multidb/healthcheck_lag_aware.go @@ -0,0 +1,473 @@ +package multidb + +import ( + "context" + "crypto/tls" + "crypto/x509" + "encoding/json" + "fmt" + "io" + "net" + "net/http" + "os" + "strconv" + "strings" + "sync" + "time" + + "github.com/redis/go-redis/v9" +) + +// Default values for LagAwareHealthCheck +const ( + DefaultRESTAPIPort = 9443 + DefaultLagTolerance = 5000 // milliseconds + DefaultHTTPTimeout = 10 * time.Second +) + +// LagAwareHealthCheck checks database health via Redis Enterprise REST API. +// It verifies that the database is healthy based on replication lag tolerance. +type LagAwareHealthCheck struct { + config HealthCheckConfig + baseURL string + restAPIPort int + lagTolerance int + httpClient HTTPClient + username string + password string + tlsConfig *tls.Config + // configErr records the first error encountered while applying options + // (e.g. an invalid PEM or unreadable cert file). When set, health checks + // fail fast rather than silently running with an incomplete TLS config. + configErr error +} + +// HTTPClient is an interface for making HTTP requests. +type HTTPClient interface { + Do(req *http.Request) (*http.Response, error) +} + +// LagAwareHealthCheckOption is a functional option for LagAwareHealthCheck. +type LagAwareHealthCheckOption func(*LagAwareHealthCheck) + +// WithLagAwareHealthCheckConfig applies generic HealthCheckOption values +// (e.g. WithProbes, WithDelay, WithTimeout) to a LagAwareHealthCheck. +func WithLagAwareHealthCheckConfig(opts ...HealthCheckOption) LagAwareHealthCheckOption { + return func(h *LagAwareHealthCheck) { + for _, opt := range opts { + opt(&h.config) + } + } +} + +// WithLagAwareBaseURL sets the base URL for the REST API. By default the +// check derives it from the member's own database host, which the local +// endpoint-availability probe relies on: the /v1/local/ API answers for the +// node that serves the request. A custom base URL must therefore point at +// (or resolve to) the cluster node hosting this member's endpoint — routing +// it through a shared admin address, proxy, or load balancer makes the check +// report the availability of whichever node the URL happens to reach. +func WithLagAwareBaseURL(baseURL string) LagAwareHealthCheckOption { + return func(h *LagAwareHealthCheck) { h.baseURL = baseURL } +} + +// WithLagAwareRESTAPIPort sets the REST API port (default: 9443). +func WithLagAwareRESTAPIPort(port int) LagAwareHealthCheckOption { + return func(h *LagAwareHealthCheck) { h.restAPIPort = port } +} + +// WithLagAwareTolerance sets the lag tolerance in milliseconds (default: 5000). +func WithLagAwareTolerance(toleranceMS int) LagAwareHealthCheckOption { + return func(h *LagAwareHealthCheck) { h.lagTolerance = toleranceMS } +} + +// WithLagAwareHTTPClient sets a custom HTTP client. +func WithLagAwareHTTPClient(client HTTPClient) LagAwareHealthCheckOption { + return func(h *LagAwareHealthCheck) { h.httpClient = client } +} + +// WithLagAwareBasicAuth sets basic authentication credentials. +func WithLagAwareBasicAuth(username, password string) LagAwareHealthCheckOption { + return func(h *LagAwareHealthCheck) { h.username = username; h.password = password } +} + +// WithLagAwareTLSConfig sets a custom TLS configuration. The config is cloned so +// later options (and the health check itself) never mutate the caller's value. +// A nil config clears any previously set TLS configuration. +func WithLagAwareTLSConfig(cfg *tls.Config) LagAwareHealthCheckOption { + return func(h *LagAwareHealthCheck) { + c := cfg.Clone() // Clone(nil) returns nil + if c != nil { + // Clone is shallow for RootCAs and shares the Certificates + // backing array; copy both so later options that append CAs or + // client certificates cannot reach the caller's values. + if c.RootCAs != nil { + c.RootCAs = c.RootCAs.Clone() + } + if c.Certificates != nil { + c.Certificates = append([]tls.Certificate(nil), c.Certificates...) + } + } + h.tlsConfig = c + } +} + +// WithLagAwareInsecureSkipVerify disables TLS certificate verification. +func WithLagAwareInsecureSkipVerify() LagAwareHealthCheckOption { + return func(h *LagAwareHealthCheck) { + if h.tlsConfig == nil { + h.tlsConfig = &tls.Config{} + } + h.tlsConfig.InsecureSkipVerify = true + } +} + +// WithLagAwareRootCAs sets the root CA certificates for TLS verification. +// If the PEM data cannot be parsed, the error is recorded and subsequent +// health checks fail rather than silently running without the CAs. +func WithLagAwareRootCAs(certPEM []byte) LagAwareHealthCheckOption { + return func(h *LagAwareHealthCheck) { + if h.tlsConfig == nil { + h.tlsConfig = &tls.Config{} + } + if h.tlsConfig.RootCAs == nil { + h.tlsConfig.RootCAs = x509.NewCertPool() + } + if !h.tlsConfig.RootCAs.AppendCertsFromPEM(certPEM) { + h.setConfigErr(fmt.Errorf("multidb: failed to parse root CA PEM")) + } + } +} + +// WithLagAwareRootCAsFromFile loads root CA certificates from a PEM file. +// If the file cannot be read or parsed, the error is recorded and subsequent +// health checks fail rather than silently running without the CAs. +func WithLagAwareRootCAsFromFile(caFile string) LagAwareHealthCheckOption { + return func(h *LagAwareHealthCheck) { + certPEM, err := os.ReadFile(caFile) + if err != nil { + h.setConfigErr(fmt.Errorf("multidb: failed to read root CA file %q: %w", caFile, err)) + return + } + if h.tlsConfig == nil { + h.tlsConfig = &tls.Config{} + } + if h.tlsConfig.RootCAs == nil { + h.tlsConfig.RootCAs = x509.NewCertPool() + } + if !h.tlsConfig.RootCAs.AppendCertsFromPEM(certPEM) { + h.setConfigErr(fmt.Errorf("multidb: failed to parse root CA PEM from file %q", caFile)) + } + } +} + +// WithLagAwareClientCert sets the client certificate for mutual TLS (mTLS). +// If the key pair cannot be loaded, the error is recorded and subsequent +// health checks fail rather than silently disabling mTLS. +func WithLagAwareClientCert(certPEM, keyPEM []byte) LagAwareHealthCheckOption { + return func(h *LagAwareHealthCheck) { + cert, err := tls.X509KeyPair(certPEM, keyPEM) + if err != nil { + h.setConfigErr(fmt.Errorf("multidb: failed to load client certificate: %w", err)) + return + } + if h.tlsConfig == nil { + h.tlsConfig = &tls.Config{} + } + h.tlsConfig.Certificates = append(h.tlsConfig.Certificates, cert) + } +} + +// WithLagAwareClientCertFromFiles loads client cert and key from files. +// If the key pair cannot be loaded, the error is recorded and subsequent +// health checks fail rather than silently disabling mTLS. +func WithLagAwareClientCertFromFiles(certFile, keyFile string) LagAwareHealthCheckOption { + return func(h *LagAwareHealthCheck) { + cert, err := tls.LoadX509KeyPair(certFile, keyFile) + if err != nil { + h.setConfigErr(fmt.Errorf("multidb: failed to load client certificate from files: %w", err)) + return + } + if h.tlsConfig == nil { + h.tlsConfig = &tls.Config{} + } + h.tlsConfig.Certificates = append(h.tlsConfig.Certificates, cert) + } +} + +// setConfigErr records the first configuration error encountered. +func (h *LagAwareHealthCheck) setConfigErr(err error) { + if h.configErr == nil { + h.configErr = err + } +} + +// NewLagAwareHealthCheck creates a new LagAwareHealthCheck. +// +// Generic health check settings (probes, delay, timeout) can be supplied via +// WithLagAwareHealthCheckConfig. +func NewLagAwareHealthCheck(opts ...LagAwareHealthCheckOption) *LagAwareHealthCheck { + h := &LagAwareHealthCheck{ + config: DefaultHealthCheckConfig(), + restAPIPort: DefaultRESTAPIPort, + lagTolerance: DefaultLagTolerance, + } + for _, opt := range opts { + opt(h) + } + if h.httpClient == nil { + transport, ok := http.DefaultTransport.(*http.Transport) + if ok { + transport = transport.Clone() + } else { + transport = &http.Transport{} + } + if h.tlsConfig != nil { + // Clone so a caller-supplied tls.Config that is shared or later + // mutated cannot race with the transport's use of it. + transport.TLSClientConfig = h.tlsConfig.Clone() + } + // Honor a configured probe timeout larger than the default so REST + // calls are not capped below the probe budget; the request context + // still bounds each individual call. + httpTimeout := DefaultHTTPTimeout + if h.config.Timeout > httpTimeout { + httpTimeout = h.config.Timeout + } + h.httpClient = &http.Client{Timeout: httpTimeout, Transport: transport} + } + return h +} + +func (h *LagAwareHealthCheck) Config() HealthCheckConfig { return h.config } + +// hostPortFromAddr is hostFromAddr plus the numeric Redis port (0 when the +// address carries none). The port disambiguates Redis Enterprise databases +// that share a DNS name but listen on different ports. +func hostPortFromAddr(addr string) (string, int, bool) { + if addr == "" { + return "", 0, false + } + if strings.HasPrefix(addr, "/") || strings.HasPrefix(addr, "unix://") { + return "", 0, false + } + if host, portStr, err := net.SplitHostPort(addr); err == nil { + port, aerr := strconv.Atoi(portStr) + if aerr != nil { + // Service-name ports ("redis.example.com:redis") are valid dial + // targets; resolve them through the local services database so + // Enterprise endpoints sharing one DNS name still disambiguate + // by port. An unresolvable name degrades to 0 (host-only match), + // as before. + if p, lerr := net.LookupPort("tcp", portStr); lerr == nil { + port = p + } + } + if host == "" { + // The ":6379" shorthand means localhost; an empty host would + // otherwise produce an unusable https://:9443 REST base URL. + host = "localhost" + } + return host, port, true + } + // No port present; treat the whole value as the host. Strip any surrounding + // IPv6 brackets (e.g. "[::1]") so callers can re-bracket the host via + // net.JoinHostPort without producing a doubly-bracketed "[[::1]]:9443". + if strings.HasPrefix(addr, "[") && strings.HasSuffix(addr, "]") { + addr = addr[1 : len(addr)-1] + } + return addr, 0, true +} + +// CheckHealth performs a single REST API health check probe. It returns +// (false, err) with the error that made the database unhealthy (config error, +// an unusable address, or a REST API failure) so callers can record it. +func (h *LagAwareHealthCheck) CheckHealth(ctx context.Context, client *redis.Client) (bool, error) { + if h.configErr != nil { + return false, h.configErr + } + host, port, ok := hostPortFromAddr(client.Options().Addr) + if !ok { + return false, fmt.Errorf("multidb: cannot derive REST API host from address %q", client.Options().Addr) + } + return h.checkLagHealth(ctx, host, port) +} + +// CheckClusterHealth performs a single REST API health check probe. The +// cluster's currently discovered shard addresses are tried first, then the +// configured seeds: the cluster can be healthy and routing through live +// nodes while the (possibly stale) startup seeds are unreachable. +func (h *LagAwareHealthCheck) CheckClusterHealth(ctx context.Context, client *redis.ClusterClient) (bool, error) { + if h.configErr != nil { + return false, h.configErr + } + opts := client.Options() + + var mu sync.Mutex + var addrs []string + seen := map[string]bool{} + add := func(addr string) { + mu.Lock() + if !seen[addr] { + seen[addr] = true + addrs = append(addrs, addr) + } + mu.Unlock() + } + // Best effort: with no loaded cluster state this errors and the seeds + // below are the only candidates. ForEachShard runs concurrently, and it + // may attempt a synchronous topology reload first — bound it to a slice + // of the probe budget so a hung reload through a dead node cannot spend + // the whole health-check context before the REST calls run. + shardCtx := ctx + if deadline, ok := ctx.Deadline(); ok { + var cancel context.CancelFunc + shardCtx, cancel = context.WithDeadline(ctx, time.Now().Add(time.Until(deadline)/4)) + defer cancel() + } + _ = client.ForEachShard(shardCtx, func(_ context.Context, shard *redis.Client) error { + add(shard.Options().Addr) + return nil + }) + for _, addr := range opts.Addrs { + add(addr) + } + if len(addrs) == 0 { + return false, fmt.Errorf("multidb: cluster client has no addresses") + } + + var lastErr error + for _, addr := range addrs { + host, port, ok := hostPortFromAddr(addr) + if !ok { + lastErr = fmt.Errorf("multidb: cannot derive REST API host from address %q", addr) + continue + } + healthy, err := h.checkLagHealth(ctx, host, port) + if healthy { + return true, nil + } + lastErr = err + if ctx.Err() != nil { + break + } + } + return false, lastErr +} + +func (h *LagAwareHealthCheck) checkLagHealth(ctx context.Context, dbHost string, dbPort int) (bool, error) { + baseURL := strings.TrimRight(h.baseURL, "/") + if baseURL == "" { + // net.JoinHostPort brackets IPv6 literals so the URL is valid + // (e.g. https://[::1]:9443 rather than https://::1:9443). + hostPort := net.JoinHostPort(dbHost, strconv.Itoa(h.restAPIPort)) + baseURL = fmt.Sprintf("https://%s", hostPort) + } + // fields keeps frequent probes cheap: matching only needs uid and + // endpoints, not full database configs. + bdbs, err := h.getBDBs(ctx, fmt.Sprintf("%s/v1/bdbs?fields=uid,endpoints", baseURL)) + if err != nil { + return false, err + } + var uid int + found := false + for _, bdb := range bdbs { + if h.bdbMatchesHost(bdb, dbHost, dbPort) { + uid = bdb.UID + found = true + break + } + } + if !found { + return false, fmt.Errorf("multidb: no matching bdb found for host %q", dbHost) + } + // Endpoint-level, not database-level, availability: without the OSS + // cluster API the database check reports healthy while ANY endpoint is + // up, which can mask an outage of the endpoint this member uses. The + // /v1/local/ form is answered by the node the request reaches (the + // member's own host unless a custom base URL points elsewhere) and does + // not redirect to the primary node. + url := fmt.Sprintf("%s/v1/local/bdbs/%d/endpoint/availability?extend_check=lag&availability_lag_tolerance_ms=%d", + baseURL, uid, h.lagTolerance) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return false, err + } + if h.username != "" { + req.SetBasicAuth(h.username, h.password) + } + resp, err := h.httpClient.Do(req) + if err != nil { + return false, err + } + defer resp.Body.Close() + // Drain the (small) body so the keep-alive connection is reusable: this + // check runs on every health-check tick, and an undrained body forces a + // fresh TCP/TLS handshake per probe. Bounded in case a proxy misbehaves. + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 64<<10)) + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return false, fmt.Errorf("multidb: availability check returned status %d", resp.StatusCode) + } + return true, nil +} + +type bdbInfo struct { + UID int `json:"uid"` + Endpoints []bdbEndpoint `json:"endpoints"` +} + +type bdbEndpoint struct { + DNSName string `json:"dns_name"` + Addr []string `json:"addr"` + Port int `json:"port"` +} + +func (h *LagAwareHealthCheck) getBDBs(ctx context.Context, url string) ([]bdbInfo, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) + if err != nil { + return nil, err + } + if h.username != "" { + req.SetBasicAuth(h.username, h.password) + } + resp, err := h.httpClient.Do(req) + if err != nil { + return nil, err + } + defer resp.Body.Close() + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + // Drain the error body too: failing probe loops (401/503) must not + // burn a TCP/TLS handshake per attempt. + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 64<<10)) + return nil, fmt.Errorf("multidb: REST API returned status %d", resp.StatusCode) + } + var bdbs []bdbInfo + if err := json.NewDecoder(resp.Body).Decode(&bdbs); err != nil { + return nil, err + } + // Drain any trailing bytes so the keep-alive connection is reusable. + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 64<<10)) + return bdbs, nil +} + +// bdbMatchesHost reports whether a database endpoint matches the client's +// host and, when both sides carry one, its Redis port — several Redis +// Enterprise databases can share a DNS name and differ only by port. +func (h *LagAwareHealthCheck) bdbMatchesHost(bdb bdbInfo, host string, port int) bool { + for _, ep := range bdb.Endpoints { + if port != 0 && ep.Port != 0 && ep.Port != port { + continue + } + // DNS resolution is case-insensitive, so the configured host must + // match the REST API's dns_name regardless of case. + if strings.EqualFold(ep.DNSName, host) { + return true + } + for _, addr := range ep.Addr { + if addr == host { + return true + } + } + } + return false +} diff --git a/multidb/healthcheck_ping.go b/multidb/healthcheck_ping.go new file mode 100644 index 0000000000..f2cdee7d72 --- /dev/null +++ b/multidb/healthcheck_ping.go @@ -0,0 +1,73 @@ +package multidb + +import ( + "context" + "fmt" + "sync/atomic" + + "github.com/redis/go-redis/v9" +) + +// PingHealthCheck checks health using the PING command. +// Each call to CheckHealth/CheckClusterHealth performs a SINGLE ping probe. +// The number of probes and how they're interpreted is controlled by the +// HealthCheckPolicy and the config (Probes, Delay, Timeout). +type PingHealthCheck struct { + config HealthCheckConfig +} + +// NewPingHealthCheck creates a new PingHealthCheck with optional configuration. +// +// Example: +// +// hc := multidb.NewPingHealthCheck() +// +// hc := multidb.NewPingHealthCheck( +// multidb.WithProbes(5), +// multidb.WithDelay(100*time.Millisecond), +// multidb.WithTimeout(5*time.Second), +// ) +func NewPingHealthCheck(opts ...HealthCheckOption) *PingHealthCheck { + return &PingHealthCheck{ + config: applyOptions(opts), + } +} + +// Config returns the health check configuration. +func (h *PingHealthCheck) Config() HealthCheckConfig { + return h.config +} + +// CheckHealth performs a single PING probe against the client. It returns +// (false, err) with the PING error when the probe fails so callers can record +// why the check was unhealthy. +func (h *PingHealthCheck) CheckHealth(ctx context.Context, client *redis.Client) (bool, error) { + if err := client.Ping(ctx).Err(); err != nil { + return false, err + } + return true, nil +} + +// CheckClusterHealth performs a single PING probe against every MASTER in +// the cluster. Replicas are deliberately not probed: MultiDB rejects the +// replica-routing options (RouteByLatency/RouteRandomly), so member traffic +// only ever reaches masters — a dead replica must not fail the member and +// trigger a failover away from a cluster that is serving normally. It +// returns (false, err) with the first master's PING error when the probe +// fails. An empty topology (no masters pinged) is reported as unhealthy +// rather than trivially healthy, matching LagAwareHealthCheck which fails +// when the cluster has no addresses. +func (h *PingHealthCheck) CheckClusterHealth(ctx context.Context, client *redis.ClusterClient) (bool, error) { + var pinged int64 + err := client.ForEachMaster(ctx, func(ctx context.Context, master *redis.Client) error { + atomic.AddInt64(&pinged, 1) + return master.Ping(ctx).Err() + }) + if err != nil { + return false, err + } + if atomic.LoadInt64(&pinged) == 0 { + return false, fmt.Errorf("multidb: cluster has no masters to ping") + } + return true, nil +} diff --git a/multidb/healthcheck_test.go b/multidb/healthcheck_test.go new file mode 100644 index 0000000000..8fc321a908 --- /dev/null +++ b/multidb/healthcheck_test.go @@ -0,0 +1,859 @@ +package multidb + +import ( + "context" + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "encoding/pem" + "io" + "math/big" + "net/http" + "net/url" + "strings" + "testing" + "time" + + "github.com/redis/go-redis/v9" +) + +func TestPingHealthCheck(t *testing.T) { + t.Run("CheckHealth returns true for healthy client", func(t *testing.T) { + client := redis.NewClient(&redis.Options{Addr: "localhost:6379"}) + defer client.Close() + + ctx := context.Background() + if err := client.Ping(ctx).Err(); err != nil { + t.Skipf("Redis not available: %v", err) + } + + hc := NewPingHealthCheck() + if ok, err := hc.CheckHealth(ctx, client); !ok { + t.Errorf("expected CheckHealth to return true for healthy client, err=%v", err) + } + }) + + t.Run("CheckHealth returns false for unreachable client", func(t *testing.T) { + client := redis.NewClient(&redis.Options{ + Addr: "localhost:0", // port 0: dial fails deterministically + DialTimeout: 100 * time.Millisecond, + }) + defer client.Close() + + hc := NewPingHealthCheck() + ctx := context.Background() + + if ok, err := hc.CheckHealth(ctx, client); ok { + t.Error("expected CheckHealth to return false for unreachable client") + } else if err == nil { + t.Error("expected CheckHealth to return a non-nil error for unreachable client") + } + }) + + t.Run("CheckClusterHealth returns false+error for unreachable/empty cluster", func(t *testing.T) { + // A cluster with no reachable shards must not be reported as trivially + // healthy: CheckClusterHealth must return (false, err) rather than + // (true, nil) when no shard was actually pinged. + client := redis.NewClusterClient(&redis.ClusterOptions{ + Addrs: []string{"localhost:0"}, + DialTimeout: 100 * time.Millisecond, + }) + defer client.Close() + + hc := NewPingHealthCheck() + if ok, err := hc.CheckClusterHealth(context.Background(), client); ok { + t.Error("expected CheckClusterHealth to return false for an empty/unreachable cluster") + } else if err == nil { + t.Error("expected CheckClusterHealth to return a non-nil error for an empty/unreachable cluster") + } + }) +} + +// mockHealthCheck is a test helper that returns a configurable result +type mockHealthCheck struct { + healthy bool +} + +func (m *mockHealthCheck) CheckHealth(ctx context.Context, client *redis.Client) (bool, error) { + return m.healthy, nil +} + +func (m *mockHealthCheck) CheckClusterHealth(ctx context.Context, client *redis.ClusterClient) (bool, error) { + return m.healthy, nil +} + +// --- LagAwareHealthCheck Tests --- + +func TestLagAwareHealthCheck(t *testing.T) { + t.Run("NewLagAwareHealthCheck with defaults", func(t *testing.T) { + hc := NewLagAwareHealthCheck() + + if hc.restAPIPort != DefaultRESTAPIPort { + t.Errorf("expected restAPIPort=%d, got %d", DefaultRESTAPIPort, hc.restAPIPort) + } + if hc.lagTolerance != DefaultLagTolerance { + t.Errorf("expected lagTolerance=%d, got %d", DefaultLagTolerance, hc.lagTolerance) + } + if hc.httpClient == nil { + t.Error("expected httpClient to be set") + } + }) + + t.Run("NewLagAwareHealthCheck with options", func(t *testing.T) { + hc := NewLagAwareHealthCheck( + WithLagAwareBaseURL("https://example.com"), + WithLagAwareRESTAPIPort(8443), + WithLagAwareTolerance(1000), + WithLagAwareBasicAuth("user", "pass"), + ) + + if hc.baseURL != "https://example.com" { + t.Errorf("expected baseURL=https://example.com, got %s", hc.baseURL) + } + if hc.restAPIPort != 8443 { + t.Errorf("expected restAPIPort=8443, got %d", hc.restAPIPort) + } + if hc.lagTolerance != 1000 { + t.Errorf("expected lagTolerance=1000, got %d", hc.lagTolerance) + } + if hc.username != "user" || hc.password != "pass" { + t.Error("expected basic auth to be set") + } + }) + + t.Run("CheckHealth returns false when REST API is unreachable", func(t *testing.T) { + client := redis.NewClient(&redis.Options{Addr: "localhost:6379"}) + defer client.Close() + + // Use a mock HTTP client that always fails + hc := NewLagAwareHealthCheck( + WithLagAwareHTTPClient(&mockHTTPClient{err: context.DeadlineExceeded}), + ) + + ctx := context.Background() + if ok, _ := hc.CheckHealth(ctx, client); ok { + t.Error("expected CheckHealth to return false when REST API is unreachable") + } + }) + + t.Run("bdbMatchesHost matches DNS name", func(t *testing.T) { + hc := NewLagAwareHealthCheck() + bdb := bdbInfo{ + UID: 1, + Endpoints: []bdbEndpoint{ + {DNSName: "redis.example.com", Addr: []string{"10.0.0.1"}, Port: 12000}, + }, + } + + if !hc.bdbMatchesHost(bdb, "redis.example.com", 0) { + t.Error("expected bdbMatchesHost to match DNS name without a port") + } + if !hc.bdbMatchesHost(bdb, "redis.example.com", 12000) { + t.Error("expected bdbMatchesHost to match DNS name with matching port") + } + if hc.bdbMatchesHost(bdb, "redis.example.com", 12001) { + t.Error("expected bdbMatchesHost to reject a same-host different-port database") + } + if !hc.bdbMatchesHost(bdb, "Redis.EXAMPLE.com", 12000) { + t.Error("expected bdbMatchesHost to match DNS name case-insensitively") + } + if !hc.bdbMatchesHost(bdb, "10.0.0.1", 12000) { + t.Error("expected bdbMatchesHost to match address") + } + if hc.bdbMatchesHost(bdb, "other.example.com", 0) { + t.Error("expected bdbMatchesHost to not match different host") + } + }) + + t.Run("TLS options are applied", func(t *testing.T) { + // Test InsecureSkipVerify + hc := NewLagAwareHealthCheck( + WithLagAwareInsecureSkipVerify(), + ) + if hc.tlsConfig == nil { + t.Fatal("expected tlsConfig to be set") + } + if !hc.tlsConfig.InsecureSkipVerify { + t.Error("expected InsecureSkipVerify to be true") + } + + // Test RootCAs with PEM data + caPEM := []byte(`-----BEGIN CERTIFICATE----- +MIIBkTCB+wIJAKHBfpegAzYCMA0GCSqGSIb3DQEBCwUAMBExDzANBgNVBAMMBnVu +dXNlZDAeFw0yMzAxMDEwMDAwMDBaFw0yNDAxMDEwMDAwMDBaMBExDzANBgNVBAMM +BnVudXNlZDBcMA0GCSqGSIb3DQEBAQUAA0sAMEgCQQC7o96WoVCH9xgnLRkMz8pN +2FteamOrPwGMKfkMqF+EAlyH3/wMP0luxSK8BOxdBz0SSlmj2PJwqFcF2rXmVykv +AgMBAAGjUzBRMB0GA1UdDgQWBBQK7ULMHX4ELihB4Bsg+caBRgLsVzAfBgNVHSME +GDAWgBQK7ULMHX4ELihB4Bsg+caBRgLsVzAPBgNVHRMBAf8EBTADAQH/MA0GCSqG +SIb3DQEBCwUAA0EA0FH0N5LT0Y6P6iKv9eDLqE8n6kWUKFq3V6sNqJBUzBuV5IpM +H8PD6BY8JK7P5K8K0K8K0K8K0K8K0K8K0K8K0A== +-----END CERTIFICATE-----`) + hc2 := NewLagAwareHealthCheck( + WithLagAwareRootCAs(caPEM), + ) + if hc2.tlsConfig == nil { + t.Fatal("expected tlsConfig to be set") + } + if hc2.tlsConfig.RootCAs == nil { + t.Error("expected RootCAs to be set") + } + }) +} + +func TestLagAwareHostPortFromAddr(t *testing.T) { + tests := []struct { + addr string + wantHost string + wantPort int + wantOK bool + }{ + {"localhost:6379", "localhost", 6379, true}, + {":6379", "localhost", 6379, true}, + {"10.0.0.1:6379", "10.0.0.1", 6379, true}, + {"redis.example.com:9443", "redis.example.com", 9443, true}, + {"[::1]:6379", "::1", 6379, true}, + {"[2001:db8::1]:6379", "2001:db8::1", 6379, true}, + // Service-name ports are valid dial targets; they must resolve (via + // the local services database) instead of degrading to the port-0 + // wildcard, which would defeat port disambiguation for Enterprise + // endpoints sharing one DNS name. + {"redis.example.com:https", "redis.example.com", 443, true}, + {"localhost", "localhost", 0, true}, + {"[::1]", "::1", 0, true}, + {"[2001:db8::1]", "2001:db8::1", 0, true}, + {"", "", 0, false}, + {"/tmp/redis.sock", "", 0, false}, + {"unix:///tmp/redis.sock", "", 0, false}, + } + for _, tc := range tests { + host, port, ok := hostPortFromAddr(tc.addr) + if ok != tc.wantOK || host != tc.wantHost || port != tc.wantPort { + t.Errorf("hostPortFromAddr(%q) = (%q, %d, %v), want (%q, %d, %v)", + tc.addr, host, port, ok, tc.wantHost, tc.wantPort, tc.wantOK) + } + } +} + +func TestLagAwareConfigErrorFailsHealthCheck(t *testing.T) { + client := redis.NewClient(&redis.Options{Addr: "localhost:6379"}) + defer client.Close() + + // An invalid PEM records a config error, which must fail health checks. + hc := NewLagAwareHealthCheck( + WithLagAwareRootCAs([]byte("not a valid pem")), + ) + if hc.configErr == nil { + t.Fatal("expected configErr to be set for invalid root CA PEM") + } + if ok, err := hc.CheckHealth(context.Background(), client); ok { + t.Error("expected CheckHealth to return false when config error is set") + } else if err == nil { + t.Error("expected CheckHealth to surface the config error") + } +} + +func TestLagAwareTLSConfigOptionIsCloned(t *testing.T) { + caller := &tls.Config{} + hc := NewLagAwareHealthCheck( + WithLagAwareTLSConfig(caller), + WithLagAwareInsecureSkipVerify(), + ) + if !hc.tlsConfig.InsecureSkipVerify { + t.Error("expected health check TLS config to have InsecureSkipVerify set") + } + if caller.InsecureSkipVerify { + t.Error("expected caller TLS config to be left unmodified") + } + if hc.tlsConfig == caller { + t.Error("expected health check to hold a clone, not the caller's TLS config") + } +} + +// genTestCAPEM builds a throwaway self-signed CA certificate at runtime so +// tests can exercise real PEM parsing without a checked-in fixture. +func genTestCAPEM(t *testing.T) []byte { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatalf("generate key: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "multidb-test-ca"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + KeyUsage: x509.KeyUsageCertSign, + BasicConstraintsValid: true, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("create certificate: %v", err) + } + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) +} + +func TestLagAwareTLSConfigDoesNotAliasCallerPool(t *testing.T) { + // tls.Config.Clone is shallow: RootCAs is a shared *x509.CertPool. A CA + // appended by a later option must land in the health check's own pool, + // never in the caller's. + callerPool := x509.NewCertPool() + caller := &tls.Config{RootCAs: callerPool} + hc := NewLagAwareHealthCheck( + WithLagAwareTLSConfig(caller), + WithLagAwareRootCAs(genTestCAPEM(t)), + ) + if hc.configErr != nil { + t.Fatalf("unexpected config error: %v", hc.configErr) + } + if !callerPool.Equal(x509.NewCertPool()) { + t.Error("appending root CAs after WithLagAwareTLSConfig mutated the caller's RootCAs pool") + } + if hc.tlsConfig.RootCAs.Equal(x509.NewCertPool()) { + t.Error("expected the appended CA to land in the health check's own pool") + } +} + +func TestLagAwareTLSConfigDoesNotAliasCallerCertificates(t *testing.T) { + // The shallow clone also shares the Certificates backing array; with + // spare capacity, a later append would write into the caller's array. + caller := &tls.Config{Certificates: make([]tls.Certificate, 1, 4)} + caller.Certificates[0] = tls.Certificate{Certificate: [][]byte{[]byte("caller")}} + hidden := caller.Certificates[:2] + + hc := NewLagAwareHealthCheck(WithLagAwareTLSConfig(caller)) + hc.tlsConfig.Certificates = append(hc.tlsConfig.Certificates, tls.Certificate{ + Certificate: [][]byte{[]byte("healthcheck")}, + }) + + if hidden[1].Certificate != nil { + t.Error("appending a certificate wrote into the caller's Certificates backing array") + } +} + +// mockHTTPClient is a mock HTTP client for testing. +type mockHTTPClient struct { + response *http.Response + err error +} + +func (m *mockHTTPClient) Do(req *http.Request) (*http.Response, error) { + if m.err != nil { + return nil, m.err + } + return m.response, nil +} + +// urlCapturingHTTPClient records the URLs it is asked to fetch and always +// fails the request, so the caller's CheckHealth returns early. It is used to +// assert how the base URL is constructed without needing a live REST API. +type urlCapturingHTTPClient struct { + urls []string +} + +func (c *urlCapturingHTTPClient) Do(req *http.Request) (*http.Response, error) { + c.urls = append(c.urls, req.URL.String()) + return nil, context.DeadlineExceeded +} + +// scriptedHTTPClient records URLs and plays back canned responses in order — +// a nil entry means a transport error — failing any request beyond the script. +type scriptedHTTPClient struct { + urls []string + responses []*http.Response +} + +func (c *scriptedHTTPClient) Do(req *http.Request) (*http.Response, error) { + c.urls = append(c.urls, req.URL.String()) + if len(c.responses) == 0 { + return nil, context.DeadlineExceeded + } + resp := c.responses[0] + c.responses = c.responses[1:] + if resp == nil { + return nil, context.DeadlineExceeded + } + return resp, nil +} + +// drainTrackingBody reports whether the response body was read to EOF before +// being closed — the precondition for HTTP keep-alive connection reuse. +type drainTrackingBody struct { + r *strings.Reader + sawEOF bool +} + +func (b *drainTrackingBody) Read(p []byte) (int, error) { + n, err := b.r.Read(p) + if err == io.EOF { + b.sawEOF = true + } + return n, err +} + +func (b *drainTrackingBody) Close() error { return nil } + +func TestLagAwareClusterTriesAllSeedAddresses(t *testing.T) { + bdbList := `[{"uid": 3, "endpoints": [{"dns_name": "seed2.example.com", "addr": [], "port": 6379}]}]` + capture := &scriptedHTTPClient{responses: []*http.Response{ + nil, // seed1's REST API is unreachable + {StatusCode: 200, Body: io.NopCloser(strings.NewReader(bdbList))}, + {StatusCode: 200, Body: io.NopCloser(strings.NewReader(`{}`))}, + }} + hc := NewLagAwareHealthCheck(WithLagAwareHTTPClient(capture)) + cc := redis.NewClusterClient(&redis.ClusterOptions{ + Addrs: []string{"seed1.example.com:6379", "seed2.example.com:6379"}, + }) + defer cc.Close() + + // The cluster can be perfectly healthy while the first seed is down; the + // lag check must try the remaining seeds instead of failing on Addrs[0]. + ok, err := hc.CheckClusterHealth(context.Background(), cc) + if !ok || err != nil { + t.Fatalf("CheckClusterHealth = (%v, %v), want healthy via the second seed", ok, err) + } +} + +func TestLagAwareDrainsErrorResponseBody(t *testing.T) { + // Repeated failing probes (401/503 loops) must also reuse the REST + // connection: error bodies need draining exactly like success bodies. + body := &drainTrackingBody{r: strings.NewReader(`{"error_code":"unauthorized"}`)} + capture := &scriptedHTTPClient{responses: []*http.Response{ + {StatusCode: 401, Body: body}, + }} + hc := NewLagAwareHealthCheck(WithLagAwareHTTPClient(capture)) + client := redis.NewClient(&redis.Options{Addr: "redis.example.com:6379"}) + defer client.Close() + + if ok, err := hc.CheckHealth(context.Background(), client); ok || err == nil { + t.Fatalf("CheckHealth = (%v, %v), want unhealthy with an error", ok, err) + } + if !body.sawEOF { + t.Error("non-2xx response body not drained before return") + } +} + +func TestLagAwareDrainsAvailabilityBody(t *testing.T) { + bdbList := `[{"uid": 7, "endpoints": [{"dns_name": "redis.example.com", "addr": [], "port": 6379}]}]` + avail := &drainTrackingBody{r: strings.NewReader(`{"status":"ok"}`)} + capture := &scriptedHTTPClient{responses: []*http.Response{ + {StatusCode: 200, Body: io.NopCloser(strings.NewReader(bdbList))}, + {StatusCode: 200, Body: avail}, + }} + hc := NewLagAwareHealthCheck(WithLagAwareHTTPClient(capture)) + client := redis.NewClient(&redis.Options{Addr: "redis.example.com:6379"}) + defer client.Close() + + if ok, err := hc.CheckHealth(context.Background(), client); !ok || err != nil { + t.Fatalf("CheckHealth = (%v, %v), want healthy", ok, err) + } + if !avail.sawEOF { + t.Error("availability response body not drained before close: the keep-alive connection cannot be reused") + } +} + +func TestLagAwareChecksLocalEndpointAvailability(t *testing.T) { + bdbList := `[{"uid": 7, "endpoints": [{"dns_name": "redis.example.com", "addr": ["10.0.0.1"], "port": 6379}]}]` + capture := &scriptedHTTPClient{responses: []*http.Response{ + {StatusCode: 200, Body: io.NopCloser(strings.NewReader(bdbList))}, + {StatusCode: 200, Body: io.NopCloser(strings.NewReader(`{}`))}, + }} + hc := NewLagAwareHealthCheck(WithLagAwareHTTPClient(capture)) + client := redis.NewClient(&redis.Options{Addr: "redis.example.com:6379"}) + defer client.Close() + + ok, err := hc.CheckHealth(context.Background(), client) + if !ok || err != nil { + t.Fatalf("CheckHealth = (%v, %v), want healthy", ok, err) + } + if len(capture.urls) != 2 { + t.Fatalf("expected 2 REST calls, got %v", capture.urls) + } + // The availability probe must target the LOCAL ENDPOINT of the matched + // database: the database-level check reports healthy as long as ANY + // endpoint is up (without the OSS cluster API), which can mask an outage + // of the endpoint this member actually uses. + want := "https://redis.example.com:9443/v1/local/bdbs/7/endpoint/availability?extend_check=lag&availability_lag_tolerance_ms=5000" + if got := capture.urls[1]; got != want { + t.Errorf("availability URL = %q, want %q", got, want) + } +} + +func TestLagAwareIPv6BaseURL(t *testing.T) { + // An IPv6 Redis address must produce a bracketed, parseable HTTPS base URL + // (https://[::1]:9443/...), not the malformed https://::1:9443/... + capture := &urlCapturingHTTPClient{} + hc := NewLagAwareHealthCheck(WithLagAwareHTTPClient(capture)) + + client := redis.NewClient(&redis.Options{Addr: "[::1]:6379"}) + defer client.Close() + + if ok, _ := hc.CheckHealth(context.Background(), client); ok { + t.Fatal("expected CheckHealth to fail with the capturing client") + } + if len(capture.urls) == 0 { + t.Fatal("expected at least one REST API request") + } + got := capture.urls[0] + // fields keeps frequent probes cheap: the matcher only needs uid and + // endpoints, not full database configs. + want := "https://[::1]:9443/v1/bdbs?fields=uid,endpoints" + if got != want { + t.Errorf("IPv6 base URL = %q, want %q", got, want) + } + // The URL must be parseable and round-trip the IPv6 host with brackets. + u, err := url.Parse(got) + if err != nil { + t.Fatalf("constructed URL %q is not parseable: %v", got, err) + } + if u.Hostname() != "::1" { + t.Errorf("parsed hostname = %q, want ::1", u.Hostname()) + } +} + +func TestLagAwareCheckHealthReturnsError(t *testing.T) { + // CheckHealth must surface the underlying failure (here: the HTTP error) + // so health-check metrics can record why the check was unhealthy. + hc := NewLagAwareHealthCheck( + WithLagAwareHTTPClient(&mockHTTPClient{err: context.DeadlineExceeded}), + ) + client := redis.NewClient(&redis.Options{Addr: "localhost:6379"}) + defer client.Close() + + ok, err := hc.CheckHealth(context.Background(), client) + if ok { + t.Error("expected CheckHealth to return false") + } + if err == nil { + t.Error("expected CheckHealth to return a non-nil error") + } +} + +func TestLagAwareUnusableAddrReturnsError(t *testing.T) { + // A unix-socket address cannot yield a REST API host; CheckHealth must + // report this as an error rather than a silent false. + hc := NewLagAwareHealthCheck() + client := redis.NewClient(&redis.Options{Network: "unix", Addr: "/tmp/redis.sock"}) + defer client.Close() + + ok, err := hc.CheckHealth(context.Background(), client) + if ok { + t.Error("expected CheckHealth to return false for unix-socket address") + } + if err == nil { + t.Error("expected CheckHealth to return an error for unix-socket address") + } +} + +func TestGetConfigClampsInvalidValues(t *testing.T) { + // A configurable check returning non-positive Probes/Timeout (or negative + // Delay) must be clamped to defaults so probe runners stay robust. + hc := &configReturningCheck{cfg: HealthCheckConfig{Probes: 0, Timeout: 0, Delay: -1}} + got := getConfig(hc) + if got.Probes != DefaultHealthCheckProbes { + t.Errorf("Probes = %d, want clamped to %d", got.Probes, DefaultHealthCheckProbes) + } + if got.Timeout != DefaultHealthCheckTimeout { + t.Errorf("Timeout = %v, want clamped to %v", got.Timeout, DefaultHealthCheckTimeout) + } + if got.Delay != DefaultHealthCheckDelay { + t.Errorf("Delay = %v, want clamped to %v", got.Delay, DefaultHealthCheckDelay) + } + + // A check with Probes=0 must not be treated as trivially healthy. + policy := NewHealthyAllPolicy() + client := redis.NewClient(&redis.Options{Addr: "localhost:6379"}) + defer client.Close() + bad := &countingCheck{} + bad.cfg = HealthCheckConfig{Probes: 0, Timeout: time.Second} + policy.Execute(context.Background(), []redis.MultiDBHealthCheck{bad}, client) + if bad.calls == 0 { + t.Error("expected at least one probe call after clamping Probes=0 to default") + } +} + +// configReturningCheck is a ConfigurableHealthCheck that returns a fixed config. +type configReturningCheck struct { + cfg HealthCheckConfig +} + +func (c *configReturningCheck) Config() HealthCheckConfig { return c.cfg } +func (c *configReturningCheck) CheckHealth(context.Context, *redis.Client) (bool, error) { + return true, nil +} + +func (c *configReturningCheck) CheckClusterHealth(context.Context, *redis.ClusterClient) (bool, error) { + return true, nil +} + +// countingCheck records how many probe calls it received. +type countingCheck struct { + cfg HealthCheckConfig + calls int +} + +func (c *countingCheck) Config() HealthCheckConfig { return c.cfg } +func (c *countingCheck) CheckHealth(context.Context, *redis.Client) (bool, error) { + c.calls++ + return true, nil +} + +func (c *countingCheck) CheckClusterHealth(context.Context, *redis.ClusterClient) (bool, error) { + c.calls++ + return true, nil +} + +// --- Health Check Policy Tests --- + +// sequenceHealthCheck returns different results for each probe call +type sequenceHealthCheck struct { + results []bool + index int + config HealthCheckConfig +} + +func newSequenceHealthCheck(results []bool) *sequenceHealthCheck { + return &sequenceHealthCheck{ + results: results, + config: HealthCheckConfig{ + Probes: len(results), + Delay: 0, + Timeout: 3 * time.Second, + }, + } +} + +func (s *sequenceHealthCheck) Config() HealthCheckConfig { + return s.config +} + +func (s *sequenceHealthCheck) CheckHealth(ctx context.Context, client *redis.Client) (bool, error) { + if s.index >= len(s.results) { + return false, nil + } + result := s.results[s.index] + s.index++ + return result, nil +} + +func (s *sequenceHealthCheck) CheckClusterHealth(ctx context.Context, client *redis.ClusterClient) (bool, error) { + return s.CheckHealth(ctx, nil) +} + +func TestHealthCheckPolicies(t *testing.T) { + t.Run("HealthyAllPolicy requires all probes to pass", func(t *testing.T) { + policy := NewHealthyAllPolicy() + client := redis.NewClient(&redis.Options{Addr: "localhost:6379"}) + defer client.Close() + + // All probes pass + hc := newSequenceHealthCheck([]bool{true, true, true}) + checks := []redis.MultiDBHealthCheck{hc} + if !policy.Execute(context.Background(), checks, client) { + t.Error("expected all passing probes to return true") + } + + // One probe fails + hc = newSequenceHealthCheck([]bool{true, false, true}) + checks = []redis.MultiDBHealthCheck{hc} + if policy.Execute(context.Background(), checks, client) { + t.Error("expected one failing probe to return false") + } + }) + + t.Run("HealthyMajorityPolicy requires majority of probes to pass", func(t *testing.T) { + policy := NewHealthyMajorityPolicy() + client := redis.NewClient(&redis.Options{Addr: "localhost:6379"}) + defer client.Close() + + // 3 probes: 2 pass, 1 fails - should succeed (majority) + hc := newSequenceHealthCheck([]bool{true, false, true}) + checks := []redis.MultiDBHealthCheck{hc} + if !policy.Execute(context.Background(), checks, client) { + t.Error("expected 2/3 passing probes to return true") + } + + // 3 probes: 1 passes, 2 fail - should fail + hc = newSequenceHealthCheck([]bool{true, false, false}) + checks = []redis.MultiDBHealthCheck{hc} + if policy.Execute(context.Background(), checks, client) { + t.Error("expected 1/3 passing probes to return false") + } + }) + + t.Run("HealthyAnyPolicy requires at least one probe to pass", func(t *testing.T) { + policy := NewHealthyAnyPolicy() + client := redis.NewClient(&redis.Options{Addr: "localhost:6379"}) + defer client.Close() + + // First probe fails, second passes - should succeed + hc := newSequenceHealthCheck([]bool{false, true, false}) + checks := []redis.MultiDBHealthCheck{hc} + if !policy.Execute(context.Background(), checks, client) { + t.Error("expected one passing probe to return true") + } + + // All probes fail - should fail + hc = newSequenceHealthCheck([]bool{false, false, false}) + checks = []redis.MultiDBHealthCheck{hc} + if policy.Execute(context.Background(), checks, client) { + t.Error("expected no passing probes to return false") + } + }) + + t.Run("Empty checks return true for all policies", func(t *testing.T) { + client := redis.NewClient(&redis.Options{Addr: "localhost:6379"}) + defer client.Close() + ctx := context.Background() + + var checks []redis.MultiDBHealthCheck + + if !NewHealthyAllPolicy().Execute(ctx, checks, client) { + t.Error("HealthyAllPolicy should return true for empty checks") + } + if !NewHealthyMajorityPolicy().Execute(ctx, checks, client) { + t.Error("HealthyMajorityPolicy should return true for empty checks") + } + if !NewHealthyAnyPolicy().Execute(ctx, checks, client) { + t.Error("HealthyAnyPolicy should return true for empty checks") + } + }) + + t.Run("Multiple health checks all must pass", func(t *testing.T) { + policy := NewHealthyAllPolicy() + client := redis.NewClient(&redis.Options{Addr: "localhost:6379"}) + defer client.Close() + + // Two health checks, both pass + checks := []redis.MultiDBHealthCheck{ + newSequenceHealthCheck([]bool{true, true, true}), + newSequenceHealthCheck([]bool{true, true, true}), + } + if !policy.Execute(context.Background(), checks, client) { + t.Error("expected both health checks to pass") + } + + // Two health checks, one fails + checks = []redis.MultiDBHealthCheck{ + newSequenceHealthCheck([]bool{true, true, true}), + newSequenceHealthCheck([]bool{true, false, true}), // fails with AllPolicy + } + if policy.Execute(context.Background(), checks, client) { + t.Error("expected one failing health check to return false") + } + }) +} + +func TestHealthCheckConfig(t *testing.T) { + t.Run("DefaultHealthCheckConfig has correct values", func(t *testing.T) { + cfg := DefaultHealthCheckConfig() + if cfg.Probes != DefaultHealthCheckProbes { + t.Errorf("expected Probes=%d, got %d", DefaultHealthCheckProbes, cfg.Probes) + } + if cfg.Delay != DefaultHealthCheckDelay { + t.Errorf("expected Delay=%v, got %v", DefaultHealthCheckDelay, cfg.Delay) + } + if cfg.Timeout != DefaultHealthCheckTimeout { + t.Errorf("expected Timeout=%v, got %v", DefaultHealthCheckTimeout, cfg.Timeout) + } + }) + + t.Run("WithProbes sets probes", func(t *testing.T) { + hc := NewPingHealthCheck(WithProbes(5)) + if hc.Config().Probes != 5 { + t.Errorf("expected Probes=5, got %d", hc.Config().Probes) + } + }) + + t.Run("WithDelay sets delay", func(t *testing.T) { + hc := NewPingHealthCheck(WithDelay(100 * time.Millisecond)) + if hc.Config().Delay != 100*time.Millisecond { + t.Errorf("expected Delay=100ms, got %v", hc.Config().Delay) + } + }) + + t.Run("WithTimeout sets timeout", func(t *testing.T) { + hc := NewPingHealthCheck(WithTimeout(5 * time.Second)) + if hc.Config().Timeout != 5*time.Second { + t.Errorf("expected Timeout=5s, got %v", hc.Config().Timeout) + } + }) +} + +// panicHealthCheck panics on every probe, simulating a buggy check. +type panicHealthCheck struct{} + +func (panicHealthCheck) CheckHealth(context.Context, *redis.Client) (bool, error) { + panic("panicHealthCheck: boom") +} + +func (panicHealthCheck) CheckClusterHealth(context.Context, *redis.ClusterClient) (bool, error) { + panic("panicHealthCheck: boom") +} + +func TestRunChecksRecoversFromPanic(t *testing.T) { + // A panicking check must be treated as unhealthy rather than dropping its + // result: otherwise the consumer could return true after fewer than + // len(checks) results. + client := redis.NewClient(&redis.Options{Addr: "localhost:6379"}) + defer client.Close() + + checks := []redis.MultiDBHealthCheck{ + newSequenceHealthCheck([]bool{true, true, true}), + panicHealthCheck{}, + } + if NewHealthyAllPolicy().Execute(context.Background(), checks, client) { + t.Error("expected a panicking health check to make the policy report unhealthy") + } +} + +func TestLagAwareHTTPClientTimeout(t *testing.T) { + t.Run("defaults to DefaultHTTPTimeout", func(t *testing.T) { + hc := NewLagAwareHealthCheck() + client, ok := hc.httpClient.(*http.Client) + if !ok { + t.Fatalf("expected default *http.Client, got %T", hc.httpClient) + } + if client.Timeout != DefaultHTTPTimeout { + t.Errorf("http client timeout = %v, want %v", client.Timeout, DefaultHTTPTimeout) + } + }) + + t.Run("honors a larger probe timeout", func(t *testing.T) { + hc := NewLagAwareHealthCheck( + WithLagAwareHealthCheckConfig(WithTimeout(30 * time.Second)), + ) + client, ok := hc.httpClient.(*http.Client) + if !ok { + t.Fatalf("expected default *http.Client, got %T", hc.httpClient) + } + if client.Timeout < 30*time.Second { + t.Errorf("http client timeout = %v, want >= 30s", client.Timeout) + } + }) +} + +func TestLagAwareTLSConfigIsCloned(t *testing.T) { + // The transport must use a clone of the supplied tls.Config so a later + // mutation by the caller cannot race the transport's use of it. + cfg := &tls.Config{InsecureSkipVerify: true} + hc := NewLagAwareHealthCheck(WithLagAwareTLSConfig(cfg)) + + client, ok := hc.httpClient.(*http.Client) + if !ok { + t.Fatalf("expected default *http.Client, got %T", hc.httpClient) + } + transport, ok := client.Transport.(*http.Transport) + if !ok { + t.Fatalf("expected *http.Transport, got %T", client.Transport) + } + if transport.TLSClientConfig == cfg { + t.Error("expected transport TLS config to be a clone, not the same pointer") + } + if transport.TLSClientConfig == nil || !transport.TLSClientConfig.InsecureSkipVerify { + t.Error("expected cloned TLS config to preserve InsecureSkipVerify") + } +} diff --git a/multidb_healthcheck.go b/multidb_healthcheck.go new file mode 100644 index 0000000000..9bcb8516d0 --- /dev/null +++ b/multidb_healthcheck.go @@ -0,0 +1,18 @@ +package redis + +import "context" + +// MultiDBHealthCheck is the interface for health checking databases. +// +// Each method reports whether the database is healthy together with the error +// that made it unhealthy (if any). A healthy result is (true, nil). An +// unhealthy result is (false, err) where err explains the failure so callers +// (e.g. health-check metrics) can record why a check failed; err may be nil +// when the check completed and simply determined the database is not healthy. +type MultiDBHealthCheck interface { + // CheckHealth checks if a standalone client is healthy. + CheckHealth(ctx context.Context, client *Client) (bool, error) + + // CheckClusterHealth checks if a cluster client is healthy. + CheckClusterHealth(ctx context.Context, client *ClusterClient) (bool, error) +} diff --git a/otel.go b/otel.go index 1ea359364f..dd4f9ba1f4 100644 --- a/otel.go +++ b/otel.go @@ -98,6 +98,32 @@ type OTelConnectionCounter interface { RecordPendingRequests(ctx context.Context, delta int, cn ConnInfo, poolName string) } +// OTelMultiDBRecorder is an optional capability interface for recording +// MultiDB (geo-failover) metrics. Implementations of OTelRecorder can +// optionally implement this interface to receive MultiDB notifications. +// This is kept separate from OTelRecorder to avoid breaking existing +// third-party implementations when new methods are added. +type OTelMultiDBRecorder interface { + // RecordMultiDBFailover records a MultiDB (geo-failover) failover from one + // member database to another. fromFQDN/toFQDN are host-only database FQDNs, + // reason is "automatic" or "manual", and duration is the wall time of the + // failover. + RecordMultiDBFailover(ctx context.Context, fromFQDN, toFQDN, reason string, duration time.Duration) + + // RecordMultiDBActiveDatabaseChange records a change of the active MultiDB + // member database (failover, fallback, or manual selection). fromFQDN/toFQDN + // are host-only database FQDNs. + RecordMultiDBActiveDatabaseChange(ctx context.Context, fromFQDN, toFQDN string) + + // RecordMultiDBCircuitStateChange records a MultiDB circuit breaker state + // transition for the database identified by dbFQDN. + RecordMultiDBCircuitStateChange(ctx context.Context, dbFQDN, fromState, toState string) + + // RecordMultiDBHealthCheck records the result of a MultiDB health-check pass + // for the database identified by dbFQDN. duration is the wall time of the check. + RecordMultiDBHealthCheck(ctx context.Context, dbFQDN string, success bool, duration time.Duration) +} + // This is used for async gauge metrics that need to pull stats from pools periodically. type OTelPoolRegistrar interface { // RegisterPool is called when a new client is created with its main connection pool. @@ -182,6 +208,30 @@ func (a *otelRecorderAdapter) RecordStreamLag(ctx context.Context, lag time.Dura a.recorder.RecordStreamLag(ctx, lag, toConnInfo(cn), streamName, consumerGroup, consumerName) } +func (a *otelRecorderAdapter) RecordMultiDBFailover(ctx context.Context, fromFQDN, toFQDN, reason string, duration time.Duration) { + if r, ok := a.recorder.(OTelMultiDBRecorder); ok { + r.RecordMultiDBFailover(ctx, fromFQDN, toFQDN, reason, duration) + } +} + +func (a *otelRecorderAdapter) RecordMultiDBActiveDatabaseChange(ctx context.Context, fromFQDN, toFQDN string) { + if r, ok := a.recorder.(OTelMultiDBRecorder); ok { + r.RecordMultiDBActiveDatabaseChange(ctx, fromFQDN, toFQDN) + } +} + +func (a *otelRecorderAdapter) RecordMultiDBCircuitStateChange(ctx context.Context, dbFQDN, fromState, toState string) { + if r, ok := a.recorder.(OTelMultiDBRecorder); ok { + r.RecordMultiDBCircuitStateChange(ctx, dbFQDN, fromState, toState) + } +} + +func (a *otelRecorderAdapter) RecordMultiDBHealthCheck(ctx context.Context, dbFQDN string, success bool, duration time.Duration) { + if r, ok := a.recorder.(OTelMultiDBRecorder); ok { + r.RecordMultiDBHealthCheck(ctx, dbFQDN, success, duration) + } +} + func (a *otelRecorderAdapter) RecordConnectionCount(ctx context.Context, delta int, cn *pool.Conn, state string, isPubSub bool) { if counter, ok := a.recorder.(OTelConnectionCounter); ok { counter.RecordConnectionCount(ctx, delta, toConnInfo(cn), state, isPubSub)