-
Notifications
You must be signed in to change notification settings - Fork 587
HDDS-5051. Ensure failover to suggested leader if any for NotLeaderException. #2141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
bharatviswa504
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still reviewing.
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/ServerNotLeaderException.java
Outdated
Show resolved
Hide resolved
...va/org/apache/hadoop/hdds/scm/protocolPB/ScmBlockLocationProtocolClientSideTranslatorPB.java
Outdated
Show resolved
Hide resolved
...rk/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java
Outdated
Show resolved
Hide resolved
...rk/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/apache/hadoop/hdds/scm/proxy/SCMContainerLocationFailoverProxyProvider.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAUtils.java
Outdated
Show resolved
Hide resolved
bharatviswa504
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I think if we can have more mock tests for check with RetriableExceptionsWithNoFailOver and NonRetriableException and normal exceptions that would be good. This will give better test coverage and uncover any misses.
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMClientConfig.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HddsServerUtil.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAUtils.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAUtils.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/RatisUtil.java
Outdated
Show resolved
Hide resolved
|
https://issues.apache.org/jira/browse/HDDS-5130 is open to track the test coverage issue. |
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAUtils.java
Outdated
Show resolved
Hide resolved
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAUtils.java
Outdated
Show resolved
Hide resolved
...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
Outdated
Show resolved
Hide resolved
...hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
Outdated
Show resolved
Hide resolved
...rk/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMBlockLocationFailoverProxyProvider.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/apache/hadoop/hdds/scm/proxy/SCMContainerLocationFailoverProxyProvider.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/apache/hadoop/hdds/scm/proxy/SCMContainerLocationFailoverProxyProvider.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java
Outdated
Show resolved
Hide resolved
...src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMSecurityProtocolFailoverProxyProvider.java
Show resolved
Hide resolved
bharatviswa504
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM.
One minor comment.
bharatviswa504
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 (Pending CI)
hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/proxy/SCMClientConfig.java
Outdated
Show resolved
Hide resolved
…ing-upgrade-master-merge2 * upstream/master: (56 commits) HDDS-2212. Genconf tool should generate config files for secure clust… (apache#1788) HDDS-5166. Remove duplicate assignment of OZONE_OPTS for freon and sh (apache#2195) Revert "HDDS-5144. Create github check to alert when dependency tree is changed (apache#2177)" HDDS-4983. Display key offset for each block in command key info (apache#2051) HDDS-5144. Create github check to alert when dependency tree is changed (apache#2177) HDDS-4585. Support bucket acl operation in S3g (apache#1701) HDDS-5153. Decommissioning a dead node should complete immediately (apache#2190) HDDS-5147. Intermittent test failure in TestContainerDeletionChoosingPolicy#testRandomChoosingPolicy (apache#2188) HDDS-5152. Fix Suggested leader in Client. (apache#2189) HDDS-5148. Bump ratis version to 2.1.0-ff8aa66-SNAPSHOT (apache#2184) HDDS-4515. Datanodes should be able to persist and load CRL (apache#2181) HDDS-5060. [SCM HA Security] Make InterSCM grpc channel secure. (apache#2187) HDDS-5051. Ensure failover to suggested leader if any for NotLeaderException. (apache#2141) HDDS-5127. Fix getServiceList when SCM HA is enabled (apache#2173) HDDS-4889. Add simple CI check for docs (apache#2156) HDDS-5131. Use timeout in github actions (apache#2176) HDDS-5103. Fix Install Snapshot Mechanism in SCMStateMachine. (apache#2155) HDDS-5124. Use OzoneConsts.OZONE_TIME_ZONE instead of "GMT" (apache#2166) HDDS-5047. Refactor Pipeline to use ReplicationConfig instead of factor/type (apache#2096) HDDS-5083. Bump version of common-compress (apache#2139) ... Conflicts: hadoop-hdds/common/pom.xml hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManager.java hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMStorageConfig.java hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OMStorage.java hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/scm/ReconStorageContainerManagerFacade.java
…k-in-auth * HDDS-3698-nonrolling-upgrade: (57 commits) Fix compilation errors afte merge Update javassist in recon pom Fix changes introduced in merge that failed TestSCMNodeManager upgrade tests Fix checkstyle Fix intermittent test failure TestSCMNodeManager#testSetNodeOpStateAndCommandFired after merge Skip scm init default layout version in TestOzoneConfigurationFields HDDS-2212. Genconf tool should generate config files for secure clust… (apache#1788) HDDS-5166. Remove duplicate assignment of OZONE_OPTS for freon and sh (apache#2195) Revert "HDDS-5144. Create github check to alert when dependency tree is changed (apache#2177)" HDDS-4983. Display key offset for each block in command key info (apache#2051) HDDS-5144. Create github check to alert when dependency tree is changed (apache#2177) HDDS-4585. Support bucket acl operation in S3g (apache#1701) HDDS-5153. Decommissioning a dead node should complete immediately (apache#2190) HDDS-5147. Intermittent test failure in TestContainerDeletionChoosingPolicy#testRandomChoosingPolicy (apache#2188) HDDS-5152. Fix Suggested leader in Client. (apache#2189) HDDS-5148. Bump ratis version to 2.1.0-ff8aa66-SNAPSHOT (apache#2184) HDDS-4515. Datanodes should be able to persist and load CRL (apache#2181) HDDS-5060. [SCM HA Security] Make InterSCM grpc channel secure. (apache#2187) HDDS-5051. Ensure failover to suggested leader if any for NotLeaderException. (apache#2141) HDDS-5127. Fix getServiceList when SCM HA is enabled (apache#2173) HDDS-4889. Add simple CI check for docs (apache#2156) HDDS-5131. Use timeout in github actions (apache#2176) HDDS-5103. Fix Install Snapshot Mechanism in SCMStateMachine. (apache#2155) HDDS-5124. Use OzoneConsts.OZONE_TIME_ZONE instead of "GMT" (apache#2166) HDDS-5047. Refactor Pipeline to use ReplicationConfig instead of factor/type (apache#2096) ...
What changes were proposed in this pull request?
Handling of suggested leader for SCM client requests
What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-5051
How was this patch tested?
Added UT