-
Notifications
You must be signed in to change notification settings - Fork 566
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix some low level stream commands for redis cluster #283
Conversation
- xgroup and xinfo they have key position on 3rd place - xread and xreadgroup have key right after 'STREAMS' keyword
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See inline nit, but I would be ok with merging (and fixing it afterwards)
src/cluster.rs
Outdated
b"XREAD" | b"XREADGROUP" => { | ||
let streams_position = args | ||
.iter() | ||
.position(|a| *a == Value::Data(b"STREAMS".to_vec()))?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mini-optimisation: we can pull out the Value::Data(b"STREAMS".to_vec())
before the whole iteration, therefore saving us the frequent re-creation (and thus re-allocation)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed that somehow, thanks.
It should be ok now?
…ction Add transaction support to Node.
* Python: add SMOVE command (redis-rs#283) * Align docs and cross-slot test with recent changes * Fix formatting
Fix key position for stream commands:
xgroup
andxinfo
have key position on 3rd placexread
andxreadgroup
have key right after STREAMS keywordSmall note:
xread
andxreadgroup
command can take multiple streams (multiple keys) and my proposed solution is calculating hash only based on first key.I think this assumption is correct because 2 things could happen:
(error) CROSSSLOT Keys in request don't hash to the same slot