Conversation
|
Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset. In case there are security findings, they will be communicated to you as a comment inside the PR. Hope you’ll enjoy using Jit. Questions? Comments? Want to learn more? Get in touch with us. |
|
@jracabado, thanks for the PR, I’ll take a look when I get a chance. |
|
run sharded pub sub tests |
|
|
@PavelPashov we've had this failure in production, would be great to see a fix merged |
|
@jracabado is it possible to provide a test that validates the correct behavior or minimal reproducible example that demonstrates the expected vs. actual behavior? Also, if I’m understanding correctly, this only happens with standard (p)subscribe in Cluster mode. As a temporary workaround, would setting const cluster = new Cluster(startupNodes, {
redisOptions: { lazyConnect: false },
}); |
This PR ensures the Cluster is refreshed when a Cluster Subscriber connection (.psubscribe, .subscribe) dies.
Previously when a subscriber connection is killed we only selected a new node from the cluster connection pool.
If the client instance is only keeping Cluster Subscriber connections this means the Cluster is never refreshed. The implication is that if the Cluster is changing and new nodes are being added, the connection pool will only contain non-available nodes and the subscription won't be able to be resumed.
This is a state that can be avoided when the client instance also maintains other non-subscriber connections, the error handling on these will try to refresh the cluster. Given there are still live nodes in the connection pool it will be able to refresh the connection pool: https://github.com/redis/ioredis/blob/main/lib/cluster/index.ts#L549