[Grivn]

Description of change

Fixes #4443.

Draining a cache entry's checked-out owner can consume another command's reply and leave that request waiting. Track the owning cache proxy and its checkout state. Idle-owner drains and checkout share a per-connection lock, including across pools that share a cache. Checkout waits outside the pool-wide lock so unrelated connections can still be checked out or returned.

When the cached entry belongs to the calling connection, a hit reuses the initial invalidation drain and avoids a second socket poll and owner-lock acquisition. Concurrent readers can use a busy owner's cached value without draining its socket or moving ownership. A 100 ms grace period, renewed only by the owner, prevents a connection held idle from keeping stale entries indefinitely. Peer hits do not extend it. Pending wire replies, PubSub, blocking commands and packed commands use a fresh read. An idle owner still processes invalidations before a hit; the hit requires the same cache entry after that check. The grace deadline is initialized before the entry becomes valid.

Idle AUTH and MOVING paths claim ownership without blocking under the pool lock. If a drain already holds the connection, they defer work through the existing re-auth token or reconnect flag. The connection lock is released before socket I/O, and release retains ownership until its event dispatch finishes, including when authentication raises. Cached release listeners run outside the pool lock while the connection remains active and unavailable for checkout. Reentrant release is a no-op; close still finds the connection. A successful idle AUTH clears an older deferred token in both sync and async pools, while errors and cancellation retain it. A newer token arriving during a cached release is kept until the old AUTH listener finishes. Checkout waits for idle maintenance to finish before using the socket. Pool disconnect joins idle drains outside the pool lock, and a stale cache candidate cannot reconnect a disconnected owner. Owners inherited from another process are rejected before taking their locks. Default-cache invalidation holds the existing cache lock across holder lookup and removal so independent owners can invalidate the same key safely.

Maintenance reconfiguration joins idle drains outside the pool lock, deferring reconnect if checkout wins the race. Direct and free-only pool-close completion callbacks run after releasing connection ownership and the pool lock, so a listener can close the pool again. Parser-generated maintenance events are also queued per thread while a peer owns an idle connection: notification state updates and cache invalidations stay inside the lock, and application listeners run after it releases, including when parsing fails. SMIGRATED removed-node cleanup uses the ownership-aware pool disconnect helper; a busy idle owner is marked for reconnect instead of losing its parser mid-read. Maintenance listeners from topology cleanup and idle AUTH run after the enclosing topology/pool locks and idle claims release. Interrupted ownership waits return the reserved slot without releasing a connection still owned by maintenance. Pool shutdown also preserves support for custom connection classes that do not implement maintenance notifications.

Validation on e3de6a2:

• All default test stages pass: fixed-client (3,591), standalone (3,735), and cluster (2,031): 9,357 passed, 1,333 skipped and 8 non-strict xpasses. Ruff check/format and Vulture pass.
• 1,223 focused cases pass, including both RESP3 parsers, both pool types, all three tracking modes, caches, and synchronous/asynchronous connection pools and maintenance handling. All 114 new cases pass; 86 fail on exact dcb8ceb, and the other 28 preserve authentication-error and cancellation behavior. They cover release listeners closing the pool during a peer MOVING drain, recursive release, reconnect completion callbacks, successful idle AUTH superseding a deferred token, and a newer refresh arriving during release. Earlier ownership, fork, custom-connection and reply-alignment regressions remain enabled.
• All 12 concurrent read/write stress cases pass without wrong replies, logged push-handler errors or hangs.

Warmed-cache throughput: five interleaved 1.5-second samples per revision, pool and reader count, measured from the exact final sources. Medians in GET/s:

Pool	Readers	Merge base 3a6c462	Previous dcb8ceb	Master be1af1d	e3de6a2
ConnectionPool	1	80,324	82,366	87,887	78,686
ConnectionPool	2	40,532	59,927	38,437	58,881
ConnectionPool	8	16,911	27,578	17,763	27,487
ConnectionPool	16	16,687	32,683	17,577	32,251
BlockingConnectionPool	1	76,082	77,798	84,819	78,606
BlockingConnectionPool	2	36,798	57,206	36,600	58,280
BlockingConnectionPool	8	28,958	27,322	28,993	27,568
BlockingConnectionPool	16	29,125	32,219	27,858	31,741

