RedisCluster: Per-node health tracking and non-blocking initialize()

[ngabhanenetskope]

Summary

Adds two mechanisms to improve RedisCluster resilience during transient node failures under high concurrency:

1. Per-node health tracking (NodeHealthManager) — After N consecutive TimeoutError/ConnectionError on a node (default: 5), subsequent requests to that node fail immediately instead of blocking for socket_timeout. After recovery_time (default: 10s), one probe request is allowed through (half-open pattern). On success, the node is marked available again.

2. Non-blocking initialize() — Changed from blocking lock acquisition to try-lock. If another thread is already refreshing topology, additional threads return immediately using stale (but functional for healthy nodes) topology instead of queuing on _initialization_lock.

Motivation

Under high concurrency (e.g., gevent with 250 greenlets, max_connections_per_node=50, socket_timeout=5s), a single node failure causes:

1. 50 greenlets block on the dead node for 5s each (pool saturates instantly)
2. All 50 timeout simultaneously and call initialize() (thundering herd)
3. All queue on _initialization_lock, serializing 50 redundant CLUSTER SLOTS calls
4. Health check can't respond → readiness probe fails → pod dies

With this fix:

• Health tracking: greenlet RFE: Sharding #6 fails instantly (0ms) instead of blocking 5s → pool never saturates beyond 5 connections
• Non-blocking initialize: only the first thread refreshes topology; others continue serving requests to healthy nodes

See #4074 for full analysis.

Configuration

rc = RedisCluster(
    startup_nodes=[...],
    node_health_failure_threshold=5,   # failures before marking unavailable
    node_health_recovery_time=10.0,    # seconds before allowing probe
)

Behavior

Scenario	Before	After
6th request to dead node	Blocks for socket_timeout (5s)	Instant ConnectionError (0ms)
50 threads calling initialize()	All queue, 50 serial CLUSTER SLOTS calls	1 refreshes, 49 return immediately
Node recovers after failover	Must wait for blocked threads to timeout	Probe succeeds after recovery_time → instant recovery

Tests

• TestNodeHealthManager — 7 tests covering threshold, recovery, per-node isolation, and cluster integration

Related

• Fixes RedisCluster: Transient node failure causes thundering herd on initialize(), leading to pool exhaustion under high concurrency #4074
• Complements PR Randomize cluster startup node order during topology refresh #4060 (randomize nodes in initialize)
• Follow-up to PR Fix ConnectionPool to raise MaxConnectionsError instead of Connection… #3698 (MaxConnectionsError distinction)

[jit-ci]

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

[cursor]

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Reviewed by Cursor Bugbot for commit ec068bd. Configure here.}

[cursor]

Health check error triggers unnecessary pool disconnect and reinitialize

Medium Severity

The ConnectionError raised by the health check is caught by the except (ConnectionError, TimeoutError) handler below, which then executes pool disconnection under pool._lock, calls nodes_manager.initialize(), and calls move_node_to_end_of_cached_nodes — all unnecessary for a synthetic fast-fail error. Additionally, ConnectionError is in ERRORS_ALLOW_RETRY, so the outer _internal_execute_command loop retries the same unavailable node multiple times, each time triggering pool manipulation and topology refresh. Under 250 greenlets with 3 retries, this causes ~1000 pool._lock acquisitions and initialize() calls per unavailable node, significantly undermining the "fail fast in 0ms" design goal.

Additional Locations (1)

• redis/cluster.py#L1719-L1764

 

^{Reviewed by Cursor Bugbot for commit ec068bd. Configure here.}

[cursor]

Half-open state allows all requests, not single probe

Medium Severity

The half-open transition in is_available deletes the node from _node_unavailable_since and returns True. After this first call, every subsequent concurrent call also returns True (the node is no longer in the dict), allowing all requests through simultaneously — not just one probe as documented and intended. Under 250 greenlets, this means the potentially-still-dead node gets flooded after recovery_time, causing the same pool saturation the circuit breaker aims to prevent.

 

^{Reviewed by Cursor Bugbot for commit ec068bd. Configure here.}