zhijiangW edited a comment on issue #9132: [FLINK-13245][network] Fix the bug 
of file resource leak while canceling partition request
URL: https://github.com/apache/flink/pull/9132#issuecomment-515394950
 
 
   @StephanEwen  thanks for helping address above comments and some further 
refactoring.
   
   I have only one concern for the issue of `notifySubpartitionConsumed`. I 
agree that it would make the logic simple on server side to not distinguish 
during release resources, and it actually makes sense to do so now. But this 
distinguish might have valuable effects in future. E.g. in the case of any 
local network exceptions on server side, if the server releases the reader/view 
together with the subpartition/partition, and then notify the client side about 
the `ErrorResponse`. That means there might be no chances for creating views 
for partition any more because it has already been removed from 
`ResultPartitionManager`. In current way, only reader/view is released and the 
client still have the opportunity to decide whether to request partitions again 
after receiving `ErrorResponse` in future.
   
   The key point is whether trigger calling `notifySubpartitionConsumed` via 
client's specific confirmation or server could do that in any ways. The 
previous assumption is that it should be done in client's specific confirmation.
   
   I would pick the relevant commits from @StephanEwen 's branch except for 
whether always calling `notifySubpartitionConsumed` during release.

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