pnowojski 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_r263323969
 
 

 ##########
 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:
   I don't think that we strongly couple `randomEmit` and `LatencyMarker` with 
my last proposal. The only "bad" thing that I see which we would be doing is 
that `randomEmit()` will have a performance penalties if used on 
`BroadcastRecordWriter`, which I guess is an acceptable trade off. If we start 
using `randomEmit()` for more use cases, we might just need to make selection 
of regular vs `BroadcastRecordWriter` either smarter or configurable.
   
   points 1 to 3 make sense to me. I don't fully understand the issue with 
`copyFromSerializerToTargetChannel`, but this is a small change, so it might be 
just better if you write the code and show it in the PR instead of explaining.

----------------------------------------------------------------
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

Reply via email to