Repository navigation
Conversation
Since #7534 the lock release is a compare-and-delete script instead of GETDEL. Redis writes the replies it has queued in one pass in the reverse order of each client's first queued reply: GETDEL queued its reply before the DEL notified the keyspace subscribers, so they were written first; the script notifies them before its reply is queued, so the releasing client hears back first. An instance with no local waiter for the lock then retakes it at once (the instaque fast path), ahead of the waiters on other instances that were still being notified. In one event loop (two managers in-process) that order is deterministic and a waiter was passed over for a whole 1000-modify loop (~4 s, oplock on and off; 0.10.1: < 0.25 s). Across prod workers it is a race the releaser now wins more often: other-worker requests against a 3000-modify server-side loop waited up to 77-165 ms (0.10.1: 14-24 ms). Send a PING ahead of the release script in the same pipeline, which queues this client's reply first again. It costs no round trip, and the script and its cancellation handling are unchanged; where the write order differs (proxies, io-threads) the behavior is the same as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dxjadq3qkhKnqZR2b9WBFi
[Medium impact] The PR appears safe to merge; no actionable regression was found. SummarySends
No actionable issues found. Tests were not run during this review. Reviews (3) · Last reviewed commit: "fix(state-manager): let a redis lock's w..." · Reviewed by Greptile |
Merging this PR will degrade performance by 8.79%2 warnings
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing |
|
I'm moving this to draft and out of the 0.11.0 fix-up set. CodSpeed's −13% on
I looked at cheaper ways to get the same reply order. None fit a fix-up:
The gain only shows up in one pattern: an instance that loops Leaving this open as a draft in case you'd rather take the trade-off anyway. Generated by Claude Code |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dxjadq3qkhKnqZR2b9WBFi
|
closing, not worth the cost. will fix properly in #7805 |
…last open Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dxjadq3qkhKnqZR2b9WBFi
Found in the reflex 0.11.0a1 pre-release testing (FINDING-030 side note / FINDING-033 investigation; artifacts on branch
claude/vibrant-pasteur-3921g3,prerelease-testing/runs/2026-11-11-v0.11.0a1/redis_runtime/andverify_rds/). Independent of #7795 (reproduces with oplock off); touches the same file in different hunks and merges cleanly with it.All Submissions:
Type of change
Changes To Core Features:
Defect
With the Redis state manager, an instance that modifies a token back to back (e.g. a loop of
app.modify_statecalls) could repeatedly re-take the token's lock ahead of a waiter on another instance. Bisected to #7534 (d36a687): the lock release changed fromGETDELto the compare-and-deleteEVALscript (_RELEASE_LOCK_SCRIPT). Redis flushes queued replies in the order each client's first reply was queued: withGETDELthe keyspace notification to the waiters' subscriber is written before the releaser's reply; withEVAL(and plainDEL) the releaser's reply comes first, so in an event loop the releaser wakes first and its nextSET NXwins before the remote waiter hears of the release. (The instaque fast path itself is unchanged since 0.10.1; 0.10.1 was fair because ofGETDEL's write order.) Measured (bin/order.py, 200 rounds, notification-first): getdel 200/200, eval 0/200, del 0/200, PING pipelined before the EVAL 200/200.Impact: with two managers in one event loop a remote waiter stalled ~3.8-4.4 s across 1000 back-to-back modifies (0.10.1: 0.01-0.04 s), oplock on or off, redis-py 7 or 8. In a real prod app (2 workers, 3000-iteration
modify_stateloop on one worker) the worst latency of other-worker requests went from 14-24 ms (0.10.1) to 77-165 ms (0.11.0a1); medians unchanged; final counts always exact. A mild tail-latency regression under a tight loop, not starvation in practice.Fix
_send_lock_releasesendsPINGand the unchanged releaseEVALin one non-transactional pipeline, restoring 0.10.1's reply order — no extra round trip (uncontendedmodify_state~1.15 ms/op before and after). The cancellation re-send under shield goes through the same helper. Mutual exclusion, fencing, leases and cancellation cleanup are untouched (same script).Test plan
test_remote_waiter_not_passed_over_by_back_to_back_modifies[oplock False/True](real Redis only; skipped on the mock): before the fix 200/200 loop iterations finished before the remote waiter got in (5 runs × both modes); after, 6-7/200 in every run.test_cancel_while_releasing_lock_releases_itnow patches_send_lock_releaseinstead ofredis.eval; the mock pipeline gainedping/eval.tests/units/istate: 269 (mock) / 270 (real Redis) / 270 (real Redis + oplock) passed. Full unit suite: 15634 passed on the mock, 15650 on real Redis, 81.4-81.5% (Playwright browser-driver module excluded locally for lack of a Chromium binary).ruff,pyright: clean.Notes
News fragment:
news/+redis-lock-release-wakes-waiters-first.bugfix.md.Generated by Claude Code