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
Support acknowledging a list of messages #7688
Conversation
I don't understand the reason for this change. there's no performance improvement in this versus just acking the individual messages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some restrictions when using the MessagesImpl, I think we can add new methods acknowledge(List<Message> msgs)
and acknowledge(List<MessageIds> msgIds)
, for some users that they already have a message list for message ID list, they can simpler batch ack messages.
Thank you @codelipenghui. |
@pg2404 I understand the motivation. However, I am not sure we are implementing this in the right way. In your use case, can't you use batchReceive to receive a batch of messages and ack the whole batch? If we want to enhance the API to support batch acknowledgment, doesn't it make more sense to support
|
@sijie - I was only looking for suggestions to implement my use case really - any guidance would be helpful even if a new API is not exposed. My basic pain point being, even if I do a receive a batch - I cannot guarantee that the entire batch will fail or pass in the single shot - I have to group the data on certain metadata and divide the messages into smaller batches and write them somewhere - if the entire process succeeds for the entire batch only then can I send the ack. Suppose, out of the batch I have 4 sub groups and out of which 2 fails I cannot ack the batch - the ack has to be done per message. |
Requirements: Solution: Suggestion: Solution2:
|
/pulsarbot run-failure-checks |
Fixes apache#7626 ### Motivation Expose `MessagesImpl` ,so that we can ack list of messages by using `CompletableFuture<Void> acknowledgeAsync(Messages<?> messages)` ### Modifications Change the visibility level of the method from protect to public ### Verifying this change unit test: org.apache.pulsar.client.api.ConsumerBatchReceiveTest#testBatchAck
Fixes apache#7626 ### Motivation Expose `MessagesImpl` ,so that we can ack list of messages by using `CompletableFuture<Void> acknowledgeAsync(Messages<?> messages)` ### Modifications Change the visibility level of the method from protect to public ### Verifying this change unit test: org.apache.pulsar.client.api.ConsumerBatchReceiveTest#testBatchAck
Fixes apache#7626 ### Motivation Expose `MessagesImpl` ,so that we can ack list of messages by using `CompletableFuture<Void> acknowledgeAsync(Messages<?> messages)` ### Modifications Change the visibility level of the method from protect to public ### Verifying this change unit test: org.apache.pulsar.client.api.ConsumerBatchReceiveTest#testBatchAck
Fixes apache#7626 ### Motivation Expose `MessagesImpl` ,so that we can ack list of messages by using `CompletableFuture<Void> acknowledgeAsync(Messages<?> messages)` ### Modifications Change the visibility level of the method from protect to public ### Verifying this change unit test: org.apache.pulsar.client.api.ConsumerBatchReceiveTest#testBatchAck
Fixes apache#7626 ### Motivation Expose `MessagesImpl` ,so that we can ack list of messages by using `CompletableFuture<Void> acknowledgeAsync(Messages<?> messages)` ### Modifications Change the visibility level of the method from protect to public ### Verifying this change unit test: org.apache.pulsar.client.api.ConsumerBatchReceiveTest#testBatchAck
Fixes #7626
Motivation
Expose
MessagesImpl
,so that we can ack list of messages by usingCompletableFuture<Void> acknowledgeAsync(Messages<?> messages)
Modifications
Change the visibility level of the method from protect to public
Verifying this change
unit test:
org.apache.pulsar.client.api.ConsumerBatchReceiveTest#testBatchAck