nandakumar131 commented on code in PR #6752:
URL: https://github.com/apache/ozone/pull/6752#discussion_r1623759639


##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/StorageContainerServiceProviderImpl.java:
##########
@@ -179,58 +185,80 @@ public DBCheckpoint getSCMDBSnapshot() {
         System.currentTimeMillis();
     File targetFile = new File(scmSnapshotDBParentDir, snapshotFileName +
             ".tar");
-
     try {
       if (!SCMHAUtils.isSCMHAEnabled(configuration)) {
-        SecurityUtil.doAsLoginUser(() -> {
-          try (InputStream inputStream = reconUtils.makeHttpCall(
-              connectionFactory, getScmDBSnapshotUrl(),
-              isOmSpnegoEnabled()).getInputStream()) {
-            FileUtils.copyInputStreamToFile(inputStream, targetFile);
-          }
-          return null;
-        });
+        fetchSCMDBSnapshotUsingHttpClient(targetFile);
         LOG.info("Downloaded SCM Snapshot from SCM");
       } else {
-        List<String> ratisRoles = scmClient.getScmInfo().getRatisPeerRoles();
-        for (String ratisRole: ratisRoles) {
-          String[] role = ratisRole.split(":");
-          if (role[2].equals(RaftProtos.RaftPeerRole.LEADER.toString())) {
-            String hostAddress = role[4].trim();
-            int grpcPort = configuration.getInt(
-                ScmConfigKeys.OZONE_SCM_GRPC_PORT_KEY,
-                ScmConfigKeys.OZONE_SCM_GRPC_PORT_DEFAULT);
-
-            SecurityConfig secConf = new SecurityConfig(configuration);
-            SCMSecurityProtocolClientSideTranslatorPB scmSecurityClient =
-                getScmSecurityClientWithMaxRetry(
-                    configuration, getCurrentUser());
-            try (ReconCertificateClient certClient =
-                     new ReconCertificateClient(
-                         secConf, scmSecurityClient, reconStorage, null, null);
-                 SCMSnapshotDownloader downloadClient = new InterSCMGrpcClient(
-                     hostAddress, grpcPort, configuration, certClient)) {
-              downloadClient.download(targetFile.toPath()).get();
-            } catch (ExecutionException | InterruptedException e) {
-              LOG.error("Rocks DB checkpoint downloading failed", e);
-              throw new IOException(e);
+        try {
+          List<String> ratisRoles = scmClient.getScmInfo().getRatisPeerRoles();
+          for (String ratisRole : ratisRoles) {
+            String[] role = ratisRole.split(":");
+            // This explicit role length check is to support older versions 
where we cannot change the default value
+            // without breaking backward compatibility during upgrade, because 
if Ratis is not enabled then the roles
+            // command output is generated outside of Ratis. It will not have 
the Ratis terminologies.
+            if (role.length <= 2) {
+              fetchSCMDBSnapshotUsingHttpClient(targetFile);

Review Comment:
   @sumitagrawl  @devmadhuu 
   The goal is to have Ratis enabled for all the cases whether it's HA or 
Non-HA.
   
   Going forward, we should deprecate  and remove the property 
`ozone.scm.ratis.enable` and use Ratis by default.
   The reason for introducing the property in first place is to support 
upgrading of old clusters and be backward compatible.
   
   We will have this property until we support the old Ozone releases, we can 
deprecate the property after that.
   Even now, we cannot disable the property once it's enabled.



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