[Redis broker] message_refc dips into negative numbers on ConnectionError
#262
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
This can cause prefetching too many messages. The reason is that when we try to call `consumer.ack` or `consumer.nack`, and a `ConnectionError` happens, we will retry. But each time we retry, we decrement `message_refc` again. Solved this by using a set to track queued messages, so calling `ack` or `nack` on the same message doesn't mess up our accounting.
benekastah
commented
Jan 9, 2020
| self.misses = 0 | ||
|
|
||
| @property | ||
| def message_refc(self): |
There was a problem hiding this comment.
This property is so I can use the same test code with this version of the file and with the previous version of the file.
benekastah
changed the title
message_refc dips into negative numbers on ConnectionError message_refc dips into negative numbers on ConnectionError
Bogdanp
requested changes
Jan 20, 2020
| return MessageProxy(message) | ||
| except IndexError: | ||
| # If there are fewer messages currently being | ||
| # processed than we're allowed to prefetch, | ||
| # prefetch up to that number of messages. | ||
| messages = [] | ||
| num_messages = len(self.queued_message_ids) | ||
| if self.message_refc < self.prefetch: |
There was a problem hiding this comment.
Seems like this is a bug since message_refc has been removed. Did you mean to use num_messages here?
|
I made a couple changes here and merged this into master with rebase. Thanks for the fix! |
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.
This can cause prefetching too many messages. The reason is that when we
try to call
consumer.ackorconsumer.nack, and aConnectionErrorhappens, we will retry. But each time we retry, we decrement
message_refcagain. Solved this by using a set to track queued messages, so calling
ackor
nackon the same message doesn't mess up our accounting.