Fix processing of redis multibulk (nested array) responses (from eval, new commands, etc) #612
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
i.e. the response of georadiuswithcoord. This is achieved by introducing a small stack in struct msg, this stack remembers the number of arguments to be parsed of current multibulk and all nesting multibulks. Co-Authored-By: qingping209 <[email protected]> Co-Authored-By: Tyson Andre <[email protected]> This is a subset of twitter#565 (only the multibulk implementation changes by qingping209)
This amends the approach originally proposed in twitter#565 We don't need to keep a stack, we only need to keep track of how many more remaining elements need to be parsed from the redis response, decreasing it when an element is processed, and increasing it when a new redis array is seen based on the array length
TysonAndre
force-pushed
the
twitter-fix-multibulk
branch
from
92f3039 to
523348f
Compare
This was referenced Jul 1, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Redis hangs or misbehaves when processing multibulk redis responses (nested arrays)
Solution
Keep track of how many more elements need to be parsed when parsing the redis response,
as well as whether the top-level value is a multi-bulk response
Continue to properly count the number of top level array elements for request types that can be fragmented such as multi-gets
Result
twemproxy will now be able to proxy results of
evaland other commands when they return nested arrays(this will help in implementing
commandandgeoradiuswithcoord. commands in the future)This is an amended version of #565