You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "devmadhuu (via GitHub)" <gi...@apache.org> on 2024/05/30 14:54:22 UTC
[PR] HDDS-10937. Ozone Recon - Handle startup failure and log reasons as error due to SCM non-HA scenario [ozone]
devmadhuu opened a new pull request, #6752:
URL: https://github.com/apache/ozone/pull/6752
## What changes were proposed in this pull request?
This PR handles Recon startup failure due to unexpected runtime error and shuts down silently without logging the actual cause of error.
Ozone Recon - Handle startup failure and log reasons as error due to SCM non-HA scenario
While ReconServer.start() method is called, if any runtime errors being thrown, those are not being logged and Recon startup fails silently without logging any reason of failure. E.g. In case of non-HA SCM case, few configurations made Recon fails to start. This scenario or any runtime exception needs to be handled for proper logging.
In non-HA SCM case, while trying to fetch SCM DB snapshot, SCM peer roles are not being returned correctly, so the logic needs to handle.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-10937
## How was this patch tested?
Tested manually and added additional integration tests in Recon for non HA SCM case.
--
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: issues-unsubscribe@ozone.apache.org
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
Re: [PR] HDDS-10937. Ozone Recon - Handle startup failure and log reasons as error due to SCM non-HA scenario [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on code in PR #6752:
URL: https://github.com/apache/ozone/pull/6752#discussion_r1622531407
##########
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:
As discussed, created a separate JIRA [HDDS-10957](https://issues.apache.org/jira/browse/HDDS-10957) to track.
--
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: issues-unsubscribe@ozone.apache.org
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
Re: [PR] HDDS-10937. Ozone Recon - Handle startup failure and log reasons as error due to SCM non-HA scenario [ozone]
Posted by "devmadhuu (via GitHub)" <gi...@apache.org>.
devmadhuu commented on PR #6752:
URL: https://github.com/apache/ozone/pull/6752#issuecomment-2139802792
@nandakumar131 @sumitagrawl kindly review.
--
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: issues-unsubscribe@ozone.apache.org
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
Re: [PR] HDDS-10937. Ozone Recon - Handle startup failure and log reasons as error due to SCM non-HA scenario [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #6752:
URL: https://github.com/apache/ozone/pull/6752#discussion_r1622314814
##########
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 {
Review Comment:
@nandakumar131
Recon is derived from SCM, So many code functionality is implicitly available in recon from scm code. We may need fix Recon before startup to know if SCM is in HA or not for others also.
Also, this is used in freon , datanode, and cli tool via HAUtils for getting CA certificates. This can impact them as per code.
This needs different solution, getting rid of this from all places.
--
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: issues-unsubscribe@ozone.apache.org
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
Re: [PR] HDDS-10937. Ozone Recon - Handle startup failure and log reasons as error due to SCM non-HA scenario [ozone]
Posted by "nandakumar131 (via GitHub)" <gi...@apache.org>.
nandakumar131 commented on PR #6752:
URL: https://github.com/apache/ozone/pull/6752#issuecomment-2146922714
Thanks @devmadhuu for the fix and thanks to @sumitagrawl for the review!
--
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: issues-unsubscribe@ozone.apache.org
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
Re: [PR] HDDS-10937. Ozone Recon - Handle startup failure and log reasons as error due to SCM non-HA scenario [ozone]
Posted by "nandakumar131 (via GitHub)" <gi...@apache.org>.
nandakumar131 merged PR #6752:
URL: https://github.com/apache/ozone/pull/6752
--
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: issues-unsubscribe@ozone.apache.org
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
Re: [PR] HDDS-10937. Ozone Recon - Handle startup failure and log reasons as error due to SCM non-HA scenario [ozone]
Posted by "nandakumar131 (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@ozone.apache.org
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
Re: [PR] HDDS-10937. Ozone Recon - Handle startup failure and log reasons as error due to SCM non-HA scenario [ozone]
Posted by "sumitagrawl (via GitHub)" <gi...@apache.org>.
sumitagrawl commented on code in PR #6752:
URL: https://github.com/apache/ozone/pull/6752#discussion_r1622318447
##########
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:
When ratis is enabled for single node SCM, it will have roles and same can be used. Only problem is old SCM where ratis is not enabled (identified dynamically overridding this property).
We need consider the impact for Recon all places as its using same SCM code, but this property is not overloaded. We may face the problem whenever the funtionality hits. Need find better solution.
--
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: issues-unsubscribe@ozone.apache.org
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
Re: [PR] HDDS-10937. Ozone Recon - Handle startup failure and log reasons as error due to SCM non-HA scenario [ozone]
Posted by "nandakumar131 (via GitHub)" <gi...@apache.org>.
nandakumar131 commented on code in PR #6752:
URL: https://github.com/apache/ozone/pull/6752#discussion_r1621914697
##########
hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/ReconServer.java:
##########
@@ -153,13 +156,21 @@ public Void call() throws Exception {
LOG.info("Initializing support of Recon Features...");
FeatureProvider.initFeatureSupport(configuration);
+
+ LOG.debug("Now starting all services of Recon...");
+ // Start all services
+ start();
+ isStarted = true;
+ LOG.debug("Start of all services of Recon completed successfully !!!");
Review Comment:
NIT: Do we need this log statement?
##########
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 {
Review Comment:
We can get rid of the `isSCMHAEnabled` check and completely rely on the output of `getScmInfo` output.
##########
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:
Instead of `role.length` check, we can do `ratisRoles.size` and enter the `for` loop only if the `size` is greater than 1.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/recon/TestReconScmNonHASnapshot.java:
##########
@@ -0,0 +1,65 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.ozone.recon;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.scm.ScmConfigKeys;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+import static org.apache.hadoop.hdds.scm.ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY;
+
+/**
+ * Test Recon SCM HA Snapshot Download implementation.
+ */
+@Timeout(300)
+public class TestReconScmNonHASnapshot {
+ private OzoneConfiguration conf;
+ private MiniOzoneCluster ozoneCluster = null;
+
+ @BeforeEach
+ public void setup() throws Exception {
+ conf = new OzoneConfiguration();
+ conf.setBoolean(OZONE_SCM_HA_ENABLE_KEY, false);
+ conf.setBoolean(
+ ReconServerConfigKeys.OZONE_RECON_SCM_SNAPSHOT_ENABLED, true);
+ conf.setInt(ReconServerConfigKeys.OZONE_RECON_SCM_CONTAINER_THRESHOLD, 0);
+ conf.setInt(ScmConfigKeys.OZONE_DATANODE_PIPELINE_LIMIT, 5);
+ ozoneCluster = MiniOzoneCluster.newBuilder(conf)
+ .setNumDatanodes(4)
+ .includeRecon(true)
+ .build();
+ ozoneCluster.waitForClusterToBeReady();
+ }
+
+ @Test
+ public void testScmNonHASnapshot() throws Exception {
+ //ozoneCluster.getReconServer().getStorageContainerServiceProvider().
Review Comment:
NIT: This line can be removed.
--
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: issues-unsubscribe@ozone.apache.org
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
Re: [PR] HDDS-10937. Ozone Recon - Handle startup failure and log reasons as error due to SCM non-HA scenario [ozone]
Posted by "nandakumar131 (via GitHub)" <gi...@apache.org>.
nandakumar131 commented on code in PR #6752:
URL: https://github.com/apache/ozone/pull/6752#discussion_r1623757014
##########
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 {
Review Comment:
@sumitagrawl I din't mean to remove `isSCMHAEnabled` the method completely.
We don't need to call the `isSCMHAEnabled` check here in this case.
--
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: issues-unsubscribe@ozone.apache.org
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