azagrebin commented on a change in pull request #13958:
URL: https://github.com/apache/flink/pull/13958#discussion_r520552244
##########
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:
Ok, I think we can merge the PR as is and then I can open a follow-up PR
right-away with my.
My concern is the private setter as a compromise to work on a partially
constructed Topology.
I still believe that having a separate step-by-step dependencies generation
with immutable data structures is more clear in future. There should be also no
risks if tests pass.
----------------------------------------------------------------
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
