[GitHub] [pulsar] Jason918 commented on a diff in pull request #17936: [fix][java-client]Take into account the buffer allocated by BatchMessageContainerImpl when limiting then memory used

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

Jason918 commented on code in PR #17936:
URL: https://github.com/apache/pulsar/pull/17936#discussion_r988531460


##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageContainerImpl.java:
##########
@@ -102,6 +103,8 @@ public boolean add(MessageImpl<?> msg, SendCallback callback) {
                 this.firstCallback = callback;
                 batchedMessageMetadataAndPayload = allocator.buffer(
                         Math.min(maxBatchSize, ClientCnx.getMaxMessageSize()));
+                extraCapacity = batchedMessageMetadataAndPayload.capacity();
+                producer.client.getMemoryLimitController().forceReserveMemory(extraCapacity);

Review Comment:
   IIUC, the main idea of this change is to limit allocated memory not just used.
   I am not sure if there are better implementations, but can we just reserve memory after allocation and release in `clean()`? , and skip previous reserve/release when it's batched.



##########
pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageContainerImpl.java:
##########
@@ -163,6 +166,11 @@ private ByteBuf getCompressedBatchMetadataAndPayload() {
         int uncompressedSize = batchedMessageMetadataAndPayload.readableBytes();
         ByteBuf compressedPayload = compressor.encode(batchedMessageMetadataAndPayload);
         batchedMessageMetadataAndPayload.release();
+
+        int delta = compressedPayload.capacity() - extraCapacity;
+        extraCapacity = compressedPayload.capacity();
+        producer.client.getMemoryLimitController().forceReserveMemory(delta);

Review Comment:
   It's not a good idea to reserve memory in a "get" method. We should always consider that a "get" method can be called anytime.



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