[GitHub] zhijiangW commented on a change in pull request #7704: [FLINK-11604][network] Extend the necessary methods in ResultPartitionWriter interface

[GitBox <gi...@apache.org>]

zhijiangW commented on a change in pull request #7704: [FLINK-11604][network] Extend the necessary methods in ResultPartitionWriter interface
URL: https://github.com/apache/flink/pull/7704#discussion_r257590168
 
 

 ##########
 File path: flink-runtime/src/main/java/org/apache/flink/runtime/io/network/api/writer/ResultPartitionWriter.java
 ##########
 @@ -31,12 +37,26 @@
 
 	BufferProvider getBufferProvider();
 
+	BufferPool getBufferPool();
 
 Review comment:
   In general I agree with the above points of retaining only most basic methods in `ResultPartitionWriter` such as `addBufferConsumer`, `flush`, `finish`, etc, and remove other methods by refactoring existing processes or belonging to special `ShuffleService` implementations.
   
   The initial motivation of this PR is to prepare the work for #7549 in order to make `ResultPartitionWriterWrapper` contain `ResultPartitionWriter` instead of specific `ResultPartition`.  Now I realize it is not the right way to make such temporary interface like this and refactor again after introducing the `ShuffleService` finally.
   
   I reviewed the #7549 again and found another way to bring `ResultPartitionWriterWrapper` separately, not changing the existing methods in `ResultPartitionWriter`.  
   Then the whole refactor work of `ResultPartitionWriter` seems more proper to be done after adding the `ShuffleService`, or we might refactor the interface step by step to narrow down the issues each time, such as make the `RecordWriter` not to call `ResultPartitionWriter#getBufferProvider` as a separate step, etc. What do you think?
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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