-
Notifications
You must be signed in to change notification settings - Fork 12
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
PSERV-1561 - Reconnect when readonly error is received from redis in Limitr client #37
Conversation
Tests run locally are passing ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving with a comment (fine if you apply it, fine if you don't) but make sure that merged/squashed commit:
- mentions jira issue in the subject of the commit
- mentions updating ioredis version somewhere
the docs uses `includes` rather than `indexOf` Co-authored-by: Carlos Campderrós <carlos@campderros.com>
@@ -15,7 +15,7 @@ | |||
"dependencies": { | |||
"async": "^2.6.1", | |||
"disyuntor": "^3.5.0", | |||
"ioredis": "^4.5.1", | |||
"ioredis": "^4.28.5", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a huge change. What's the changelog is saying here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly bug fixes!
Most feature changes are small, I do not see any red or salmon colored flags, PTAL though and lmk if anything stands out to you.
Description
Adding a
reconnectOnError
handler for ioredis client that will reconnect if an error is emitted from the client that includesREADONLY
in the message.This is important for environments where auto failover is not enabled to minimize incidents in limitr use cases when a failover does occur with an elasticache cluster.
This mimics the behavior used in
belgrano
.References
https://auth0team.atlassian.net/browse/PSERV-1561
https://github.com/auth0/belgrano/blob/62589d1e1d31ed5e91ea92a68c8a9b4da5fffc23/lib/cache/redis.js#L58-L63
https://github.com/luin/ioredis#reconnect-on-error
Testing
Checklist