All 160 samples made zero server GETs after warming. Single-reader medians changed by -4.5% for ConnectionPool and +1.0% for BlockingConnectionPool relative to dcb8ceb. The cached release now uses a second short pool-lock section to keep callbacks outside the lock; event deferral is allocated only on an actual reconnect, rather than every release. These short samples include scheduling variance and do not establish zero overhead or a zero miss rate outside this workload. Master contains newer changes, so comparisons with it do not isolate this fix.

The default suites used Python 3.13.18, pytest 9.1.1 and the plain parser on Ubuntu 24.04 in a four-vCPU task-owned VM with the repository's Redis 8.10.2 Docker test services (modules, TLS and a six-node cluster). Focused parser coverage and throughput measurements used hiredis 3.5.0. Benchmark variants ran serially with no concurrent tests. The full protocol/response matrix, alternate Python versions, uvloop and external scenario suites were not run. Upstream CI is tracked separately.

Pull Request check-list

• Do tests and lints pass with this change? (All default test stages and configured linters pass.)
• Do the CI tests pass with this change (enable it first in your forked repo and wait for the github action build to finish)?
• Is the new or changed code fully tested? (Regression coverage and default integration suites pass; broader matrix limits are listed above.)
• Is a documentation update included (if this change modifies existing APIs, or introduces new ones)? (No public method signatures change.)
• Is there an example added to the examples folder (if applicable)? (Not applicable.)

Prepared with AI assistance; the checks listed above were run locally.

---

Note

High Risk
Changes lock ordering, socket access rules, and cache invalidation semantics across connection pools, shared caches, maintenance notifications, and re-auth—core concurrency paths where regressions would cause stale reads, hung checkouts, or deadlocks.

Overview
Fixes client-side cache correctness and deadlocks when multiple connections share a cache by tracking cache ownership on CacheProxyConnection (not the raw socket) and coordinating checkout, invalidation drains, and pool maintenance with a per-connection _in_use_lock.

Cache hits: Peers no longer read a checked-out owner's socket to drain invalidations. Idle owners are drained under the owner lock; busy owners can still serve hits for a short grace window (owner-only renewal) so concurrent readers avoid extra wire traffic without letting PubSub/blocking commands serve stale data indefinitely. Invalidation deletes in DefaultCache are now atomic under the cache lock so concurrent invalidations for the same Redis key stay safe.

Pool lifecycle: Checkout waits for idle maintenance outside the pool lock; release of cached connections keeps ownership through release events and deferred AUTH, with _releasing_connections so token refresh during release keeps the newest token. Pool disconnect, re-auth on idle connections, maintenance config updates, and cluster SMIGRATED cleanup join or defer idle-owner drains and dispatch maintenance completion listeners only after locks release via _defer_maintenance_events. Successful idle re-auth clears the deferred token; failures preserve the previous token.

Extensive new test_cache_pool.py coverage exercises these concurrency and listener scenarios; existing unit tests now expect connection_ref to point at the proxy.

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

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-05T06:46:29.657393Z	e3de6a2	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[petyaslavova]

Hey @Grivn, thank you for your contribution!

The fix is directionally right. Never draining a checked-out owner closes the reply theft from #4443, and it also covers owners held by PubSub or a blocking command such as BLPOP key 0. Using the owner's own pool lock also fixes the cluster case, where the cache is shared across node pools.

However, I measured a regression that blocks the merge. With two or more threads reading through one client, the cache stops serving hits. Each miss moves ownership to the requesting connection, and the next reader from another thread finds that owner checked out. With no writes at all, 2 readers reached a 99% miss ratio, and 8-16 readers reached 100% on both pool classes, with throughput down 30-55% compared with master. Single-threaded use is unaffected.

Please change the busy-owner path so concurrent readers still get hits. For example, serve the cached value without draining when the owner is checked out, and keep the fresh read only where staleness could be unbounded. Please also add a test that asserts concurrent readers keep hitting the cache. Optionally, the _lock acquire in BlockingConnectionPool.release is not needed just to clear _in_use.

[Grivn]

Thanks @petyaslavova. I reproduced this with a warmed key and no writes: two-second local runs with 2 and 8 readers sent 93–98% of reads to Redis on this branch, versus none on the unchanged base, for both pools. My earlier stress checks covered wrong replies and hangs but missed cache-hit assertions.

I'll revise the busy-owner path and add a concurrent-reader hit test alongside the reply-preservation tests. Serving every busy-owner entry would leave PubSub and indefinite blocking commands without a freshness bound, so those cases still need a fallback. I agree this needs fixing before merge.

[Grivn]

Updated in 56d0728, including the merge with current master.

Busy owners can now serve hits without touching their sockets or moving cache ownership. Sharing has a 100 ms limit since the owner's last invalidation check or cache-fill completion; another reader's hit cannot extend it. This also bounds stale reads when a checked-out connection is left idle. Pending wire replies, PubSub, blocking commands and packed commands keep the fresh-read fallback.

Added concurrent-hit assertions for 2 and 8 readers, plus expiry, PubSub and blocking-command coverage. The original reply-preservation tests are unchanged. With the final commit, 1/2/8/16-reader runs on both pools made zero extra GET requests after warming the key (470,005 reads total). The 12 read/write stress cases also passed.

All three default test stages pass: 8,683 passed, 929 skipped and 8 non-strict xpasses. Ruff and Vulture pass too.

[cursor]

Stale Bugbot comment from a previous run.

[petyaslavova]

Thanks for the update. I re-ran my benchmark on b229c11. Concurrent readers now get cache hits on both pool classes, with zero server GETs on a warmed key at 2, 8 and 16 readers. The #4443 repro also completes without hangs or wrong replies.

One regression remains. BlockingConnectionPool throughput with 8-16 readers is 14-17% below master (about 22.5k vs 26.5k ops/s), while ConnectionPool is faster. Most of the cost comes from the new _lock acquire around _in_use = True in get_connection. Every checkout now waits on the lock that peers hold while draining an idle owner. Please reduce that cost, for example with a per-connection lock shared by the drain and checkout instead of the pool-wide _lock. Please also drop the _lock acquire in BlockingConnectionPool.release, since nothing touches the socket at that point.

A small one: the _BUSY_CACHE_GRACE_SECONDS comment says active readers renew the window, but only the owner's own commands do. Peer hits do not.

[Grivn]

@petyaslavova Fixed in 53b86ce. Checkout and idle-owner drains now share a per-connection lock. Both pools release the pool lock before waiting for a drain, and BlockingConnectionPool.release no longer acquires _lock. The grace-window comment now says only the owner renews it.

Five interleaved 1.5-second runs per case on macOS arm64, Python 3.13.12 and Redis 8.10.2 gave these median warmed-key rates:

Blocking-pool readers	b229c11	53b86ce	master afe7fc7
8	29.1k/s	46.2k/s	33.2k/s
16	29.4k/s	46.9k/s	33.5k/s

All read-only cases had zero extra GETs. Added regressions show unrelated checkout/release can proceed during a drain while checkout of the owner still waits, including shared caches. The default test stages passed 9,216 tests; Ruff, Vulture and all 12 read/write stress cases passed. These are local results; upstream CI still needs to run.

[cursor]

Stale Bugbot comment from a previous run.

[petyaslavova]

Thanks for the update. All three points from my last review are addressed: checkout and the drain share a per-connection lock, BlockingConnectionPool.release no longer takes _lock, and the grace-window comment is accurate.

One regression remains, which Bugbot also flagged. While the drain held the pool lock, it could not overlap the pool paths that use an idle connection's socket under _lock. Now re_auth_callback can send AUTH while a peer drains the same connection, and so can re_auth() from AfterConnectionReleasedEvent, because release clears _in_use before dispatching it. The drain can consume the AUTH reply and leave read_response() blocked while _lock is held, which stalls the pool. disconnect_free_connections on MOVING has the same overlap. A streaming credential provider or maintenance notifications, combined with concurrent cache readers, is enough to hit this.

Please have these paths claim the connection the same way checkout does: set _in_use under _in_use_lock before the socket I/O, without holding that lock during it. In release, clear _in_use only after the event dispatch. Please also add a regression test where a drain overlaps re_auth_callback on an idle owner.

[Grivn]

@petyaslavova Agreed: release must keep _in_use set through the event, and idle AUTH/disconnect need ownership before touching the socket.

I added an overlap regression and checked a local variant that sets _in_use under the connection lock, then releases that lock before AUTH. There is also a lock-order issue to handle: re_auth_callback still holds the pool lock while waiting for an existing peer drain, and that drain can dispatch a maintenance notification needing the pool lock. All 12 parser/tracking/shared-pool cases caught that wait. The MOVING path also enters disconnect_free_connections with outer locks held.

The repair needs to reserve idle connections against checkout before waiting outside the pool lock. I haven't pushed this variant; the reply-safety and lock-order regressions both need to pass first.

[petyaslavova]

