-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Client Side Caching #2854
base: master
Are you sure you want to change the base?
Client Side Caching #2854
Conversation
@@ -218,6 +231,7 @@ export default class RedisClusterSlots< | |||
} | |||
|
|||
await Promise.all(promises); | |||
this.clientSideCache?.enable(); |
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.
need to think about this clear/disable/enable.
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.
timing, et al
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.
so the reason I did disabled/enabled as I did, as while nothing could change, topology (master/replicas, slot mapping) could change without any clients being added.
this concerns me and make me think that on any reconfigure we have to clear cache. I'm not sure the order matters as the only "await" seems to be the await Promises.all() so moving them around wont effect much I think.
63601a1
to
ff9fca2
Compare
Description
Checklist
npm test
pass with this change (including linting)?