[GitHub] [pulsar] BewareMyPower commented on a diff in pull request #16072: [fix][Java Client] Fix thread safety issue of `LastCumulativeAck`

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

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


##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/PersistentAcknowledgmentsGroupingTracker.java:
##########
@@ -116,8 +118,8 @@ public PersistentAcknowledgmentsGroupingTracker(ConsumerImpl<?> consumer, Consum
      * resent after a disconnection and for which the user has already sent an acknowledgement.
      */
     @Override
-    public boolean isDuplicate(@NonNull MessageId messageId) {
-        final MessageId messageIdOfLastAck = lastCumulativeAck.messageId;
+    public boolean isDuplicate(MessageId messageId) {
+        final MessageIdImpl messageIdOfLastAck = lastCumulativeAck.getMessageId();

Review Comment:
   It's added by me before. The null check is redundant here because the only usage accepts a `MessageId` object returned by `new`, which cannot be `null`. See the following code:
   
   See https://github.com/apache/pulsar/blob/d5d8923265b3a4b9731892879b2f2b9dc48c0f98/pulsar-client/src/main/java/org/apache/pulsar/client/impl/ConsumerImpl.java#L1267-L1268
   
   In addition, `@NonNull` will still throw an exception, see https://projectlombok.org/features/NonNull.



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