[GitHub] [flink] 1996fanrui commented on a diff in pull request #22084: [WIP][FLINK-31293][runtime] LocalBufferPool request overdraft buffer only when no available buffer and pool size is reached

["1996fanrui (via GitHub)" <gi...@apache.org>]

1996fanrui commented on code in PR #22084:
URL: https://github.com/apache/flink/pull/22084#discussion_r1125605884


##########
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/buffer/LocalBufferPool.java:
##########
@@ -396,7 +396,7 @@ private MemorySegment requestMemorySegment(int targetChannel) {
         synchronized (availableMemorySegments) {
             checkDestroyed();
 
-            if (availableMemorySegments.isEmpty()) {
+            if (availableMemorySegments.isEmpty() && isRequestedSizeReached()) {

Review Comment:
   I'm not sure whether `availableMemorySegments.poll();` should be executed when `availableMemorySegments.isEmpty()`. 
   
   1. How about call it just when `!availableMemorySegments.isEmpty()`?
   2. We should add some comments for `requestOverdraftMemorySegmentFromGlobal`, explain when to request the overdraft buffer.
   
   ```suggestion
               if (!availableMemorySegments.isEmpty()) {
                   segment = availableMemorySegments.poll();
               } else if (isRequestedSizeReached()) {
                   // Only when the buffer request reaches the upper limit, requests an overdraft buffer
                   segment = requestOverdraftMemorySegmentFromGlobal();
               }
   ```



-- 
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: issues-unsubscribe@flink.apache.org

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