[GitHub] [pulsar] BewareMyPower commented on a diff in pull request #12343: fix lastCumulativeAck.messageId npe when PersistentAcknowledgmentsGroupingTracker.flushSync

[GitBox <gi...@apache.org>]

BewareMyPower commented on code in PR #12343:
URL: https://github.com/apache/pulsar/pull/12343#discussion_r897714232


##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PersistentAcknowledgmentsGroupingTracker.java:
##########
@@ -477,13 +477,16 @@ public void flush() {
     private void flushAsync(ClientCnx cnx) {
         boolean shouldFlush = false;
         if (cumulativeAckFlushRequired) {
-            newMessageAckCommandAndWrite(cnx, consumer.consumerId, lastCumulativeAck.messageId.ledgerId,
-                    lastCumulativeAck.messageId.getEntryId(), lastCumulativeAck.bitSetRecyclable,
-                    AckType.Cumulative, null, Collections.emptyMap(), false,
-                    this.currentCumulativeAckFuture, null);
-            this.consumer.unAckedChunkedMessageIdSequenceMap.remove(lastCumulativeAck.messageId);
-            shouldFlush = true;
-            cumulativeAckFlushRequired = false;
+            final MessageIdImpl messageIdOfLastAck = lastCumulativeAck.messageId;

Review Comment:
   @gaozhangmin @lhotari I picked up this issue today. After looking deeper into this issue, I found another bug and it would be better to synchronize the update operations of `LastCumulativeAck`. See https://github.com/apache/pulsar/pull/16072, PTAL when you have time.



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