fix(connection): Clear dispatch queue before pipeline squashing #2163
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
dranikpg
commented
Nov 12, 2023
Comment on lines
-1001
to
-1005
| // Dispatch queue could have grown, so handle strictly as many as we executed | ||
| for (size_t i = 0; i < args.size(); i++) { | ||
| recycle(move(dispatch_q_.front())); | ||
| dispatch_q_.pop_front(); | ||
| } |
There was a problem hiding this comment.
The bug here is that we don't account for commands that have been inserted upfront and we were deleting the wrong messages
dranikpg
force-pushed
the
fix-pl-sq-consumption
branch
from
be5264b
to
97220cd
Compare
chakaz
approved these changes
Nov 13, 2023
| @@ -239,6 +240,9 @@ class Connection : public util::Connection { | |||
|
|
|||
| void SendAsync(MessageHandle msg); | |||
|
|
|||
| // Updates memory stats and pooling, must be called for all used messages | |||
| void RecycleMessage(MessageHandle msg); | |||
There was a problem hiding this comment.
Nit: I never liked this name tbh, how about UsedMessageAccounting() or something like that?
There was a problem hiding this comment.
I doesn't sound like an action and somewhat confusing, I'd stick with the current
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.
Fixes a bug in #2156
Move block that is pipeline squashed out of the dispatch queue before applying it, see comment below