Skip to content
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

Consumers blocked with 0 availablePermits #100

Closed
sschepens opened this issue Nov 1, 2016 · 13 comments
Closed

Consumers blocked with 0 availablePermits #100

sschepens opened this issue Nov 1, 2016 · 13 comments
Labels
type/bug The PR fixed a bug or issue reported a bug
Milestone

Comments

@sschepens
Copy link
Contributor

When a consumer with ackTimeout connects, it negotiates a given amount of permits with broker (say 1000), it then receives 1000 messages. At this point the consumer has 0 availablePermits and 1000 unackedMessages, If no one pulls these messages from this consumer, they are eventually redelivered to the broker, now the broker schedules redelivery of those messages, but never increments availablePermits for the consumer, leaving the consumer stuck with 0 availablePermits.

This happens at least with perMessageRedelivery, as a redelivery does not necessarily imply that permits have to be granted to the consumer. Unfortunately, I don't know if the Broker can really do something here.

I believe this issue is a consequence of a consumer asking for redelivery of messages that have never been pulled from the queue.

The best approach seems to be for the consumer to ask for more permits when this is the case.
What do you think?

@merlimat
Copy link
Contributor

merlimat commented Nov 1, 2016

I'm going to be out for the next few days. @rdhabalia can you take a look at this?

@sschepens
Copy link
Contributor Author

sschepens commented Nov 1, 2016

Also, redeliverUnacknowledgedMessages(List<MessageIdImpl> messageIds) doesn't seem to be removing redelivered messages from the queue at any time, so the queue will always be stuck with redelivered messages.

I can't think of any simple way of solving this right now, maybe we can keep track of messages in the queue on a separate set.

@rdhabalia
Copy link
Contributor

redeliverUnacknowledgedMessages : it cleans up all the messages present into incomingMessages and sends separate flow-message command which increments availablePermits at broker for a given consumer. So, consumer will not be stuck at any time.

However, right redeliverUnacknowledgedMessages doesn't clean up incomingMessages queue and asks broker for redelivery. So, as soon as client consumes N/2 messages from the incomingMessages queue it triggers flow-message command to broker and broker will first release those redelivered messages as part of received permits. In this case: again consumer will not be stuck.

Also, redeliverUnacknowledgedMessages(List messageIds) doesn't seem to be removing redelivered messages from the queue at any time

Yes, we have to make similar change of redeliverUnacknowledgedMessages to clean up from incomingMessages queue and sends permits to broker by triggering flow-message command.

@rdhabalia
Copy link
Contributor

I will make changes and add test-cases for this scenario.

@sschepens
Copy link
Contributor Author

@rdhabalia I just created a PR for this, please give it a look, perhaps you can think of a better way of approaching this.

@rdhabalia
Copy link
Contributor

Ok. thanks, I will look into it.

@sschepens
Copy link
Contributor Author

@merlimat @rdhabalia any news on this? We really need to solve this issue

@merlimat
Copy link
Contributor

merlimat commented Nov 9, 2016

@sschepens just got back in office. reviewing the PRs

@sschepens
Copy link
Contributor Author

Just to clarify, this issue was introduced when tracking timeout on messages not pulled from the queue.
Perhaps we can take a different approach to this, in the end what we want to prevent is a clientes to hold messages when it's not processing.
We could empty the queue and not as for messages until it resumes acknowledging messages or something of the sort.

@merlimat
Copy link
Contributor

merlimat commented Nov 9, 2016

OK, it looks like it would be easier to just empty the queue and have the broker resend the all the messages that were on it. Anyway, they're not going to be "duplicates" since the application would not have seen them already.

@sschepens
Copy link
Contributor Author

Yup, but we maybe should make the consumer not receive messages until it resumes acks. Just to prevent constant redelivery.

@merlimat
Copy link
Contributor

merlimat commented Nov 9, 2016

I agree on that, the library should be playing with the permits to do that, but I'd keep it separate from this PR, because:

  1. It might be tricky to make sure we don't forget any corner case and leave the delivery stopped
  2. It should not be super-awful anyway.. you get 1000 messages resent every n-minutes

@merlimat merlimat added this to the 1.16 milestone Nov 16, 2016
@merlimat merlimat added the type/bug The PR fixed a bug or issue reported a bug label Nov 16, 2016
@merlimat
Copy link
Contributor

Fixed in #101

sijie pushed a commit to sijie/pulsar that referenced this issue Mar 4, 2018
hrsakai pushed a commit to hrsakai/pulsar that referenced this issue Dec 10, 2020
* Add multi topic consumer.

* Remove unused lock.
hangc0276 pushed a commit to hangc0276/pulsar that referenced this issue May 26, 2021
in PR apache#82, structure is refined, but publish.sh leaked the change.


* fix publish.sh for dir refine

* delete integrations/__init__.py

* ignore dir dev in script
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

No branches or pull requests

3 participants