[I] [Bug] MultiTopicsConsumer might change message processing order when there's an acknowledgement time [pulsar]

["lhotari (via GitHub)" <gi...@apache.org>]

lhotari opened a new issue, #22863:
URL: https://github.com/apache/pulsar/issues/22863

   ### Search before asking
   
   - [X] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar.
   
   
   ### Read release policy
   
   - [X] I understand that unsupported versions don't get bug fixes. I will attempt to reproduce the issue on a supported version of Pulsar client and Pulsar broker.
   
   
   ### Version
   
   master
   
   ### Minimal reproduce step
   
   Analysed by looking at code
   https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiTopicsConsumerImpl.java#L889-L911
   
   ### What did you expect to see?
   
   Message ordering shouldn't change. 
   
   ### What did you see instead?
   
   Message ordering could change. There are also other problems in this code since this logic races with message consuming by the application or message listener.
   
   ### Anything else?
   
   _No response_
   
   ### Are you willing to submit a PR?
   
   - [ ] I'm willing to submit a PR!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Bug][client] Consumer implementation might change message processing order when ack timeout is set [pulsar]

["merlimat (via GitHub)" <gi...@apache.org>]

merlimat commented on issue #22863:
URL: https://github.com/apache/pulsar/issues/22863#issuecomment-6381509027

   Checked on current master. The single-topic path is fixed: #23943 changed `ConsumerImpl.removeExpiredMessagesFromQueue` (pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java:3247) to use `incomingMessages.pollIf(...)`, so it only removes expired messages at the head of the queue. `MultiTopicsConsumerImpl.removeExpiredMessagesFromQueue` (pulsar-client/src/main/java/org/apache/pulsar/client/impl/MultiTopicsConsumerImpl.java:954) still has the old logic. It polls the first message that has not expired, discards it and adds it to the redelivery set (line 969), so that message is redelivered out of order. Its peek/poll sequence also still races with the application consuming. The likely fix is to apply the same `pollIf` approach in `MultiTopicsConsumerImpl`.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org