-
Notifications
You must be signed in to change notification settings - Fork 473
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(stream): Fix XPENDING serialization issue #2566
Conversation
Stream test time out after 30 minutes. Maybe there is a block or infinite wait (for example, waiting for a message that is never added). |
@nathanlo-hrt Thank you for this catch. |
So.. is this another bug of XPENDING? Or we can just change the test case to fix it? |
I believe this is a bug in my changes to the test code; I'll put up a fix later today. Thanks for the swift reviews! |
Found another issue while testing: Kvrocks currently deviates from Redis' behavior when there are no pending messages. I've added a special case for this and a test to cover the new code. |
@nathanlo-hrt Good job! You can run |
|
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.
LGTM with minor comment.
Quality Gate passedIssues Measures |
Co-authored-by: Aleks Lozovyuk <aleks.raiden@gmail.com> Co-authored-by: Twice <twice.mliu@gmail.com>
Currently
XPENDING
doesn't properly create and serialize an array of the right length: the response to anXPENDING
command is always an array of length 4, regardless of the number of consumer infos it has to send back.The existing tests pass because they all happened to have exactly one consumer.