[GitHub] [flink] zhijiangW edited a comment on issue #8362: [FLINK-11391] Introduce shuffle master interface

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

zhijiangW edited a comment on issue #8362: [FLINK-11391] Introduce shuffle master interface
URL: https://github.com/apache/flink/pull/8362#issuecomment-492491082
 
 
   Thanks for the confirmation @azagrebin .
   
   1. Yes, I have not thought through the changes caused by single channel in `PartitionInfo` and all channels in `IGDD`. Just from the aspect of rpc call `taskManagerGateway.updatePartitions(partitionInfos)`, the parameter is a collection of `PartitionInfo` which is the same as array of channels in `IGDD`. Maybe the `IGDD` should support cache `ICDD` internally and replace the array with collection. It might involve in more refactoring and I would also further consider it.
   
   2. From functional aspect the current way is no problem. But I was ever suggested in my PR not using `instanceof` via introducing the interface method `ChannelSelector#isBroadcast`. Because `instanceof` sounds like a hacky, not a proper solution. I am not sure whether it is not suggested in common sense atm,  or maybe it is just a personal preference.  I think you could confirm this way with other guys. :)
   
   BTW, I have not finished the whole review yet. I would continue on it later.

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