Skip to content

Conversation

htemelski-redis
Copy link
Contributor

When issuing a ssubscribe command, the Cluster client creates a "regular" Redis client to establish a connection to the correct shard. This doesn't work if there's a failover since the client doesn't know how to correctly handle a MOVED response. This PR handles such events by propagating the ssubscribe MOVED error from the DataHandler, through the ClusterSubscriber and the ClusterSubscriberGroup to the RedisCluster client and forces slots refresh.

Direct port from #1987

Copy link

@nkaradzhov nkaradzhov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CLGTM

@htemelski-redis htemelski-redis changed the title Force slots refresh on MOVED error when using ssubscribe fix(ssubscribe): Force slots refresh on MOVED error when using ssubscribe Jun 16, 2025
@htemelski-redis htemelski-redis force-pushed the ssubscribe_moved_latest branch from d6d5099 to 0766449 Compare June 17, 2025 08:14
@elena-kolevska elena-kolevska changed the title fix(ssubscribe): Force slots refresh on MOVED error when using ssubscribe fix(ssubscribe): [cherrypick from v4] Force slots refresh on MOVED error when using ssubscribe Jun 17, 2025
@coveralls
Copy link

Pull Request Test Coverage Report for Build 17939972268

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.05%) to 90.681%

Files with Coverage Reduction New Missed Lines %
lib/connectors/StandaloneConnector.ts 2 94.64%
Totals Coverage Status
Change from base Build 17939975139: -0.05%
Covered Lines: 2330
Relevant Lines: 2502

💛 - Coveralls

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

Successfully merging this pull request may close these issues.

3 participants