You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/09/12 22:36:36 UTC

[GitHub] [kafka] hachikuji commented on a diff in pull request #12625: KAFKA-14222; KRaft's memory pool should always allocate a buffer

hachikuji commented on code in PR #12625:
URL: https://github.com/apache/kafka/pull/12625#discussion_r968988957


##########
raft/src/test/java/org/apache/kafka/raft/internals/BatchMemoryPoolTest.java:
##########
@@ -104,4 +128,15 @@ public void testReleaseBufferNotMatchingBatchSize() {
         assertThrows(IllegalArgumentException.class, () -> pool.release(buffer));
     }
 
+    private ByteBuffer touch(ByteBuffer buffer) {

Review Comment:
   nit: `touch` seems a little vague. I think we're just trying to simulate some buffer usage?



##########
raft/src/main/java/org/apache/kafka/raft/internals/BatchMemoryPool.java:
##########
@@ -72,7 +74,13 @@ public void release(ByteBuffer previouslyAllocated) {
                     + previouslyAllocated.limit());
             }
 
-            free.offer(previouslyAllocated);
+            // Free the buffer if the number of pooled buffers is already the maximum number of batches.
+            // Otherwise return the buffer to the memory pool.
+            if (free.size() >= maxBatches) {

Review Comment:
   Perhaps we should rename `maxBatches` since it is no longer serving as a max. How about `maxRetainedBatches` or something like that since it is still a bound on the number of batches which the pool can hold onto indefinitely.



##########
raft/src/main/java/org/apache/kafka/raft/internals/BatchMemoryPool.java:
##########
@@ -90,18 +98,12 @@ public long size() {
 
     @Override
     public long availableMemory() {
-        lock.lock();
-        try {
-            int freeBatches = free.size() + (maxBatches - numAllocatedBatches);
-            return freeBatches * (long) batchSize;
-        } finally {
-            lock.unlock();
-        }
+        return Integer.MAX_VALUE;

Review Comment:
   2 billion bytes is 2GB? Is that enough?



-- 
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: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org