zhijiangW commented on a change in pull request #7713: [FLINK-10995][network]
Copy intermediate serialization results only once for broadcast mode
URL: https://github.com/apache/flink/pull/7713#discussion_r263291549
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/api/writer/BroadcastRecordWriter.java
##########
@@ -44,4 +52,67 @@ public BroadcastRecordWriter(
public void emit(T record) throws IOException, InterruptedException {
broadcastEmit(record);
}
+
+ @Override
+ public void broadcastEmit(T record) throws IOException,
InterruptedException {
Review comment:
Regarding with abstracting `RecordWriter`, I have some plans for your
confirm.
1. Make current `RecordWriter` as abstract class, so it is no need to
refactor existing references in other places. Then it has two different
implementations `BroadcastRecordWriter` and `SelectorRecordWriter` separately.
2. There are two main differences for these two implementations.
`BroadcastRecordWriter` maintain single `BufferBuilder` and no need
`ChannelSelector`. `SelectorRecordWriter` maintains the array of
`BufferBuilder` and has the `ChannelSelector`.
3. Based on above two differences, we might need to abstract related methods
such as `emit`, `randomEmit`, `broadcastEmit`, `getBufferBuilder`,
`tryFinishCurrentBufferBuilder`, `requestNewBufferBuilder`,
`closeBufferBuilder`.
4. In order to reuse the common main process of
`RecordWriter#copyFromSerializerToTargetChannel` for all kinds of emit, we
still need the boolean flag `randomTriggered` in `BroadcastRecordWriter`,
because it would have two different paths in `requestNewBufferBuilder` and
`flushTargetPartition` based on this condition. And this flag would only be
updated to true at the beginning of `BroadcastRecordWriter#randomEmit`, and
updated to false at the end of this method.
Do you have other concerns for above thoughts ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
