wsry commented on code in PR #20200:
URL: https://github.com/apache/flink/pull/20200#discussion_r920828843
##########
flink-runtime/src/main/java/org/apache/flink/runtime/shuffle/PartitionDescriptor.java:
##########
@@ -53,14 +54,22 @@ public class PartitionDescriptor implements Serializable {
/** Connection index to identify this partition of intermediate result. */
private final int connectionIndex;
+ /** Whether the intermediate result is a broadcast result. */
+ private final boolean isBroadcast;
+
+ /** The distribution pattern of the intermediate result. */
+ private final DistributionPattern distributionPattern;
Review Comment:
I would suggest to use a boolean type instead of DistributionPattern to
avoid exposing a the new internal type which is not public. Currently, this new
information is never used in the NettyShuffleMaster, if someone modifies the
DistributionPattern, for example, removing some enum fields, the compatibility
will be broken and the person who modifies DistributionPattern will never
realize that. I think we can use something like this to replace the current
field:
```private final boolean isAllToAllDistribution;```
then we only rely on the all to all property and we convert the
DistributionPattern to isAllToAllDistribution internally, if someone modifies
the code, then he/she must also take care of the internal conversion logic.
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
