symious commented on a change in pull request #3658:
URL: https://github.com/apache/hadoop/pull/3658#discussion_r750944899



##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java
##########
@@ -1679,11 +1679,13 @@ private Connection getConnection(ConnectionId remoteId,
     private final boolean doPing; //do we need to send ping message
     private final int pingInterval; // how often sends ping to the server in 
msecs
     private String saslQop; // here for testing
+    private final AtomicBoolean fallbackToSimpleAuth;
     private final Configuration conf; // used to get the expected kerberos 
principal name
     
     ConnectionId(InetSocketAddress address, Class<?> protocol, 
                  UserGroupInformation ticket, int rpcTimeout,
-                 RetryPolicy connectionRetryPolicy, Configuration conf) {
+                 RetryPolicy connectionRetryPolicy, Configuration conf,
+                 AtomicBoolean fallbackToSimpleAuth) {

Review comment:
       By default, connections from the same server with same configurations 
will reuse the connection tagged with the same ConnectionId. Say Client1, to 
Client10, these 10 clients will reuse the same connection.
   
   If we add a field of AtomicBoolean in the comparison of ConnectionId, 
Client1 to Client10 will each initial a new AtomicBoolean, since the result of 
Object.equals are always false, the Client1 to Client10 will have 10 
connections. That's connection leak.
   If we change AtomicBoolean to boolean, there will at most be two different 
connections, since "false" is the default value, normally we will only have 
have one connection, only when we have a "true" value connection will the 
second connection be created.
   
   I assume the problem we initially trying to solve is that clients with 
"fallback"  set to true will be omitted. When we check with "boolean" instead 
of "AtomicBoolean", normal clients will go to the connection with "boolean" set 
to "false", special clients will go to the connection with "boolean" set to 
"true".  
   IMO, two connections are not leak, since idle connection will be collected 
and closed.




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to