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_r262787909
##########
File path:
flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/writer/RecordWriterTest.java
##########
@@ -461,6 +572,56 @@ private void
emitRecordWithBroadcastPartitionerOrBroadcastEmitRecord(boolean isB
}
}
+ private void randomEmitMixedBroadcastBufferOrEvent(boolean
isBroadcastEvent) throws Exception {
Review comment:
My motivation of adding this test is for verifying the logic after switching
modes between broadcast and random. That is we have to finish the current
`BufferBuilder` once switching modes, and this logic can be reflected on the
number of buffers in all the partition queues.
I could understand your concern of best-effort ignoring the previous tests
failure after refactoring something, and I agree with this rule. If we change
this new test into data correctness like we already have in
`RecordWriterTest#emitOrBroadcastRecord`, it might be difficult to verify how
many `LatencyMarker` are emitted actually because this marker would not affect
data correctness sometimes. I mean comparing `broadcastEmit` maker with
`randomEmit` marker, the results might be same in aspect of data correctness,
but the behavior is different. I would have a try to re-write this test by
both verifying data correctness and the number of markers in channel if you
think this way make sense.
----------------------------------------------------------------
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
