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

Redis: Implement double write client #3900

Closed
Dav1dde opened this issue Aug 6, 2024 · 2 comments · Fixed by #4064
Closed

Redis: Implement double write client #3900

Dav1dde opened this issue Aug 6, 2024 · 2 comments · Fixed by #4064
Assignees

Comments

@Dav1dde
Copy link
Member

Dav1dde commented Aug 6, 2024

For lossless Redis migration we will need to make it possible for Relay to double write quotas into two separate clusters. It should be possible to configure a 'double write' Redis client, with a primary and secondary (or multiple secondaries), the client only reads from the primary but writes to all configured Redis instances.

Sketeched implementation:

Config:

redis:
    a_good_namespace_for_multi_write:
    - url: redis://redis1 # primary
    - url: redis://redis2

# for the multi config
redis:
    project_configs:
        a_good_namespace_for_multi_write:
        - url: redis://redis1 # primary
        - url: redis://redis2
    misc: # No multi write
       url: redis://redis1 

Implementation:

There is already a Client Wrapper in relay-redis, implement a MultiWriteOrBetterNamedRedisClient which dispatches writes to all of it's clients but only reads from the primary. Writes should happen in concurrently (joined).

Error Behaviour:

Log errors of secondary nodes but not fail and only use the primary response.

@Dav1dde
Copy link
Member Author

Dav1dde commented Sep 18, 2024

Planning this for early next week (~23.9.), ideally we'd like to separate the Redis cluster in the week after (~30.9.).

@mwarkentin
Copy link
Member

Following this issue, rc-ingest in saas is currently on a quite old version of redis which doesn't support ACLs / read-only users (that I can see).

Apparently this is a blocker for migration to memorystore / newer redis version. This will be helpful for https://getsentry.atlassian.net/browse/SRE-307.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants