compat(redis): make TIME-using Lua scripts work on Redis 3.2-4.x#2970
Open
ghostg00 wants to merge 1 commit into
Open
compat(redis): make TIME-using Lua scripts work on Redis 3.2-4.x#2970ghostg00 wants to merge 1 commit into
ghostg00 wants to merge 1 commit into
Conversation
…3.2-4.x
Call redis.replicate_commands() at the top of every Lua script that
reads redis.call('TIME'), so the resulting writes (ZADD, ZREMRANGEBYSCORE,
SET, DEL, EXPIRE) replicate correctly on Redis 3.2-4.x.
This is a no-op on Redis 5.0+, where effects replication is the default.
The function remains a public API on Redis 7.0+ (deprecated but supported)
and is still the documented way to opt into effects replication on older
versions, so calling it unconditionally is safe and forward-compatible.
Affected scripts (8 in total, all already using redis.call('TIME') by
design to avoid client-side clock skew across multiple app instances):
- concurrency_cache.go: acquireScript, getCountScript,
cleanupExpiredSlotsScript
- session_limit_cache.go: registerSessionScript, refreshSessionScript,
getActiveSessionCountScript, isSessionActiveScript
- user_msg_queue_cache.go: releaseLockScript
Scripts that do not invoke non-deterministic commands are intentionally
left untouched.
Contributor
|
All contributors have signed the CLA. ✅ |
Author
|
I have read the CLA Document and I hereby sign the CLA |
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
Eight Lua scripts in
backend/internal/repository/callredis.call('TIME')to read the Redis server clock — by design, to avoid client-side clock skew across multiple application instances (the existing inline comments document this rationale).TIMEis non-deterministic, and Redis replicates that to replicas / AOF differently across versions. This PR adds a singleredis.replicate_commands()call at the top of each affected script so the resulting writes (ZADD,ZREMRANGEBYSCORE,SET,DEL,EXPIRE) replicate correctly on Redis 3.2 – 4.x.Compatibility
redis.replicate_commands()The bundled
deploy/docker-compose*.ymlfiles pinredis:8-alpine, so this never surfaces on the default stack. It only surfaces in deployments running against older Redis instances (private/enterprise registries, on-prem setups), which is where this fix is needed.Why not pass the timestamp in via
ARGV?Each affected script's inline comment explicitly documents the design intent: read the time from the Redis server so the timestamp source stays consistent across multiple application instances. Switching to a client-supplied
ARGVtimestamp would regress that design and reintroduce the clock-skew problem this code was written to avoid. Opting into explicit command replication is the correct fix.Affected scripts (8 total)
backend/internal/repository/concurrency_cache.goacquireScript,getCountScript,cleanupExpiredSlotsScriptbackend/internal/repository/session_limit_cache.goregisterSessionScript,refreshSessionScript,getActiveSessionCountScript,isSessionActiveScriptbackend/internal/repository/user_msg_queue_cache.goreleaseLockScriptScripts that do not invoke non-deterministic commands (
incrementWaitScript,decrementWaitScript,acquireLockScript,forceReleaseLockScript,startupCleanupScript, …) are intentionally left untouched.Test plan
go build ./...passesredis:8-alpine(no behavior change, sinceredis.replicate_commands()is a no-op there)Diff size
3 files changed, 24 insertions(+) — two comment lines + one call per script.