Thanks for catching this. Note that the branch is still at 53b86ce, so none of the changes from my last review are on the PR yet. You're right that my suggestion would deadlock: re_auth_callback and the MOVING path (run_proactive_reconnect -> disconnect_free_connections) run with the pool lock held, and an idle-owner drain that holds _in_use_lock can dispatch a MOVING notification whose handler takes that same pool lock.

I don't think these paths need to wait for the drain, though. Under the pool lock, try _in_use_lock.acquire(blocking=False). If it succeeds, set _in_use = True, release the lock, do the socket I/O and clear _in_use afterwards. If a drain holds it, fall back to the deferred handling that in-use connections already get: set_re_auth_token() for re-auth and mark_for_reconnect() for MOVING. Then nothing blocks under the pool lock, and the maintenance handler needs no restructuring, which reserving connections outside the pool lock would require for the MOVING path.

When you push, please also make release clear _in_use only after the event dispatch, and add regressions for a drain overlapping re_auth_callback on an idle owner, and for a drain that dispatches MOVING while disconnect_free_connections runs.

[Grivn]

@petyaslavova Pushed 1afc08d. Idle AUTH and MOVING now try a nonblocking ownership claim and defer through set_re_auth_token() or mark_for_reconnect() if a drain holds the connection. The connection lock is released before socket I/O. Release keeps _in_use set until event dispatch completes, including the error path.

The 60 added overlap cases fail on 53b86ce and pass here. All 228 cache cases pass across both pools, both parsers and all tracking modes; the 108 connection-pool and 80 maintenance-notification tests also pass. The three default test stages and repository linters pass with Python 3.13.18, pytest 9.1.1 and Redis 8.10.2. The full protocol/response matrix has not been run locally; the new head's workflows await approval.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1afc08d2c8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[cursor]

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

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

[Grivn]

Pushed a follow-up in 4236732. Checkout now waits for idle maintenance, pool disconnect joins drains outside the pool lock, and cache candidates cannot reconnect closed owners or acquire an inherited owner's lock after fork. Shared-cache invalidation is atomic as well; the reproduced KeyError was caught and logged by the parser, rather than directly failing the client read.

All 72 added cases pass; 66 fail on 1afc08d, and six are ordinary-pool controls. The default stages pass 9,171 tests, the focused suite passes 639, and linters and all 12 read/write stress cases pass. Five interleaved throughput runs per case show a 1.9–3.8% cost at 1–2 readers and less than 0.7% difference at 8–16 readers, with zero extra GETs in the final samples. The full protocol/response matrix has not been run locally.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 423673206d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Grivn]

Pushed 4b7c518 to cover three more ownership paths: maintenance reconfiguration now joins idle drains outside the pool lock, completion listeners run after pool disconnect releases the owner lock, and interrupted checkouts restore their reserved slot.

All 60 added cases pass; 48 fail on 4236732. The 9,201 default tests, 699 focused tests, linters and 12 read/write stress cases also pass. The description includes the repeated throughput comparison and the one sample with extra server GETs.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4b7c5181ec

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[petyaslavova]

Thanks for the update. I checked 1afc08d, 4236732 and 4b7c518. All of the points from my last review are addressed: idle AUTH and MOVING claim the connection without blocking and fall back to deferral, release keeps _in_use set through the event dispatch, and the overlap regressions are in place. I re-ran my benchmark against the merge base. With 2-16 readers, both pools are now at or above it, with zero server GETs on a warmed key, and the #4443 repro completes on both pools.

Two items remain:

1. The open Codex comment about custom connection classes is real, and I reproduced it. _disconnect_from_pool calls _reset_maintenance_on_disconnect() without the isinstance(self._conn, MaintNotificationsAbstractConnection) guard that CacheProxyConnection.disconnect uses. So pool.disconnect() and close() raise NotImplementedError and leave the socket open when the connection class does not inherit the maintenance mixin. Please add the same guard and a regression test.
2. With a single reader, ConnectionPool is about 7% slower than the merge base, and BlockingConnectionPool about 3%. When owner is self, the hit path does not need _in_use_lock or the pid and is_connected checks: nothing else touches the socket of a checked-out connection, and send_command has just drained it. Please add a fast path for that case and include a 1-reader comparison against master.

Please also resolve the Codex threads that the latest commits fixed: the shared-cache invalidation deletion, the inherited cache owners after fork, the maintenance-config disconnects, the completion listeners closing the pool, and the interrupted ownership wait. Keep the custom connection class thread open until item 1 is fixed.

After that, the PR needs the upstream CI run.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b34798d58f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Grivn]

