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