🏙️ feat: Add Redis Cluster Support - #17
Conversation
|
|
0aabfff to
418e509
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 418e509eb3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@danny-avila all issues should be fixed Added : Merged from upstream to fix conflicts |
0deb596 to
045d5a2
Compare
… handling * Refactor job processing in workers.ts for improved readability and maintainability. * Introduce Redis connection management in redis-connection.ts. * Add tests for Redis connection utilities in redis-connection.test.ts. * Implement TLS options handling for secure Redis connections. * Enhance error handling and logging throughout the job processing flow.
* Updated the project dependency to version 2.3.1. * Ensured compatibility with existing codebase. * Ran tests to verify functionality post-upgrade.
- Hash-tag per-execution Redis keys ({execution_id}) in replay-state and
tool-call-server so multi-key Lua scripts, MULTI/EXEC and multi-key DELs
stay on one Cluster slot (avoids CROSSSLOT errors)
- Add shared hashTag/stripHashTag/scanKeys helpers in redis-connection
- Replace blocking KEYS with cluster-aware SCAN in tool-call-server cleanup
- Fix wait-for-redis probe to target a single startup node in cluster mode
via new codeapi.redis.probeHost/probePort helpers
- Gate REDIS_TLS/CA env and volumes behind external Redis (not bundled subchart)
- Document redis.enabled=false requirement for cluster mode in README
045d5a2 to
7477efe
Compare
|
Rebased yet again :) |
|
@codex review the latest head |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7477efe4e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Resolve conflicts in config.ts, queue.ts, workers.ts, replay-state.ts, values.yaml and api-deployment.yaml.
main's bridge stores took a concrete Redis; the cluster branch's shared connection is Redis | Cluster. Widen both constructors so the merged tree typechecks.
|
Hi @pedrojreis — this had drifted into conflict against git remote add upstream https://github.com/LibreChat-AI/code-interpreter.git
git fetch upstream pr-17-redis-cluster-merged
git reset --hard upstream/pr-17-redis-cluster-merged
git push --force-with-leaseWhat conflicted, and why there was so much of it14 hunks across 6 files. Only 8 of the 19 files you touched had actually moved on Worth deciding before this merges: left as-is, the next commit to How I resolved each
That last one is the only judgment call worth flagging. I also widened Still blocking, and yours to resolveThe branch does not typecheck — but it didn't before I touched it either, and my merge added zero new errors (verified against your branch's own baseline). 10 errors remain:
Everything else checks out: 701/703 tests pass, and both failures are pre-existing Thanks for this — the |
danny-avila
changed the title
e1fd741 to
a7c1ea3
Compare
|
@danny-avila did what you said and fixed issues previously detected :) |
|
To use Codex here, create a Codex account and connect to github. |
3 participants
Overview
Adds opt-in Redis Cluster support to every service component. Standalone
Redis remains the default — existing deployments require zero configuration
changes and behave exactly as before.
Validated in production against Google Cloud Memorystore in cluster mode with
TLS and CA-certificate verification.
Motivation
The service previously constructed Redis connections with inline
new IORedis({ ... })calls in four separate modules, each hardcoded tostandalone mode. Connecting to a clustered Redis (GCP Memorystore cluster, AWS
ElastiCache cluster) was impossible: the client would only ever reach a single
shard and fail with
MOVED/CROSSSLOTerrors under load.This PR centralizes connection creation behind a single factory and teaches
every component to speak the Redis Cluster protocol when asked.
What's new
🔌 Cluster mode (opt-in, auto-detected)
Enable it either explicitly or implicitly:
🔐 TLS with CA-certificate validation
When
REDIS_CAis set it takes precedence and enables validated TLS.REDIS_TLS=trueon its own keeps the previousrejectUnauthorized: falsebehaviour for backward compatibility.
🧩 BullMQ cluster-safety
Queue, Worker and QueueEvents receive a
{codeapi}hash-tag prefix in clustermode so all BullMQ keys map to a single hash slot (a hard requirement for BullMQ
on Redis Cluster). Standalone deployments keep their existing key layout — no
migration needed.
New environment variables
USE_REDIS_CLUSTERfalseREDIS_HOSTcontains a comma.REDIS_CAREDIS_TLS.Existing variables are unchanged and fully backward-compatible:
REDIS_HOST,REDIS_PORT,REDIS_PASSWORD,REDIS_TLS,REDIS_USE_ALTERNATIVE_DNS_LOOKUP,REDIS_KEEP_ALIVE_MS.Implementation
service/src/redis-connection.ts(new — single source of truth)createRedisConnection(overrides)Redis | Clusterbased on env; each caller passes its own retry / readyCheck overridesisClusterMode()USE_REDIS_CLUSTER=trueor comma inREDIS_HOSTparseRedisNodes()REDIS_HOSTinto[{ host, port }]startup nodesbuildTlsOptions()REDIS_CA→{ ca }(validated); elseREDIS_TLS=true→{ rejectUnauthorized: false }; else no TLSbullmqPrefix()'{codeapi}'in cluster mode,undefinedotherwiseRefactored clients
All four inline
new IORedis({ ... })blocks now callcreateRedisConnection():queue.ts— shared BullMQ connection +prefix: bullmqPrefix()onQueue/QueueEventsworkers.ts—prefix: bullmqPrefix()on bothWorkerinstancesegress-ledger.ts— mutation-connection pool made cluster-safe (Clusterhas no.duplicate(), so a freshcreateRedisConnection()is used instead)tool-call-server.ts,file-server.ts— session-state clientsservice/src/service/replay-state.tsscanKeys()is now cluster-aware.ioredis.Clusterhas no top-levelscanStream, so in cluster mode the helper fans out across every master nodevia
cluster.nodes('master')and streamsSCANon each. Masters own disjointhash-slot ranges, so results never overlap. This fixes the runtime crash:
service/src/config.tsAdds the
USE_REDIS_CLUSTERflag to the parsed env.Helm chart (
helm/codeapi/)New
values.yamlsurface:New
_helpers.tpltemplates —codeapi.redis.clusterEnabled,codeapi.redis.tlsEnv,codeapi.redis.caVolume,codeapi.redis.caVolumeMount— are wired into all five component Deployments, including mounting the CA cert
from a Secret into each pod.
service/.env.exampleDocuments every new variable with inline guidance.
Tests
New
service/src/redis-connection.test.ts— 18 unit tests, no live Redis required:parseRedisNodesisClusterModebuildTlsOptionsREDIS_TLSonly,REDIS_CAfile read, CA precedence overREDIS_TLS, missing CA filebullmqPrefixBackward compatibility
REDIS_TLS=truewithoutREDIS_CAkeeps the priorrejectUnauthorized: falsebehaviour.How to verify