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

ref(redis): Use a readonly client for project configs [INGEST-1557] #1393

Closed
wants to merge 2 commits into from

Conversation

jan-auer
Copy link
Member

@jan-auer jan-auer commented Aug 8, 2022

We use a single Redis client for both rate limiting and fetching project configs. Since the former needs to modify keys, the client cannot be set to readonly mode. Project config fetching only reads keys from redis, so setting the cluster client to readonly mode allows it to read from replicas, too.

Note - This PR is not yet merge ready:

  • There should be a single Redis client in non-cluster mode
  • Redis pool initialization should be moved and deduplicated
  • Changes from master need to be merged in
  • Cluster connections aren't configurable for single mode.

Based off of 1d91c35, which is currently deployed.

See INC-192

@jan-auer jan-auer force-pushed the release/redis-readonly branch from 3cb6332 to 4b4f705 Compare August 8, 2022 18:43
@jan-auer
Copy link
Member Author

jan-auer commented Aug 8, 2022

This is much less urgent after #1394. Both of the changes in this PR may still be useful, although there's open questions on how to expose the config and whether it's desirable to have separate connection pools.

@jan-auer jan-auer changed the title ref(redis): Use a readonly client for project configs ref(redis): Use a readonly client for project configs [INGEST-1557] Aug 9, 2022
@jan-auer jan-auer closed this Aug 23, 2022
@jan-auer jan-auer deleted the release/redis-readonly branch August 23, 2022 15:06
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.

2 participants