1996fanrui commented on code in PR #22381:
URL: https://github.com/apache/flink/pull/22381#discussion_r1164914879
##########
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/buffer/LocalBufferPool.java:
##########
@@ -671,13 +675,20 @@ public void setNumBuffers(int numBuffers) {
currentPoolSize = Math.min(numBuffers, maxNumberOfMemorySegments);
- // reset overdraft buffers
+ // If pool size increases, try to convert overdraft buffer to
ordinary buffer.
while (numberOfRequestedOverdraftMemorySegments > 0
&& numberOfRequestedMemorySegments < currentPoolSize) {
numberOfRequestedOverdraftMemorySegments--;
numberOfRequestedMemorySegments++;
}
+ // If pool size decreases, try to convert ordinary buffer to
overdraft buffer.
+ while (numberOfRequestedMemorySegments > currentPoolSize
+ && numberOfRequestedOverdraftMemorySegments <
maxOverdraftBuffersPerGate) {
+ numberOfRequestedMemorySegments--;
+ numberOfRequestedOverdraftMemorySegments++;
+ }
Review Comment:
Thanks for the clarification.
> When numberOfRequestedMemorySegments <= poolSize, all buffers are ordinary
buffer.
> When numberOfRequestedMemorySegments > poolSize, the `ordinary buffer size
= poolSize`, and `the overdraft buffer size = numberOfRequestedMemorySegments -
poolSize`
From the discussion of FLINK-31610, this
restriction(`numberOfRequestedOverdraftMemorySegments <=
maxOverdraftBuffersPerGate`) will be broken in FLINK-31764, right?
If so, I prefer FLINK-31764 only refactor the code, not change the overdraft
strategy or behavior, and we change the strategy in this PR, WDYT?
Please correct my if my understanding is wrong, thanks!
##########
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/buffer/LocalBufferPool.java:
##########
@@ -49,7 +49,11 @@
*
* <p>The size of this pool can be dynamically changed at runtime ({@link
#setNumBuffers(int)}. It
* will then lazily return the required number of buffers to the {@link
NetworkBufferPool} to match
- * its new size.
+ * its new size. New buffers can be requested only when {@code
numberOfRequestedMemorySegments +
+ * numberOfRequestedOverdraftMemorySegments < currentPoolSize +
maxOverdraftBuffersPerGate}. In
Review Comment:
This PR just ensure that new buffers cannot be requested when `the total
number of requested buffers` exceeds `poolSize + maxOverdraftBuffersPerGate`,
and cannot ensure that `the total number of requested buffers does not exceed
poolSize + maxOverdraftBuffersPerGate`, right?
If so, the commit message and JIRA title should be changed.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
