zhijiangW commented on a change in pull request #7631: [FLINK-11391][shuffle] 
Introduce PartitionShuffleDescriptor and ShuffleDeploymentDescriptor
URL: https://github.com/apache/flink/pull/7631#discussion_r255818212
 
 

 ##########
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/deployment/InputChannelDeploymentDescriptor.java
 ##########
 @@ -52,30 +52,38 @@
        /** The ID of the partition the input channel is going to consume. */
        private final ResultPartitionID consumedPartitionId;
 
-       /** The location of the partition the input channel is going to 
consume. */
-       private final ResultPartitionLocation consumedPartitionLocation;
+       /** The location type of the partition the input channel is going to 
consume. */
+       private final LocationType locationType;
+
+       /** The connection to use to request the remote partition. */
+       private final Optional<ConnectionID> connectionId;
 
 Review comment:
   I also considered using `ShuffleDeploymentDescriptor` here to replace 
`ConnectionID` before. But there are two concerns in implementation:
   
   1.  In eager schedule mode when receiving all required slots, we might not 
assume the deployment sequence must be strict with topology sequence. That 
means the consumer execution deployment might be earlier than the producer 
execution. So in the process of `InputChannelDeploymentDescriptor#fromEdges`, 
we might not get cached SDD directly from producer execution. But we can 
generate `ConnectionID` based on other infos. Otherwise we must confirm the 
deployment sequence is from producer to consumer or generate producer's SDD 
during deploying consumer in `InputChannelDeploymentDescriptor#fromEdges`.
   
   2. I thought of introducing `UnknownShuffleDeploymentDescriptor` before, but 
from semantic aspect it is a bit redundant with `LocationType.Unknown`. In 
addition, it seems no specific usages like ` instanceof 
UnknownShuffleDeploymentDescriptor` in other processes.  The SDD should be 
generated by `ShuffleMaster` by design, but the special 
`UnknownShuffleDeploymentDescriptor` is generated only in the case of 
`LocationType.Unknown` which is not via `ShuffleMaster`.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services

Reply via email to