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



##########
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:
       Let me quote myself here:
   >Note that the AtomicBoolean value is not there to determine if we are 
allowed to fall back, that comes from configuration via a variable 
fallbackAllowed into Connection#setupIOStreams. This atomicBoolean determines 
during the real communication which type of authentication will be used, and we 
do only know if we really have to fall back to simple auth after we connected 
to the server address and discovered its expected authentication method.
   
   Please see [this 
code](https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java#L870),
 the falbackAllowed boolean is what you are talking about. It is initialized 
based on config 
[here](https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java#L1338).
   
   The AtomicBoolean is initialized in the [Connection's setupIOStreams 
method](https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java#L808)
 which is called from [the getConnection 
method](https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java#L1614),
 which also caches the connection based on ConnectionID 
[here](https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java#L1637).
   
   The fallbackToSimpleAuth AtomicBoolean is created [in the DfsClient 
here](https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java#L1637),
 and is shared with the SaslDataTransferClient 
[here](https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Client.java#L1637),
 while it is provided to the ipc Client via the protocol proxy between this two 
points.
   
   Later on the SaslDataTransferClient uses the AtomicBoolean to decide wether 
to fallback or not [in this 
method](https://github.com/apache/hadoop/blob/trunk/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/protocol/datatransfer/sasl/SaslDataTransferClient.java#L279).
   
   
   So again:
   the config we can not use, as even if the fallback is allowed we do not want 
to fallback against a secured server, and we do not know if the server is 
secure or unsecure until we are connected to it.
   The AtomicBoolean is initialized as false, and set to be true only after the 
Connection has been set up and cached based on the ConnectionID, so we can not 
rely on its boolean value.
   
   
   So the question still stands, do you still think we should go down this 
route, and tinker with the ConnectionID, instead of setting the AtomicBoolean's 
value in setupIOStreams if it should be true?




-- 
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: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to