zhijiangW commented on a change in pull request #8191: [FLINK-12213][network]
Pass TaskManagerMetricGroup into constructor of NetworkEnvironment
URL: https://github.com/apache/flink/pull/8191#discussion_r276534055
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/NetworkEnvironment.java
##########
@@ -63,11 +70,19 @@
private boolean isShutdown;
- public NetworkEnvironment(NetworkEnvironmentConfiguration config,
TaskEventPublisher taskEventPublisher) {
+ public NetworkEnvironment(
Review comment:
I guess your mentioned factory is similar with the `Builder` function to
create every internal components or pass them from outsides as in test usages.
I thought of this issue from the motivation of tests aspect, but there seems
only one `TaskEventDispatcher` component, so I have not focused on it before.
Now I add the new component `ComponentMetricGroup`, it seems necessary.
My suggestion is providing the simple build method atm for avoiding creating
these two components in related tests. The final form of factory is related to
`NetworkEnvironment#fromConfiguration(flink configuration)` and
`ShuffleMaster.createShuffleService(flinkConfig)`, so it might be better to
consider them together when working the PR for
`ShuffleMaster.createShuffleService(flinkConfig)`. What do you think?
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
