Fix cluster pipeline deadlock on connection acquisition

[lh0156]

Fixes #4557.

Problem

I added deterministic coverage for the cluster pipeline deadlock condition described in #4557.

The problematic condition is:

1. pipeline A borrows a connection to node A and writes a command
2. pipeline B borrows a connection to node B and writes a command
3. pipeline A then tries to borrow node B
4. pipeline B then tries to borrow node A

If each node pool has no immediately available connection, both pipelines can block before sync() while still holding previously borrowed node connections with pending responses. This creates a hold-and-wait cycle.

Options considered

I considered a few possible approaches:

• buffering commands by node and acquiring connections later in deterministic order
• draining/releasing already-held connections before waiting for another node
• failing fast when a cluster pipeline already holds a node connection and cannot immediately acquire a new node connection

I chose the fail-fast approach because it is the smallest change that directly breaks the deadlock condition. Buffering would change the eager-write behavior of pipeline commands, and drain/release would make partial response handling significantly more complex.

Fix

This does not make all cluster pipeline acquisition non-blocking. The first node acquisition keeps the existing behavior, because the pipeline does not hold any node connection yet.

For subsequent node acquisitions, ClusterPipeline now uses a zero-wait acquisition path. If the new node connection is not immediately available, the pipeline fails fast instead of waiting while holding another node connection.

When this happens, the already-held connections are marked broken before being closed, because commands may already have been written and their responses may still be pending. Returning those connections as healthy could contaminate the pool.

Why this prevents the deadlock

The deadlock requires a pipeline to hold one node connection while waiting for another node connection.

This change removes that condition from the cluster pipeline path:

• holding no connection: blocking acquisition is still allowed
• holding one or more node connections: acquisition of a different node must succeed immediately or fail fast

So two pipelines can no longer wait on each other while each keeps a different node connection checked out.

Tests

Added focused unit coverage for:

• the existing blocking acquisition model that can circularly block before sync()
• first node acquisition still allowing blocking
• second node acquisition using the non-blocking/fail-fast path
• held connections being marked broken and closed on fail-fast
• close() not attempting to read pending responses after fail-fast cleanup
• same-node commands reusing the existing connection without zero-wait acquisition
• fail-fast error message guidance
• ConnectionPool#getResource(Duration.ZERO) failing fast when exhausted

Verified locally with:

mvn -Dtest=MultiNodePipelineBaseDeadlockTest test

Result:

Tests run: 15, Failures: 0, Errors: 0, Skipped: 0

---

Note

Medium Risk
Changes connection acquisition semantics for multi-shard cluster pipelines—pools may see more immediate failures under contention—but behavior is scoped to pipeline use and includes explicit error guidance and broken-connection handling.

Overview
Fixes cluster pipeline deadlocks (#4557) when concurrent pipelines each hold one shard connection and then block borrowing another.

MultiNodePipelineBase only allows blocking pool borrow for the first node in a pipeline; further nodes go through getConnection(nodeKey, allowBlocking). On non-blocking acquisition failure it clears held connections, marking them broken when responses are still pending so pools are not poisoned.

ClusterPipeline uses normal acquisition for the first node and Duration.ZERO borrows for additional nodes via ClusterConnectionProvider, surfacing a JedisClusterOperationException with deadlock/pool tuning guidance when no connection is immediately available.

Pool / ConnectionPool gain getResource(Duration) to support bounded waits. MultiNodePipelineBaseDeadlockTest documents the old circular-wait behavior and asserts the new policy end-to-end.

^{Reviewed by Cursor Bugbot for commit b0120ce. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

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

[ggivo]

@lh0156
Thanks for the PR. The team is a bit stretched at the moment, and I’ll be away for a few days this week. We’ll get to it as soon as we can.

[ggivo]

Not sure there is a reliable way of detecting a deadlock.
With the suggested "fail-fast approach implementation," we are changing ClusterPipeline behaviour to discard pool-level configured maxWait. Means that now, in the case of a loaded pool, we can close pipelines preemptively.

I think having a dedicated client with a configured maxWait can cover this scenario without the need for a code change.