Shawyeok commented on PR #26135: URL: https://github.com/apache/pulsar/pull/26135#issuecomment-4873798919
> > The main fix in `receiveIndividualMessagesFromBatch` looks correct to me: syncing the broker-provided ack set into `ackSetInMessageId` fixes the normal batch path. > > However, I think the coverage/fix is incomplete. When a consumer is configured with `messagePayloadProcessor(...)`, `ConsumerImpl.messageReceived` returns through `processPayloadByProcessor(...)`, and `MessagePayloadContextImpl.get(...)` still creates a fresh all-unacked `ackSetInMessageId`. It only uses the broker ack set for `ackBitSet`, so previously acked/skipped batch indexes are not reflected in the `BatchMessageIdImpl`. > > That means consumers using `messagePayloadProcessor(MessagePayloadProcessor.DEFAULT)` or a custom processor can still hit the same fully-acked-batch detection failure, leaving `unAckedMessageTracker` / `possibleSendToDeadLetterTopicMessages` uncleared. > > Could we apply the same `ackSetInMessageId.and(BitSet.valueOf(ackSet))` logic to the payload-processor path as well, preferably via a shared helper, and add a regression variant that sets `.messagePayloadProcessor(MessagePayloadProcessor.DEFAULT)`? > > Sounds reasonable. I'll fix it soon. @void-ptr974 Fixed, feel free to take a look. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
