zhijiangW commented on a change in pull request #7199: [FLINK-10662][network]
Refactor the ChannelSelector interface for single selected channel
URL: https://github.com/apache/flink/pull/7199#discussion_r250484212
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/api/writer/RecordWriter.java
##########
@@ -68,20 +68,29 @@
private final boolean flushAlways;
+ private final boolean isBroadcastSelector;
+
private Counter numBytesOut = new SimpleCounter();
private Counter numBuffersOut = new SimpleCounter();
public RecordWriter(ResultPartitionWriter writer) {
- this(writer, new RoundRobinChannelSelector<T>());
+ this(writer, new RoundRobinChannelSelector<T>(), false);
}
- @SuppressWarnings("unchecked")
- public RecordWriter(ResultPartitionWriter writer, ChannelSelector<T>
channelSelector) {
- this(writer, channelSelector, false);
+ public RecordWriter(
Review comment:
I am considering the way for solving this issue.
I agree it seems better to introduce either a builder method or
`RecordWriterBuilder` class for creating the specific RecordWriter. But it
seems unfeasible to check ChannelSelector instance in current architecture,
because the classes of `BatchTask` and `StreamTask` are in different modules
and the` instanceof BroadcastPartitioner` check would only work under streaming
module, not meanwhile work for `BatchTask` in runtime module.
There might be two ways for this issue:
1. Introduce `ChannelSelector#isBroadcast` method to solve the module
dependence.
2. Check the broadcast mode separately in `StreamTask` and `BatchTask`
classes, that means no unified builder for creating `RecordWriter`
I prefer the second way a bit in order not to change the interface much,
although this check instance would be scattered in two different places. What
do you think?
----------------------------------------------------------------
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
