KarmaGYZ commented on a change in pull request #18417:
URL: https://github.com/apache/flink/pull/18417#discussion_r793379797



##########
File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/netty/PartitionRequestClientFactory.java
##########
@@ -75,6 +78,11 @@
      */
     NettyPartitionRequestClient createPartitionRequestClient(ConnectionID 
connectionId)
             throws IOException, InterruptedException {
+        // Proactively close idle connections with error.
+        if (connectionReuseEnabled) {
+            closeErrorChannelConnections();
+        }

Review comment:
       With connection reused, we cannot find a better place to close error 
connections. We need to refactor the process of creating 
`PartitionRequestClient` or maybe rename the `PartitionRequestClientFactory` to 
`PartitionRequestClientManager`. But I think that is out of the scope of this 
PR.
   
   > How should the safe net be triggered? Having to iterate over all exist 
connections whenever creates a new connection does not sounds like a good idea. 
Maybe this can be triggered when the new connection cannot be created due to 
the max connection limit, or somehow asynchronously?
   
   Yes, we can only check whether the required connection is normal.




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

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to