[jira] [Comment Edited] (FLINK-31610) Refactoring of LocalBufferPool

["Weijie Guo (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/FLINK-31610?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17709317#comment-17709317 ] 

Weijie Guo edited comment on FLINK-31610 at 4/6/23 12:31 PM:
-------------------------------------------------------------

Thanks [~fanrui] for the clarification.

{quota}
General principle: new buffers can be requested when "numOfRequestedMemorySegments + numberOfRequestedOverdraftMemorySegments < poolSize + maxOverdraftBuffersPerGate".
{quota}

Totally agreed with this principle. If the previous behavior was not what it was designed for, I will create a ticket to do this(i.e. convert {{numberOfRequestedMemorySegments}} to {{numberOfRequestedOverdraftMemorySegments}} when poolSize is decreased).

{quota}
When numberOfRequestedMemorySegments <= poolSize, all buffers are ordinary buffer.
When numberOfRequestedMemorySegments > poolSize, the `ordinary buffer size = poolSize`, and `the overdraft buffer size = numberOfRequestedMemorySegments - poolSize`
{quota}

UUIC, That's exactly what I said in my first comment: {{changes the definition of overdraft buffer from static to dynamic(i.e. Whether it is an overdraft buffer depends on the poolSize at the current moment.)}}., This approach avoid a lot of complexity, I'm +1 for this.







was (Author: weijie guo):
Thanks [~fanrui] for the clarification.

{noformat}
General principle: new buffers can be requested when "numOfRequestedMemorySegments + numberOfRequestedOverdraftMemorySegments < poolSize + maxOverdraftBuffersPerGate".
{noformat}

Totally agreed with this principle. If the previous behavior was not what it was designed for, I will create a ticket to do this(i.e. convert {{numberOfRequestedMemorySegments}} to {{numberOfRequestedOverdraftMemorySegments}} when poolSize is decreased).

{noformat}
When numberOfRequestedMemorySegments <= poolSize, all buffers are ordinary buffer.
When numberOfRequestedMemorySegments > poolSize, the `ordinary buffer size = poolSize`, and `the overdraft buffer size = numberOfRequestedMemorySegments - poolSize`
{noformat}

UUIC, That's exactly what I said in my first comment: {{changes the definition of overdraft buffer from static to dynamic(i.e. Whether it is an overdraft buffer depends on the poolSize at the current moment.)}}., This approach avoid a lot of complexity, I'm +1 for this.






> Refactoring of LocalBufferPool
> ------------------------------
>
>                 Key: FLINK-31610
>                 URL: https://issues.apache.org/jira/browse/FLINK-31610
>             Project: Flink
>          Issue Type: Improvement
>          Components: Runtime / Network
>    Affects Versions: 1.17.0
>            Reporter: Anton Kalashnikov
>            Priority: Major
>
> FLINK-31293 bug highlighted the issue with the internal mutual consistency of different fields in LocalBufferPool. ex.:
> -  `numberOfRequestedOverdraftMemorySegments`
> -  `numberOfRequestedMemorySegments`
> -  `availableMemorySegment`
> -  `currentPoolSize`
> Most of the problem was fixed already(I hope) but it is a good idea to reorganize the code in such a way that all invariants between all fields inside will be clearly determined and difficult to break.
> As one example I can propose getting rid of numberOfRequestedOverdraftMemorySegments and using existing numberOfRequestedMemorySegments instead. That means:
> - the pool will be available when `!availableMemorySegments.isEmpty() && unavailableSubpartitionsCount == 0`
> - we don't request a new `ordinary` buffer when `numberOfRequestedMemorySegments >=  currentPoolSize` but we request the overdraft buffer instead
> - `setNumBuffers` should work automatically without any changes
> I think we can come up with a couple of such improvements to simplify the code.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)