You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by "BewareMyPower (via GitHub)" <gi...@apache.org> on 2023/03/18 05:25:36 UTC
[GitHub] [pulsar-client-go] BewareMyPower opened a new pull request, #994: Fix batched messages not ACKed correctly when batch index ACK is disabled
BewareMyPower opened a new pull request, #994:
URL: https://github.com/apache/pulsar-client-go/pull/994
Fixes https://github.com/apache/pulsar-client-go/issues/993
### Motivation
When batch index ACK is disabled, if N messages in a batch are acknowledged, currently only the batched message ID of the last message will be acknowledged. This behavior is wrong because we need to acknowledge the whole batch.
### Modifications
- Create a `messageID` instance to ACK for this case
- Add `TestConsumerBatchIndexAckDisabled` to cover this case
--
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
[GitHub] [pulsar-client-go] BewareMyPower commented on a diff in pull request #994: Fix batched messages not ACKed correctly when batch index ACK is disabled
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on code in PR #994:
URL: https://github.com/apache/pulsar-client-go/pull/994#discussion_r1141951441
##########
pulsar/consumer_partition.go:
##########
@@ -499,6 +499,12 @@ func (pc *partitionConsumer) ackID(msgID MessageID, withResponse bool) error {
trackingID := toTrackingMessageID(msgID)
if trackingID != nil && trackingID.ack() {
+ trackingID = &trackingMessageID{
Review Comment:
Good suggestion. I agree with you.
--
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
[GitHub] [pulsar-client-go] shibd commented on a diff in pull request #994: Fix batched messages not ACKed correctly when batch index ACK is disabled
Posted by "shibd (via GitHub)" <gi...@apache.org>.
shibd commented on code in PR #994:
URL: https://github.com/apache/pulsar-client-go/pull/994#discussion_r1141824147
##########
pulsar/consumer_partition.go:
##########
@@ -499,6 +499,12 @@ func (pc *partitionConsumer) ackID(msgID MessageID, withResponse bool) error {
trackingID := toTrackingMessageID(msgID)
if trackingID != nil && trackingID.ack() {
+ trackingID = &trackingMessageID{
Review Comment:
It is best to add some comments explaining why, which can be intuitively understood by developers who are not familiar with this implementation
--
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
[GitHub] [pulsar-client-go] shibd merged pull request #994: Fix batched messages not ACKed correctly when batch index ACK is disabled
Posted by "shibd (via GitHub)" <gi...@apache.org>.
shibd merged PR #994:
URL: https://github.com/apache/pulsar-client-go/pull/994
--
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
[GitHub] [pulsar-client-go] BewareMyPower commented on a diff in pull request #994: Fix batched messages not ACKed correctly when batch index ACK is disabled
Posted by "BewareMyPower (via GitHub)" <gi...@apache.org>.
BewareMyPower commented on code in PR #994:
URL: https://github.com/apache/pulsar-client-go/pull/994#discussion_r1142038756
##########
pulsar/consumer_partition.go:
##########
@@ -499,6 +499,12 @@ func (pc *partitionConsumer) ackID(msgID MessageID, withResponse bool) error {
trackingID := toTrackingMessageID(msgID)
if trackingID != nil && trackingID.ack() {
+ trackingID = &trackingMessageID{
Review Comment:
Addressed.
--
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
[GitHub] [pulsar-client-go] RobertIndie commented on a diff in pull request #994: Fix batched messages not ACKed correctly when batch index ACK is disabled
Posted by "RobertIndie (via GitHub)" <gi...@apache.org>.
RobertIndie commented on code in PR #994:
URL: https://github.com/apache/pulsar-client-go/pull/994#discussion_r1142024286
##########
pulsar/consumer_partition.go:
##########
@@ -499,6 +499,12 @@ func (pc *partitionConsumer) ackID(msgID MessageID, withResponse bool) error {
trackingID := toTrackingMessageID(msgID)
if trackingID != nil && trackingID.ack() {
+ trackingID = &trackingMessageID{
Review Comment:
+1
--
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