Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance Redis cluster support #5784

Closed
wants to merge 9 commits into from

Conversation

gafda
Copy link
Contributor

@gafda gafda commented Feb 10, 2025

Summary

This adds support for connecting to Redis clusters with TLS authentication, specifically enabling LibreChat to work with Google Memorystore Redis Cluster.
The changes are focused on the api/cache/keyvRedis.js file.

Change Type

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Config Changes

There are two new environment variables:

Key Type Description Example
USE_REDIS_CLUSTER boolean Instructs LibreChat to use the Cluster mode to connect to Redis true
REDIS_CA string If set, instruct LibreChat to use TLS with server authentication, using the file set in this variable as a PEM-encoded certificate authority. /memorystore/ca
REDIS_KEY_PREFIX string Default is empty string. If set, all keys will automatically be set with a prefix. It is useful, for example, when more than one different instance of LibreChat with REDIS connection is present in the same environment (ie.: Staging, Stable, ...) librechat-staging:
REDIS_MAX_LISTENERS number Maximum number of event listeners allowed for the Redis client instance. It helps prevent memory leaks by limiting event listeners. If set to 0 (zero) it will be considered limitless. Defaults to 10 if not specified. 10

Testing

Regex testing:
image

Started a node pod in the cluster via kubectl run --rm -it node-18 --image node:18-bullseye (should have used node-20 but that was what the developer-defined in .devcontainer/Dockerfile so I went with it.

Then attached to that running pod via vscode: https://code.visualstudio.com/docs/devcontainers/attach-container

Checked out the code, defined my .env file (using the deployment env vars where required, like MongoDB or pointing to the RAG API), and used my very own vscode debug file to check my work.

I was able to validate that the object is correctly recognized as a Cluster after creation:
image

We see the TLS definition there:
image

After letting the program run for a while and breaking, for instance, in here:
image

I can now see that the Redis client is in the read status.
image

Test Configuration:

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • My changes do not introduce new warnings

@gafda gafda marked this pull request as ready for review February 10, 2025 17:00
@gafda gafda force-pushed the add-redis-cluster-support branch 2 times, most recently from e12f24e to f64f2d6 Compare February 11, 2025 09:21
Copy link

@github-advanced-security github-advanced-security bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ESLint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

@gafda gafda force-pushed the add-redis-cluster-support branch from f64f2d6 to 069c08d Compare February 14, 2025 10:32
Comment on lines 62 to 69
port: value.port
};

Check failure

Code scanning / ESLint

Require or disallow trailing commas

Missing trailing comma.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@@ -495,16 +495,23 @@ HELP_AND_FAQ_URL=https://librechat.ai
# Google tag manager id
#ANALYTICS_GTM_ID=user provided google tag manager id

#===============#
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two new environment variables:

Please also update the librechat docs: https://github.com/LibreChat-AI/librechat.ai/blob/main/pages/docs/configuration/dotenv.mdx

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here it is!

Please let us know if you need any changes. :)

keyvRedis.on('error', (err) => logger.error('KeyvRedis connection error:', err));
keyvRedis.setMaxListeners(20);
keyvRedis.setMaxListeners(0);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is max listeners set to 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was needed for our use case.
Since it differs from the original, I've made it customizable with REDIS_MAX_LISTENERS instead. (Description updated.)

port: port || null,
};
} else {
// Handle cases without a scheme
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please address linting

@gafda gafda force-pushed the add-redis-cluster-support branch from 4cc1293 to 1e51084 Compare February 19, 2025 10:29

let keyvRedis;
const redis_prefix = REDIS_KEY_PREFIX || "";

Check failure

Code scanning / ESLint

Enforce the consistent use of either backticks, double, or single quotes

Strings must use singlequote.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@danny-avila
Copy link
Owner

Thanks, merging here: #5963

@pedrojreis pedrojreis deleted the add-redis-cluster-support branch February 28, 2025 13:35
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.

4 participants