wsry commented on a change in pull request #18173:
URL: https://github.com/apache/flink/pull/18173#discussion_r776549954



##########
File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/buffer/NetworkBufferPool.java
##########
@@ -158,7 +159,8 @@ public MemorySegment requestMemorySegment() {
 
     public List<MemorySegment> requestMemorySegmentsBlocking(int 
numberOfSegmentsToRequest)
             throws IOException {
-        return internalRequestMemorySegments(numberOfSegmentsToRequest);
+        return internalRequestMemorySegments(
+                numberOfSegmentsToRequest, 
this::internalRecycleMemorySegments);

Review comment:
       > Is it because all of the buffers in the LocalBufferPool are floating 
buffers? And that's the basis of the implementation of the whole 
exclusive/floating buffer concept?
   
   I guess you are right, if we use only one LocalBufferPool, the buffers will 
be shared by all remote channels, but the credit based flow control  needs 
exclusive buffers (if we set the exclusive buffer to 0 and makes all buffers 
floating, some network micro benchmarks incurs at most about 20% regression). 
Another possible way is to create a local buffer pool per remote channel?
   
   In a word, we can rename the method name, add some comments or try to unify 
the code to always use local buffer pool (need run micro benchmark to ensure 
that there is no regression). All of these are acceptable to me. WDYT, any 
suggestion?




-- 
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-unsubscr...@flink.apache.org

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


Reply via email to