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

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

lhotari commented on a change in pull request #12343:
URL: https://github.com/apache/pulsar/pull/12343#discussion_r754842709



##########
File path: 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 When reading the code, it seems that messageId should never be null. As @michaeljmarshall pointed out, there could be another issue hiding here. 
   It might be a thread safety issue which could cause other problems. @gaozhangmin  Did you check how messageId could become null?




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