unlizhao commented on a change in pull request #9817:
URL: https://github.com/apache/kafka/pull/9817#discussion_r553135010



##########
File path: 
clients/src/main/java/org/apache/kafka/clients/producer/internals/BufferPool.java
##########
@@ -212,10 +212,15 @@ protected void recordWaitTime(long timeNs) {
     /**
      * Allocate a buffer.  If buffer allocation fails (e.g. because of OOM) 
then return the size count back to
      * available memory and signal the next waiter if it exists.
+     *
+     * if producer is closing, will throw close-exception
      */
     private ByteBuffer safeAllocateByteBuffer(int size) {
         boolean error = true;
         try {
+            if (this.closed) {

Review comment:
       @chia7712 thanks for your reviewed. This is what I thought when I wrote 
this code 
   
   In the PR of #3053, for efficiency, the operation of allocating memory is 
moved out of the locked code block.
   Subsequent modifications should be based on this consensus, and do some 
time-consuming operations without locking.
   
   So what we can do is to try our best to check whether the memory can be 
allocated before allocating, and reduce the operation of allocating memory, 
just like the 'fail-fast' mechanism.
   
   In this scenario, both `BufferPool#allocate()` and `BufferPool#close() 
`compete for locks.
   Here we can think that lock blocking is more likely to occur in concurrent 
cases.
   
   Finally, even if the memory is allocated in the `close` state, there is a 
plan to release the memory. Therefore, this COMMIT is like an optimization 
rather than bug-fix




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

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


Reply via email to