[GitHub] [pulsar] Jason918 commented on a change in pull request #14382: [pulsar-broker] support client configurable message chunk size

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

Jason918 commented on a change in pull request #14382:
URL: https://github.com/apache/pulsar/pull/14382#discussion_r822234916



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
##########
@@ -492,6 +496,7 @@ public void sendAsync(Message<?> message, SendCallback callback) {
                 compressedPayload.release();
                 return;
             }
+            payloadChunkSize = Math.min(chunkMaxMessageSize, payloadChunkSize);

Review comment:
       I think we can remove the field `chunkMaxMessageSize` and use the value from conf directly.

##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
##########
@@ -1525,6 +1530,7 @@ public int messagesCount() {
     @Override
     public void connectionOpened(final ClientCnx cnx) {
         previousExceptions.clear();
+        chunkMaxMessageSize = Math.min(chunkMaxMessageSize, ClientCnx.getMaxMessageSize());

Review comment:
       If we assume that `ClientCnx.getMaxMessageSize()` may change on new connections. Consider the case :
   1. Previous connection: `conf.getChunkMaxMessageSize()` == 100 and `ClientCnx.getMaxMessageSize() == 50`, `chunkMaxMessageSize` init as 50;
   2. New connection: `chunkMaxMessageSize` == 50 and `ClientCnx.getMaxMessageSize()` == 200, the result is 50, but it should be 100.




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