flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [flink] zhijiangW commented on issue #8310: [FLINK-12331][network] Introduce partition/gate setup to decouple task registration with NetworkEnvironment
Date Mon, 06 May 2019 04:12:16 GMT
zhijiangW commented on issue #8310: [FLINK-12331][network] Introduce partition/gate setup to
decouple task registration with NetworkEnvironment
URL: https://github.com/apache/flink/pull/8310#issuecomment-489495824
 
 
   Thanks for further reviews @azagrebin 
   
   My previous understanding of providing `Supplier` for partition/gate was not changing the
existing main processes much, then the unit tests could almost work after the refactoring.

   
   But from the aspect of circular dependency you mentioned, I guess you mean the `NetworkEnvironment`
relies on `ResultPartition` and `SingleInputGate` during setup `BufferPool`, meanwhile the
constructors of `ResultPartition` and `SingleInputGate` also rely on some components of `NetworkEnvironment`.

   
   To be honest, I am not very sure what is the benefits of decoupling among them, because
the `ResultPartition/SingleInputGate` are both from `NetworkEnvironment`. It seems reasonable
to make them dependent with each other. 
   
   - The proposed `MemorySegmentProvider` could avoid relying on `NetworkBufferPool` in partition/gate,
but partition/gate would also rely on other components `ResultPartitionManager`, `ConnectionManager`,
`IOManager` etc from `NetworkEnvironment`, so it still could not solve circular dependency
completely. 
   
   - We could not rely on `ResultPartition/SingleInputGate` directly in `NetworkEnvironment#setupPartition/Gate`
by passing specific parameters `ResultPartitionType`, `numberOfChannels` etc. But considering
not affecting tests, I have not changed this.
   
   - The proposed `MemorySegmentProvider` interface seems a bit overlap with current `BufferProvider`
in semantics. Furthermore we might have only one type of implementation in `NetworkEnvironment`.
So it seems not very reasonable to define a new interface to replace specific `NetworkBufferPool`
here.
   
   Maybe there are any other concerns I have not caught up with? Nevertheless I update the
codes based on your above suggestions of introducing `MemorySegmentProvider` and `Supplier<BufferPool>`
(I have not fixed the tests), then we could further confirm whether this way is suitable.


----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message