flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [flink] zhijiangW commented on a change in pull request #7713: [FLINK-10995][network] Copy intermediate serialization results only once for broadcast mode
Date Fri, 15 Mar 2019 14:09:24 GMT
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_r265996143
 
 

 ##########
 File path: flink-runtime/src/test/java/org/apache/flink/runtime/io/network/api/writer/BroadcastRecordWriterTest.java
 ##########
 @@ -21,30 +21,30 @@
 import org.apache.flink.core.io.IOReadableWritable;
 
 Review comment:
   Not really, the new added test in `BroadcastRecordWriterTest` for 2nd commit actually tests
the function for `BroadcastRecordWriter`. In this commit, the `BroadcastRecordWriterTest`
also extends the `RecordWriterTest`, so it would run all the existing tests in `RecordWriterTest`,
but the created writer is still the `SelectorRecordWriter`. 
   
   In short words, `BroadcastRecordWriterTest` extends all the existing tests in `RecordWriterTest`
and run them in the way of `SelectorRecordWriter`. So in the 3rd commit, I introduced the
`isBroadcastWriter` in the constructor for creating the specific `RecordWriter` instance based
on this tag. Then all the existing tests would run for `BroadcastRecordWriterTest` in the
way of `BroadcastRecordWriter`.
   
   Anyway, I think it still makes sense to squash these two commits. The separation is just
for distinguishing the new added test and refactoring all the old tests.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message