Fixed CROSSSLOT ERROR in Cluster Mode for PUBSUB SHARDNUMSUB #1369
Conversation
Signed-off-by: Nikhil Manglore <[email protected]>
src/commands/pubsub-shardnumsub.json
Outdated
| "key_specs": [ | ||
| { | ||
| "flags": [ | ||
| "NOT_KEY" | ||
| ], | ||
| "begin_search": { | ||
| "index": { | ||
| "pos": 1 | ||
| } | ||
| }, | ||
| "find_keys": { | ||
| "range": { | ||
| "lastkey": -1, | ||
| "step": 1, | ||
| "limit": 0 | ||
| } | ||
| } | ||
| } | ||
| ], |
There was a problem hiding this comment.
Also, with this change, doesn't it update commands.def ?
There was a problem hiding this comment.
Yes it does, should I push those changes as well?
There was a problem hiding this comment.
It needs to be committed to make the CI pass.
There was a problem hiding this comment.
I have pushed the commands.def file. I'll push the test cases once I have finished running the tests.
|
It's undeniably a breaking change, since we are throwing errors when something used to work. It should go with 9.0. |
zuiderkwast
added
the
breaking-change
Signed-off-by: Nikhil Manglore <[email protected]>
|
@Nikhil-Manglore Since this is a breaking change, we will release it only in the next major version, which is 9.0. This means that we can't merge it until after we've branched off for the 8.1 release. We'll do that around January or February. Sorry for the inconvenience. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## unstable #1369 +/- ##
============================================
+ Coverage 70.76% 70.79% +0.03%
============================================
Files 118 118
Lines 63405 63497 +92
============================================
+ Hits 44866 44955 +89
- Misses 18539 18542 +3
|
Signed-off-by: Nikhil Manglore <[email protected]>
|
When writing the test cases in tcl I realized there was an issue with the keyspecs in my original code. I fixed that issue by changing the starting position and then tested it with the testcases. This gave the proper results and i have pushed the final changes including the tcl testcases. |
|
@Nikhil-Manglore Thanks for the PR. As Viktor mentioned it's a breaking change, so we will merge this in for the major version (need to wait it out). |
Resolved #560
Added key specification to the pubsub-shardnumsub.json file to force the CROSSSLOT error when a user subscribes to two channels in different hash slots.