@petyaslavova The custom-connection guard landed in b34798d. 185793e adds the own-connection hit path: it reuses the initial invalidation drain and skips the owner lock, PID check and connected check. It also defers free-disconnect completion callbacks until idle ownership and the pool lock are released, covering the newer reentrant-close finding.

Single-reader medians from five interleaved 1.5-second runs per revision and pool:

Revision	ConnectionPool GET/s	BlockingConnectionPool GET/s
merge base 3a6c462	79,760	76,643
previous b34798d	76,726	72,334
master be1af1d	88,210	84,492
185793e	82,438	78,979

That is +7.4% / +9.2% over b34798d and +3.4% / +3.0% over the merge base. The branch remains about 6.5% below current master, which also has newer changes, including the metrics-collector optimization. These comparisons do not isolate the cause of that remaining difference. All 160 samples across 1, 2, 8 and 16 readers made zero server GETs after warming; the full median table is in the PR description.

Validation on 185793e: 9,237 default-suite tests, 771 focused cases, all linters and 12 read/write stress cases pass. The 24 new regression failures on b34798d pass with this change; custom-connection coverage now includes free-only disconnect too. I resolved the fixed threads. The five upstream workflows still await approval.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 185793ed7b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Grivn]

The idle-owner parser drain needed the same callback protection as pool shutdown. d75d36c now queues maintenance events while the drain owns the socket and invokes application listeners after releasing that lock. Notification state updates and invalidations still run under ownership, and parser errors still propagate. This covers a MIGRATED listener calling pool.close().

All 36 new RESP3 socket-pair regressions fail on 185793e and pass on this revision. The final diff passes 9,255 default tests, 901 focused cases (including async maintenance handling), the configured linters, and 12 stress cases. I updated the description with the repeated four-source throughput comparison, including the peer-hit overhead. Upstream CI is still awaiting maintainer approval.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d75d36c01e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Grivn]

dcb8ceb covers two more maintenance paths. SMIGRATED removed-node cleanup now respects an idle owner being drained, including cleanup during topology refresh. Its listeners and idle AUTH listeners run after their ownership and enclosing locks release, so a completion listener can call pool.close(). Authentication errors and cancellation still propagate.

All 84 new regression cases fail on d75d36c and pass here. The final diff passes 9,297 default tests, 985 focused cases, the configured linters, and 12 read/write stress cases. The description keeps the previous throughput measurements labeled at d75d36c; this change leaves the cache-hit path unchanged. The five upstream workflows still await approval.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: dcb8ceb3e4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Grivn]

e3de6a2 moves cached release listeners outside the pool lock while keeping the connection unavailable until they finish. This removes the release/close/MOVING lock inversion. Successful idle AUTH now clears an older deferred token in both sync and async pools; a newer refresh arriving during release is retained. Error and cancellation behavior is covered.

All 114 new cases pass (86 fail on dcb8ceb), along with 9,357 default tests, 1,223 focused cases, linters and 12 stress cases. I reran the four-source throughput comparison: single-reader ConnectionPool is 78,686 GET/s versus 82,366 on dcb8ceb (-4.5%). The full table and workload limits are in the description. The five upstream workflows still await approval.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e3de6a26a8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Invalidate entries when an idle reconnect is deferred

When NodesManager._free_pool_connections() or SMIGRATED cleanup encounters an idle owner being drained by another node's pool, this branch leaves its entries intact and only marks it for reconnect. The owner was never checked out, so finishing the drain does not trigger release(), and subsequent peer hits do not consult should_reconnect(). Because node pools share the cache, they can keep serving values tracked on the removed source instead of reading the replacement, even after the 100 ms grace period, which only limits busy owners. A socket-pair reproduction returns fresh replacement data on the parent but repeatedly returns the old cached value here. Invalidate those entries immediately or complete the disconnect when the drain releases ownership.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Release the pool lock before idle AUTH socket work

With maintenance notifications enabled, credential refresh can deadlock if AUTH on one idle connection processes MOVING while a peer drains another idle owner. re_auth_callback() retains self._lock across socket I/O, then waits for the shared MaintNotificationsPoolHandler._lock; the peer drain already holds that handler lock and waits for self._lock in handle_node_moving_notification(). Deferring application events does not defer those internal handlers, so both the refresh and cached GET remain blocked and the pool cannot make progress. This reproduces with RESP3 socket pairs on this revision but completes on the parent; perform the claimed idle connection's AUTH outside the pool lock.

Useful? React with 👍 / 👎.