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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to