azagrebin commented on a change in pull request #7785: [FLINK-11707][network] 
Make InputGate extend AutoCloseable
URL: https://github.com/apache/flink/pull/7785#discussion_r268091358
 
 

 ##########
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/partition/consumer/UnionInputGate.java
 ##########
 @@ -264,6 +265,23 @@ public int getPageSize() {
                return pageSize;
        }
 
+       @Override
+       public void close() throws IOException {
 
 Review comment:
   UnionInputGate looks like something core Flink specific, some artificial 
abstraction, not related to ShuffleService.
   At the moment, it is virtual and does not have real resources behind.
   I would not even call it InputGate from shuffle service perspective, it is 
rather a partial decorator.
   The real gates are managed by Task, one level down of abstraction.
   Although, this close implementation looks logical, currently, it is not used.
   My concern is that if we leave it, it might confuse in future: somebody 
might start calling it and break the lifecycle of real gates managed only by 
Task atm. As it is just a refactoring, I would leave this close method empty 
then, because it looks like it is not supposed to be used anyways.
   or you think, there is another reason to have it like this?

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