dineshchitlangia commented on a change in pull request #2247:
URL: https://github.com/apache/ozone/pull/2247#discussion_r633219987



##########
File path: 
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMContainerLocationFailoverProxyProvider.java
##########
@@ -63,8 +63,15 @@
   private final Map<String, SCMProxyInfo> scmProxyInfoMap;
   private List<String> scmNodeIds;
 
-  private String currentProxySCMNodeId;
-  private int currentProxyIndex;
+  // As when SCM Client is shared across threads, performFailOver method
+  // updates the currentProxySCMNodeId based on updateLeaderNodeId which is
+  // updated in shouldRetry. So, when 2 threads run parallel for
+  // one of the thread performFailOver is not called which is taken care by
+  // RetryInvocationHandler by checking expectedFailOverCount. So other thread
+  // shall not call performFailOver it will call getProxy() where we use
+  // currentProxySCMNodeId and return proxy.

Review comment:
       ```suggestion
     // As SCM Client is shared across threads, performFailOver()
     // updates the currentProxySCMNodeId based on the updateLeaderNodeId which 
is
     // updated in shouldRetry(). When 2 or more threads run in parallel, the
     // RetryInvocationHandler will check the expectedFailOverCount
     // and not execute performFailOver() for one of them. So the other 
thread(s)
     // shall not call performFailOver(), it will call getProxy() which uses
     // currentProxySCMNodeId and returns the proxy.
   ```

##########
File path: 
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java
##########
@@ -61,21 +61,28 @@
   private Map<String, SCMProxyInfo> scmProxyInfoMap;
   private List<String> scmNodeIds;
 
-  private String currentProxySCMNodeId;
-  private int currentProxyIndex;
+  // As when SCM Client is shared across threads, performFailOver method
+  // updates the currentProxySCMNodeId based on the updateLeaderNodeId which is
+  // updated in shouldRetry. So, when 2 threads run parallel for
+  // one of the thread performFailOver is not called which is taken care by
+  // RetryInvocationHandler by checking expectedFailOverCount. So other thread
+  // shall not call performFailOver it will call getProxy() where we use
+  // currentProxySCMNodeId and return proxy.

Review comment:
       ```suggestion
     // As SCM Client is shared across threads, performFailOver()
     // updates the currentProxySCMNodeId based on the updateLeaderNodeId which 
is
     // updated in shouldRetry(). When 2 or more threads run in parallel, the
     // RetryInvocationHandler will check the expectedFailOverCount
     // and not execute performFailOver() for one of them. So the other 
thread(s)
     // shall not call performFailOver(), it will call getProxy() which uses
     // currentProxySCMNodeId and returns the proxy.
   ```

##########
File path: 
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java
##########
@@ -64,8 +64,16 @@
 
   private List<String> scmNodeIds;
 
-  private String currentProxySCMNodeId;
-  private int currentProxyIndex;
+  // As when SCM Client is shared across threads, performFailOver method
+  // updates the currentProxySCMNodeId based on updateLeaderNodeId which is
+  // updated in shouldRetry. So, when 2 threads run parallel for
+  // one of the thread performFailOver is not called which is taken care by
+  // RetryInvocationHandler by checking expectedFailOverCount. So other thread
+  // shall not call performFailOver it will call getProxy() where we use
+  // currentProxySCMNodeId and return proxy.

Review comment:
       ```suggestion
     // As SCM Client is shared across threads, performFailOver()
     // updates the currentProxySCMNodeId based on the updateLeaderNodeId which 
is
     // updated in shouldRetry(). When 2 or more threads run in parallel, the
     // RetryInvocationHandler will check the expectedFailOverCount
     // and not execute performFailOver() for one of them. So the other 
thread(s)
     // shall not call performFailOver(), it will call getProxy() which uses
     // currentProxySCMNodeId and returns the proxy.
   ```




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



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

Reply via email to