Execute TimeSeries commands not including keys in their syntax on the default node of a Redis Cluster #2470
Conversation
…n their syntax on default node
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #2470 +/- ##
==========================================
- Coverage 92.29% 92.27% -0.02%
==========================================
Files 115 115
Lines 29759 29763 +4
==========================================
Hits 27465 27465
- Misses 2294 2298 +4
... and 1 file with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
Please, can someone review this? I think it is straightforward. I should also mention, in case this has not been clear, that this is actually a bug fix. Without this patch, the TimeSeries commands mentioned above cause an exception when executed on Redis Cluster. I have no permission to add the "bug" issue label myself though. |
|
@dvora-h If you have a couple of minutes, I would appreciate it if you could review this. |
|
This pull request is marked stale. It will be closed in 30 days if it is not updated. |
Some commands of the TimeSeries module, such as TS.MGET, TS.MRANGE, TS.MREVRANGE and TS.QUERYINDEX, do not include explicitly the keys (time series) they will operate on in their syntax. Instead, any keys will be possibly matched through filters (by label, value, etc.) that are part of the syntax. Moreover, the matched keys could be stored in different slots of different cluster nodes (shards). So, when we use RedisCluster client to execute these commands on a Redis Cluster, it makes no sense to use the client's slot finding mechanism in order to choose the node to execute the command (which is the default mechanism for most other commands), because there is no key to be used for that in the arguments of the command. With this change, the aforementioned commands will be handled separately and sent to the default cluster node instead.
A fix is also included for a case in which the server may respond on the "COMMAND" command with a "subcommands" field with an empty list as its value, which, along with some other attributes in the response, would lead to an exception because the "subcommands" field was not checked before for equaling to an empty list.