azagrebin commented on a change in pull request #13958:
URL: https://github.com/apache/flink/pull/13958#discussion_r519676104
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/scheduler/adapter/DefaultExecutionTopology.java
##########
@@ -51,20 +53,20 @@
private static final Logger LOG =
LoggerFactory.getLogger(DefaultExecutionTopology.class);
- private final ExecutionGraph executionGraph;
-
private final Map<ExecutionVertexID, DefaultExecutionVertex>
executionVerticesById;
private final List<DefaultExecutionVertex> executionVerticesList;
private final Map<IntermediateResultPartitionID,
DefaultResultPartition> resultPartitionsById;
- private final Map<ExecutionVertexID, DefaultSchedulingPipelinedRegion>
pipelinedRegionsByVertex;
+ @Nullable
+ private Map<ExecutionVertexID, DefaultSchedulingPipelinedRegion>
pipelinedRegionsByVertex;
- private final List<DefaultSchedulingPipelinedRegion> pipelinedRegions;
+ @Nullable
+ private List<DefaultSchedulingPipelinedRegion> pipelinedRegions;
- public DefaultExecutionTopology(ExecutionGraph graph) {
- this.executionGraph = checkNotNull(graph, "execution graph can
not be null");
+ private DefaultExecutionTopology(ExecutionGraph graph) {
Review comment:
If we are already factoring out `initializePipelinedRegions`, could we
generate all dependencies of `DefaultExecutionTopology` in the factory method
`fromExecutionGraph`? This should generally separate concerns and make
potential testing simpler.
I also would put this commit before the actual change of this PR if this is
not much effort.
----------------------------------------------------------------
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]
