[GitHub] [pulsar] BewareMyPower commented on a diff in pull request #16196: [fix][Java Client] Fix large message sometimes cannot be split into chunks after PIP-132

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

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


##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java:
##########
@@ -488,6 +488,10 @@ public void sendAsync(Message<?> message, SendCallback callback) {
             return;
         }
 
+        // Update the message metadata before computing the payload chunk size to avoid a large message cannot be split
+        // into chunks.
+        final long sequenceId = updateMessageMetadata(msgMetadata, uncompressedSize);

Review Comment:
   Yes, we can skip `isMessageSizeExceeded` if client chunking is enabled. But I think the existing changes in this PR make the computation of the payload chunk size **more accurate**.
   
   Regarding to the metadata update after the computation,
   
   >  msgMetadata.setUuid(...) and msgMetadata.setChunkId().....
   
   These metadata fields are updated only for chunks:
   
   ```protobuf
       optional string uuid = 26;
       optional int32 num_chunks_from_msg = 27;
       optional int32 total_chunk_msg_size = 28;
       optional int32 chunk_id = 29;
   ```
   
   In this case, `isMessageSizeExceeded` is already skipped.
   
   And yes, `ProducerImpl#encryptMessage` could update the metadata as well. I think it's better to skip the message size check when encryption is enabled, because message encryption could also increase the size of the payload buffer.



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