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_r262877734
##########
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:
Thought of another two points:
- `randomTriggered` field could not be removed completely, because the
behaviors are different in the methods of `requestNewBufferBuilder` and
`flushTargetPartition` based on this tag. Otherwise the current
`copyFromSerializerToTargetChannel` could not be reused and
`BroadcastRecordWriter` has to implement the private process of `randomEmit`.
If we could retain this tag field, and finish current `BufferBuilder`
immediately after `randomEmit`. Then only `randomEmit` would change this field,
and only `requestNewBufferBuilder` and `flushTargetPartition` would read this
tag. It would seem easy to understand compared with now.
- Considering immediately finishing current `BufferBuilder` once
`randomEmit` done, it might increase the frequency of finishing
`BufferBuilder`. For example, if the `randomEmit` is triggered very frequently
and there are no common `emit` or `broadcastEmit` calling for a while, one
`BufferBuilder` might write multiple random emitted elements, no need one
`BufferBuilder` for one `randomEmit`. Or this worry is not worth considering
for most scenarios?
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
