Skip to content

compat(redis): make TIME-using Lua scripts work on Redis 3.2-4.x#2970

Open
ghostg00 wants to merge 1 commit into
Wei-Shaw:mainfrom
ghostg00:compat/redis-replicate-commands
Open

compat(redis): make TIME-using Lua scripts work on Redis 3.2-4.x#2970
ghostg00 wants to merge 1 commit into
Wei-Shaw:mainfrom
ghostg00:compat/redis-replicate-commands

Conversation

@ghostg00
Copy link
Copy Markdown

@ghostg00 ghostg00 commented Jun 2, 2026

Summary

Eight Lua scripts in backend/internal/repository/ call redis.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).

TIME is non-deterministic, and Redis replicates that to replicas / AOF differently across versions. This PR adds a single redis.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 version Behavior of redis.replicate_commands() Effect of this PR
3.2 – 4.x Required — switches Lua from script replication to effects replication so non-deterministic command effects can be propagated Fixes incorrect replication / silent divergence on replicas and AOF
5.0 – 6.x No-op — effects replication is the default No-op — behavior unchanged
7.0+ Still callable, marked deprecated (not removed); the docs still list it as the supported way to opt into effects replication when needed No-op — behavior unchanged

The bundled deploy/docker-compose*.yml files pin redis: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 ARGV timestamp 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)

File Scripts
backend/internal/repository/concurrency_cache.go acquireScript, getCountScript, cleanupExpiredSlotsScript
backend/internal/repository/session_limit_cache.go registerSessionScript, refreshSessionScript, getActiveSessionCountScript, isSessionActiveScript
backend/internal/repository/user_msg_queue_cache.go releaseLockScript

Scripts that do not invoke non-deterministic commands (incrementWaitScript, decrementWaitScript, acquireLockScript, forceReleaseLockScript, startupCleanupScript, …) are intentionally left untouched.

Test plan

  • go build ./... passes
  • Existing repository tests pass against redis:8-alpine (no behavior change, since redis.replicate_commands() is a no-op there)
  • Manual smoke test against a Redis 4.x primary + replica setup, verifying the affected scripts execute and replicate cleanly

Diff size

3 files changed, 24 insertions(+) — two comment lines + one call per script.

…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.
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 2, 2026

All contributors have signed the CLA. ✅
Posted by the CLA Assistant Lite bot.

@ghostg00
Copy link
Copy Markdown
Author

ghostg00 commented Jun 2, 2026

I have read the CLA Document and I hereby sign the CLA

github-actions Bot added a commit that referenced this pull request Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant