feat: add Redis lock backend - #101
Open
rustatian wants to merge 24 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
There are confirmed correctness issues in the new concurrent writer test and Redis wait-loop timer handling that can cause nondeterministic behavior and busy looping, plus an empty-key cleanup issue in the Lua script that can lead to unbounded key buildup.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds a Redis-backed implementation for the lock plugin so multiple RoadRunner instances can share exclusive/read locks via the existing RPC API, while keeping in-memory locks as the default when no lock config section is provided.
Changes:
- Introduces a Redis backend using
go-redis/v9plus an embedded Lua script for atomic lock state transitions and TTL handling. - Implements wait/notification behavior using Redis Pub/Sub and client-side timers.
- Expands documentation and adds Redis-focused integration tests; updates CI to run the full
./tests/...module with a Redis service.
File summaries
| File | Description |
|---|---|
redis.go |
Implements Redis backend operations, waiting via Pub/Sub + timers. |
redis.lua |
Atomic Lua script for lock/read/exists/release/ttl/force operations. |
config.go |
Adds lock/Redis configuration structs with mapstructure tags. |
plugin.go |
Selects memory vs Redis backend based on config; wires backend into plugin lifecycle. |
memory.go |
Wraps existing in-memory locker behind the new backend interface (preserves default behavior). |
rpc.go |
Routes RPC methods through the backend interface and propagates backend errors. |
tests/redis_test.go |
Adds Redis integration tests covering exclusivity, readers, waits, expiry, and stop behavior. |
tests/go.mod / tests/go.sum |
Adds go-redis/v9 and required indirect deps for the test module. |
go.mod / go.sum |
Adds go-redis/v9 and required indirect deps for the plugin module. |
.github/workflows/linux.yml |
Adds Redis service and runs go test ./... for the tests module. |
README.md |
Documents backend selection, Redis config, behavior semantics, and how to run tests locally. |
Review details
- Files reviewed: 11/13 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #101 +/- ##
==========================================
+ Coverage 78.82% 87.48% +8.66%
==========================================
Files 4 9 +5
Lines 595 1007 +412
==========================================
+ Hits 469 881 +412
Misses 126 126 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Signed-off-by: Valery Piashchynski <piashchynski.valery@gmail.com>
The command now targets ./..., so a Redis failure stopped the run before the memory-backend suite started.
The deadline was absolute and per connection, so it capped the total RPC span of a test at 5 seconds. The go test timeout bounds the run.
go mod tidy -diff now exits 0 in the root module and in tests.
The memory locker keeps a resource after its last lock ends, so ForceRelease reported a removal for a resource with no live lock. The Redis backend returns the DEL result and already follows the rule.
Determine the ForceRelease result while broadcasting to registered lock entries. Cleanup can zero counters after a reader registers, so counters do not prove that the resource is empty. Keep the existing nonblocking broadcasts and cover the reachable state with a deterministic regression test.
A repeated LockRead by the same ID granted a second reader on memory and refreshed the member on Redis. Both backends now refuse it, so one Release frees the resource. A waiting caller no longer retries at its own expiry.
Use a distinct Lua delay sentinel to stop duplicate read acquisitions before subscribing or retrying. Add an RPC regression proving that a notification after expiry cannot recreate the refused reader.
Reject a non-zero db with more than one address, because the cluster client uses database 0. Reject a negative dial_timeout, because every dial then fails at once. Reject negative read and write timeouts, because they remove the deadline from each command. Select the in-memory backend with driver: memory.
The Redis lock configuration now accepts master_name, sentinel_password, pool_size and a tls block with cert, key and root_ca. Validation rejects a negative pool_size and a tls block with only one of cert and key.
Every RPC method checks the ttl and wait values it forwards against the time.Duration limit. A negative or overflowing value returns an error and changes no lock state.
A deadline that fires during a Redis command returned Ok: false with no error. Redis can grant the lock after the caller stops waiting, which leaves a lock nobody releases. The command error now reaches the caller. Wait expiry with no command running still returns Ok: false.
Each waiting acquisition opened its own Pub/Sub connection, a receive goroutine and a health check. The backend keeps one connection, one dispatcher goroutine and a registry of the waiters of each channel.
Receive subscription replies directly so server-side failures reach waiting RPCs. Close failed subscriptions before replacement, preserve reconnect wakeups and idle health checks, and join the receiver during shutdown.
Send health pings independently of the receive loop and use bounded Receive calls that discard a failed stream. Keep partial notifications intact across health ticks and join the health worker with the dispatcher.
The wait field has one meaning for Lock and LockRead and another meaning for Release, ForceRelease, Exists and UpdateTTL. The two backends also apply a zero wait differently. Add a README section and a Redis regression test for a wait deadline during a Redis command.
Prevent ambiguous script replay through the cluster NoRetry hook while preserving MOVED, ASK, and NOSCRIPT handling. Queue Pub/Sub I/O outside the waiter registry, preserve confirmation generations, and close backend-owned sockets on deadline-aware shutdown. Reject negative databases before dialing, run root race regressions in CI, and correct the configuration documentation.
Reissue the pending idempotent UNSUBSCRIBE on receive, resubscription, or health reconnect signals while retaining its acknowledgment barrier. Cover lost unsubscribe replies through real RPC and Redis, including unrelated surviving waiters, same-resource replacements, new resources, and last-channel recovery.
Notify the pending unsubscribe through the backend-owned dial hook whenever a physical connection is established. Preserve the matching acknowledgment barrier and fixed operation deadline. Exercise successful MOVING/PONG handoff with an unread last-channel unsubscribe acknowledgment through real RPC and Redis, covering new and replacement waiters.
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.
go-redis/v9, the client used by the RoadRunner Redis plugin.closes: roadrunner-server/roadrunner#2070.