Add shared RedisLock support - #125
Merged
Merged
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
…ock interpreter exit Failing shared-lock tests on CI left unreleased locks whose non-daemon pubsub worker kept pytest alive until the 30 minute job timeout. Losing the connection releases a redis lock by design, so process exit must not be blocked by a held lock. Also normalizes formatting that 'ruff format --check' flagged on CI.
…r paths deterministically The pending-writer and election tests raced heavily loaded CI runners: Windows exhausted a 5s acquire timeout and macOS missed the 1s liveness window during election, electing the wrong writer. Timeouts are now sized for worst-case runners while the assertions still never wait for them on the happy path. The fail-fast regression now proves independence from the timeout with a 30s budget instead of racing a 0.5s wall clock. New deterministic elected-writer tests cover the shared-holder wait, unavailable-holder wait and subscription reuse paths that timing previously left uncovered on macOS.
…nistically Both paths race the subscribers==1 fast path in the integration tests, so their coverage depended on scheduler timing and failed the 100% gate on quiet runners.
RedisLock.pubsub is assigned before SUBSCRIBE reaches the server, so waiting for it proved nothing about election membership. On stalled runners the second writer could subscribe, observe a complete holder set without the first writer and legitimately elect itself. Both contention tests now wait for the actual subscriber count, making the election outcome deterministic.
…ners Windows CI needs multiple seconds per fakeredis pubsub round trip, so the contention tests can legitimately exceed the global 20s pytest timeout while behaving correctly. The generous acquire budgets from the previous commit only work if the surrounding test is allowed to wait for them.
An unelected pending writer backs off by dropping its subscription whenever its holder sample is incomplete, so it is legitimately invisible to new readers until its first complete sample elects it. On slow runners that window outlived the previous synchronization, letting a late shared reader acquire and, in the election test, the wrong writer win. Both tests now wait for the election state that the reader-gating and ordering guarantees are actually defined by.
wolph
marked this pull request as ready for review
July 25, 2026 14:51
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
LockFlags.SHAREDtox -e redis-liveenvironmentWhy
RedisLockpreviously treated every subscriber as an exclusive holder. That made it unsuitable as an NFS-safe replacement for workflows such as parallel compilation that require multiple concurrent readers.The heartbeat protocol now communicates holder identity and mode. Legacy timestamp responses remain exclusive, so rolling upgrades fail safely rather than allowing incompatible overlap.
User impact
Existing callers retain exclusive behavior by default. Callers can request concurrent readers with:
Waiting writers gate later readers, avoiding writer starvation.
Validation
uv run pytest -q— passed with 100% coverageREDIS_PORT=6399 uv run tox -e redis-live— 74 passed, 1 expected fakeredis-only skip; no live-server skipsuv run mypy --show-error-codes --no-color-outputuv run basedpyrightuv run pyrefly check --output-format min-text --summary=none --color neveruv run ty checkuv run tox -e codespelluv run tox -e docsCloses #124