node-redis
node-redis copied to clipboard
Fix race condition when slots are re-calculated
Description
I was getting an error similar to https://github.com/redis/node-redis/issues/2704.
What was happening for me is that I was creating a redis cluster client and then calling client.eval()
. Since eval
doesn't seem to be cluster-aware, it returned a MOVE
error since it was routed to an incorrect node. This seems to cause a call to discover()
to re-discover the hash slot mapping.
The bug I am trying to address is a race condition between when the internal data is cleared to when it's re-populated again. In the current implementation, there's an asynchronous command being run in-between, which means if any other command was run in between, would cause this.slots[val]
of cluster-slots.ts
(such as in getSlotRandomNode()
) to return undefined and throw an exception.
async #discover(rootNode?: RedisClusterClientOptions) {
this.#resetSlots(); // !!! Reset is being called here
const addressesInUse = new Set<string>();
try {
const shards = await this.#getShards(rootNode), // !!! But there's an async command here
promises: Array<Promise<unknown>> = [],
eagerConnect = this.#options.minimizeConnections !== true;
for (const { from, to, master, replicas } of shards) {
This changes moves when it is cleared so that it's cleared then populated, without any async stuff inbetween.
Checklist
- [x] Does
npm test
pass with this change (including linting)? - [x] Is the new or changed code fully tested?
- [ ] Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?
Hi, can this be taken a look at? I can't seem to add people as reviewers. Thank you.
@jjsimps sorry for the delay. I'll take a look at the start of next week. If i forget - ping me.. ;)