zhuzhurk commented on a change in pull request #13958:
URL: https://github.com/apache/flink/pull/13958#discussion_r520499647



##########
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:
       The topology consists of vertices, results and edges. 
`DefaultExecutionTopology` currently built these things in the constructor. I 
feel that moving all these things into the factory method does not bring any 
immediate benefit but instead introduces risk. So I prefer to do it only when 
it is needed.




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


Reply via email to