You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/05/17 04:54:00 UTC
[GitHub] [ozone] dineshchitlangia commented on a change in pull request #2247: Hdds 5216: Fix race condition causing SCM failOverProxy which is causing failover wrongly.
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org