Skip to content

Commit

Permalink
Fix incorrect HashSlot calculation for XREAD and XREADGROUP commands (#…
Browse files Browse the repository at this point in the history
…2093)

I have created a fix for the incorrect HashSlot calculation for the XREAD and XREADGROUP commands (#2086) Since these differ a lot from other messages I have created some new CommandMessage classes:
- SingleStreamReadGroupCommandMessage 
- MultiStreamReadGroupCommandMessage
- SingleStreamReadCommandMessage 
- MultiStreamReadCommandMessage 

The single CommandMessage classes could be removed by only using the multi command messages, but then we have to create a new StreamPostion array and a new StreamPostion object. I wasn't sure which option too choose, so feel free to give input on this (GC vs duplicate code).

There is also some code duplication between the Read and ReadGroup Command messages as the ReadGroup command is very similar to the XREAD but has some additional values in the front of the command (group, consumer, noack). A base class could be created as well to reduce some of the duplication (calculating the hashslot in case of multiple streams and writing the streams part of the commands).

The method CommandAndKey is also not overriden for these Messages as I wasn't which argument of the command to include in the string.

Co-authored-by: Niels Derdaele <niels.derdaele@alsic.be>
Co-authored-by: Nick Craver <nrcraver@gmail.com>
  • Loading branch information
3 people authored Jun 14, 2022
1 parent 67297e3 commit 8c9cb1b
Show file tree
Hide file tree
Showing 2 changed files with 206 additions and 118 deletions.
1 change: 1 addition & 0 deletions docs/ReleaseNotes.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
- Adds: Support for `ZMPOP` with `.SortedSetPop()`/`.SortedSetPopAsync()` ([#2094 by slorello89](https://github.com/StackExchange/StackExchange.Redis/pull/2094))
- Adds: Support for `XAUTOCLAIM` with `.StreamAutoClaim()`/.`StreamAutoClaimAsync()` and `.StreamAutoClaimIdsOnly()`/.`StreamAutoClaimIdsOnlyAsync()` ([#2095 by ttingen](https://github.com/StackExchange/StackExchange.Redis/pull/2095))
- Fix [#2071](https://github.com/StackExchange/StackExchange.Redis/issues/2071): Add `.StringSet()`/`.StringSetAsync()` overloads for source compat broken for 1 case in 2.5.61 ([#2098 by NickCraver](https://github.com/StackExchange/StackExchange.Redis/pull/2098))
- Fix [#2086](https://github.com/StackExchange/StackExchange.Redis/issues/2086): Correct HashSlot calculations for `XREAD` and `XREADGROUP` commands ([#2093 by nielsderdaele](https://github.com/StackExchange/StackExchange.Redis/pull/2093))
- Adds: Support for `LCS` with `.StringLongestCommonSubsequence()`/`.StringLongestCommonSubsequence()`, `.StringLongestCommonSubsequenceLength()`/`.StringLongestCommonSubsequenceLengthAsync()`, and `.StringLongestCommonSubsequenceWithMatches()`/`.StringLongestCommonSubsequenceWithMatchesAsync()` ([#2104 by Avital-Fine](https://github.com/StackExchange/StackExchange.Redis/pull/2104))
- Adds: Support for `OBJECT FREQ` with `.KeyFrequency()`/`.KeyFrequencyAsync()` ([#2105 by Avital-Fine](https://github.com/StackExchange/StackExchange.Redis/pull/2105))
- Performance: Avoids allocations when computing cluster hash slots or testing key equality ([#2110 by Marc Gravell](https://github.com/StackExchange/StackExchange.Redis/pull/2110))
Expand Down
Loading

0 comments on commit 8c9cb1b

Please sign in to comment.