zhijiangW commented on a change in pull request #11687: URL: https://github.com/apache/flink/pull/11687#discussion_r419502301
########## File path: flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/LocalInputChannel.java ########## @@ -87,10 +90,14 @@ public LocalInputChannel( int maxBackoff, InputChannelMetrics metrics) { - super(inputGate, channelIndex, partitionId, initialBackoff, maxBackoff, metrics.getNumBytesInLocalCounter(), metrics.getNumBuffersInLocalCounter()); + super(inputGate, channelIndex, partitionId, initialBackoff, maxBackoff, metrics); this.partitionManager = checkNotNull(partitionManager); this.taskEventPublisher = checkNotNull(taskEventPublisher); + // In most cases we only need one buffer for reading recovered state except for very large record. + // Then only one floating buffer is required. Even though we need more buffers for recovery for + // large record, it only increases some interactions with pool. + this.bufferManager = new BufferManager(this, 1); Review comment: Yes, I agree it is a bit hard to understand. The `numRequiredBuffers` factor is only the complex point of this buffer manager model for getting a bit optimization. Actually we can give any initial value for `numRequiredBuffers` (e.g. 0) for unifying the local and remote channels. And ideally we should adjust this value based on how many total channel states are under unspilling exactly like the concept of `backlog` in credit-based mode. Actually any value for `numRequiredBuffers` can work correctly now and the only cost is increasing some unnecessary interactions between `BufferManager` and `LocalBufferPool`. I am really a bit torn here when implementation whether to retain this optimization. ---------------------------------------------------------------- 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