Consumers blocked with 0 availablePermits about pulsar

[merlimat]

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

[sschepens]

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]

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]

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

[sschepens]

@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]

Ok. thanks, I will look into it.

[sschepens]

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

[merlimat]

@sschepens just got back in office. reviewing the PRs

[sschepens]

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]

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]

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

[merlimat]

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]

Fixed in #101