You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "sodonnel (via GitHub)" <gi...@apache.org> on 2026/05/11 11:54:11 UTC
[PR] HDDS-15208. OM should learn to finalize from SCM [ozone]
sodonnel opened a new pull request, #10236:
URL: https://github.com/apache/ozone/pull/10236
## What changes were proposed in this pull request?
When OM is started, it checks if finalization is needed. If so, it spawns a background service which polls SCM to see if it is time to finalize or not. If SCM indicates it should finalize, then it will finalize the OMs via Ratis and then shutdown the background service to avoid any further polling.
The poll interval defaults to 60 seconds, but its configurable via ozone.om.upgrade.finalization.check.interval.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15208
## How was this patch tested?
Existing integration tests were modified to remove the calls to the old OM finalize command, and instead trigger finalize via SCM. New unit tests have been added for the new polling service.
--
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-15208. OM should learn to finalize from SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #10236:
URL: https://github.com/apache/ozone/pull/10236#discussion_r3225483405
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizeService.java:
##########
@@ -0,0 +1,127 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.om.upgrade;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.BackgroundService;
+import org.apache.hadoop.hdds.utils.BackgroundTask;
+import org.apache.hadoop.hdds.utils.BackgroundTaskQueue;
+import org.apache.hadoop.hdds.utils.BackgroundTaskResult;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.ScmClient;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils;
+import org.apache.hadoop.ozone.om.request.OMClientRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.ratis.protocol.ClientId;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A background service that periodically checks whether SCM has completed finalization of an upgrade and, if so,
+ * finalizes the OM upgrade.
+ */
+public class OMUpgradeFinalizeService extends BackgroundService {
+
+ private static final Logger LOG = LoggerFactory.getLogger(OMUpgradeFinalizeService.class);
+
+ private static final int THREAD_POOL_SIZE = 1;
+ private static final TimeUnit INTERVAL_UNIT = TimeUnit.MILLISECONDS;
+ private static final long TIMEOUT = 60000;
+ private static final AtomicLong RUN_COUNT = new AtomicLong(0);
+
+ private final OzoneManager ozoneManager;
+ private final OMVersionManager versionManager;
+ private final ScmClient scmClient;
+ private final AtomicBoolean stopInitiated = new AtomicBoolean(false);
+
+ /**
+ * Creates an {@code OMUpgradeFinalizeService} with a custom check interval.
+ * Primarily intended for testing.
Review Comment:
I've removed that.
--
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-15208. OM should learn to finalize from SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #10236:
URL: https://github.com/apache/ozone/pull/10236#discussion_r3225476371
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMUpgradeFinalization.java:
##########
@@ -93,12 +99,10 @@ void testOMUpgradeFinalizationWithOneOMDown() throws Exception {
long prepareIndex = omClient.prepareOzoneManager(120L, 5L);
assertClusterPrepared(prepareIndex, runningOms);
AuditLogTestUtils.verifyAuditLog(OMAction.UPGRADE_PREPARE, AuditEventStatus.SUCCESS);
-
omClient.cancelOzoneManagerPrepare();
AuditLogTestUtils.verifyAuditLog(OMAction.UPGRADE_CANCEL, AuditEventStatus.SUCCESS);
- StatusAndMessages response =
- omClient.finalizeUpgrade("finalize-test");
- System.out.println("Finalization Messages : " + response.msgs());
+ // Send the finalize command to SCM which triggers the OM finalize when SCM reports it is complete.
+ cluster.getStorageContainerLocationClient().finalizeUpgrade();
Review Comment:
I guess this work (now and in preexisting tests) because the audit log check retries until it finds what it needs.
I didn't know there was a system audit log. I guess its still kind of triggered by the use, as someone has to send a finalize command which then propagates through the system.
--
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-15208. OM should learn to finalize from SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #10236:
URL: https://github.com/apache/ozone/pull/10236#discussion_r3225476371
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMUpgradeFinalization.java:
##########
@@ -93,12 +99,10 @@ void testOMUpgradeFinalizationWithOneOMDown() throws Exception {
long prepareIndex = omClient.prepareOzoneManager(120L, 5L);
assertClusterPrepared(prepareIndex, runningOms);
AuditLogTestUtils.verifyAuditLog(OMAction.UPGRADE_PREPARE, AuditEventStatus.SUCCESS);
-
omClient.cancelOzoneManagerPrepare();
AuditLogTestUtils.verifyAuditLog(OMAction.UPGRADE_CANCEL, AuditEventStatus.SUCCESS);
- StatusAndMessages response =
- omClient.finalizeUpgrade("finalize-test");
- System.out.println("Finalization Messages : " + response.msgs());
+ // Send the finalize command to SCM which triggers the OM finalize when SCM reports it is complete.
+ cluster.getStorageContainerLocationClient().finalizeUpgrade();
Review Comment:
I guess this work (now and in preexisting tests) because the audit log check retries until it finds what it needs. I swapped the lines around.
I didn't know there was a system audit log. I guess its still kind of triggered by the use, as someone has to send a finalize command which then propagates through the system.
--
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-15208. OM should learn to finalize from SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #10236:
URL: https://github.com/apache/ozone/pull/10236#discussion_r3317284425
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -1900,6 +1908,10 @@ public void start() throws IOException {
bootstrap(omNodeDetails);
}
+ if (omUpgradeFinalizeService != null) {
Review Comment:
No, this is not temporary. The service is created in the code like this only if finalization is needed:
```
if (versionManager.needsFinalization()) {
long intervalMs = conf.getTimeDuration(OMConfigKeys.OZONE_OM_UPGRADE_FINALIZATION_CHECK_INTERVAL,
OMConfigKeys.OZONE_OM_UPGRADE_FINALIZATION_CHECK_INTERVAL_DEFAULT, TimeUnit.MILLISECONDS);
omUpgradeFinalizeService = new OMUpgradeFinalizeService(this, versionManager, getScmClient(), intervalMs);
}
```
And then it is started. It will check periodically if the should finalize key has been set which will be set by the upcoming client command PR. Once finalize is completed the service shuts itself down.
--
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-15208. OM should learn to finalize from SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #10236:
URL: https://github.com/apache/ozone/pull/10236#discussion_r3225545584
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizeService.java:
##########
@@ -0,0 +1,127 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.om.upgrade;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.BackgroundService;
+import org.apache.hadoop.hdds.utils.BackgroundTask;
+import org.apache.hadoop.hdds.utils.BackgroundTaskQueue;
+import org.apache.hadoop.hdds.utils.BackgroundTaskResult;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.ScmClient;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils;
+import org.apache.hadoop.ozone.om.request.OMClientRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.ratis.protocol.ClientId;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A background service that periodically checks whether SCM has completed finalization of an upgrade and, if so,
+ * finalizes the OM upgrade.
+ */
+public class OMUpgradeFinalizeService extends BackgroundService {
+
+ private static final Logger LOG = LoggerFactory.getLogger(OMUpgradeFinalizeService.class);
+
+ private static final int THREAD_POOL_SIZE = 1;
+ private static final TimeUnit INTERVAL_UNIT = TimeUnit.MILLISECONDS;
+ private static final long TIMEOUT = 60000;
+ private static final AtomicLong RUN_COUNT = new AtomicLong(0);
+
+ private final OzoneManager ozoneManager;
+ private final OMVersionManager versionManager;
+ private final ScmClient scmClient;
+ private final AtomicBoolean stopInitiated = new AtomicBoolean(false);
+
+ /**
+ * Creates an {@code OMUpgradeFinalizeService} with a custom check interval.
+ * Primarily intended for testing.
+ *
+ * @param ozoneManager the OzoneManager instance
+ * @param versionManager the {@link OMVersionManager} to query the finalization status
+ * @param scmClient the scmClient instance used to query SCM
+ * @param intervalMs the duration to wait between checks
+ */
+ public OMUpgradeFinalizeService(OzoneManager ozoneManager, OMVersionManager versionManager, ScmClient scmClient,
+ long intervalMs) {
+ super("OMUpgradeFinalizeService", intervalMs, INTERVAL_UNIT, THREAD_POOL_SIZE, TIMEOUT,
+ ozoneManager.getThreadNamePrefix());
+ this.ozoneManager = ozoneManager;
+ this.versionManager = versionManager;
+ this.scmClient = scmClient;
+ }
+
+ @Override
+ public BackgroundTaskQueue getTasks() {
+ BackgroundTaskQueue queue = new BackgroundTaskQueue();
+ if (!versionManager.needsFinalization()) {
+ // Finalization is done (or was never needed), so this service can now shutdown. To avoid deadlocking on the
+ // executor.awaitTermination by calling shutdown directly, spawn a thread to perform the shutdown which will
+ // block until this task / thread completes in the executor.
+ if (stopInitiated.compareAndSet(false, true)) {
+ LOG.info("OMUpgradeFinalizeService: finalization is no longer needed, shutting down.");
+ Thread stopper = new Thread(this::shutdown, "OMUpgradeFinalizeService-stopper");
+ stopper.setDaemon(true);
+ stopper.start();
+ }
+ return queue; // empty — PeriodicalTask.run() will return without scheduling work
+ }
+ if (ozoneManager.isLeaderReady()) {
+ queue.add(new UpgradeStatusCheckTask());
+ }
+ return queue;
+ }
+
+ /**
+ * Periodic task that checks upgrade finalization status and logs the result.
+ */
+ private class UpgradeStatusCheckTask implements BackgroundTask {
+
+ @Override
+ public BackgroundTaskResult call() {
+ if (!ozoneManager.isLeaderReady()) {
+ LOG.debug("OMUpgradeFinalizeService: skipping check — not the leader.");
+ return BackgroundTaskResult.EmptyTaskResult.newResult();
+ }
+ if (versionManager.needsFinalization()) {
+ try {
+ HddsProtos.UpgradeStatus upgradeStatus = scmClient.getContainerClient().queryUpgradeStatus();
+ if (upgradeStatus.getShouldFinalize()) {
+ LOG.info("The SCM Upgrade has been finalized. OM will now finalize");
+
+ OzoneManagerProtocolProtos.OMRequest omRequest = OzoneManagerProtocolProtos.OMRequest.newBuilder()
+ .setCmdType(OzoneManagerProtocolProtos.Type.FinalizeUpgrade)
+ .setClientId("todo")
+ .build();
+ OMClientRequest omClientRequest = OzoneManagerRatisUtils.createClientRequest(omRequest, ozoneManager);
+ omRequest = omClientRequest.preExecute(ozoneManager);
+ OzoneManagerRatisUtils.submitRequest(ozoneManager, omRequest, ClientId.randomId(),
Review Comment:
Yea that makes sense. I kind of assumed failure would result in an exception, but that probably isn't always the 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-15208. OM should learn to finalize from SCM after finalization command [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 merged PR #10236:
URL: https://github.com/apache/ozone/pull/10236
--
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-15208. OM should learn to finalize from SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #10236:
URL: https://github.com/apache/ozone/pull/10236#discussion_r3225591310
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMUpgradeFinalization.java:
##########
@@ -93,12 +99,10 @@ void testOMUpgradeFinalizationWithOneOMDown() throws Exception {
long prepareIndex = omClient.prepareOzoneManager(120L, 5L);
assertClusterPrepared(prepareIndex, runningOms);
AuditLogTestUtils.verifyAuditLog(OMAction.UPGRADE_PREPARE, AuditEventStatus.SUCCESS);
-
omClient.cancelOzoneManagerPrepare();
AuditLogTestUtils.verifyAuditLog(OMAction.UPGRADE_CANCEL, AuditEventStatus.SUCCESS);
- StatusAndMessages response =
- omClient.finalizeUpgrade("finalize-test");
- System.out.println("Finalization Messages : " + response.msgs());
+ // Send the finalize command to SCM which triggers the OM finalize when SCM reports it is complete.
+ cluster.getStorageContainerLocationClient().finalizeUpgrade();
Review Comment:
I wonder if we should change to the system audit in this PR, or in the one that makes the OM command a noop. If we change it here, it may break things in the existing command which we want to remove. I am open to either way.
--
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-15208. OM should learn to finalize from SCM [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #10236:
URL: https://github.com/apache/ozone/pull/10236#discussion_r3228623141
##########
hadoop-ozone/integration-test/src/test/resources/auditlog.properties:
##########
@@ -52,27 +52,34 @@ filter.write.onMismatch = NEUTRAL
# TRACE (least specific, a lot of data)
# ALL (least specific, all data)
-appenders = console, audit
+appenders = console, audit, systemaudit
appender.console.type = Console
appender.console.name = STDOUT
appender.console.layout.type = PatternLayout
-appender.console.layout.pattern = %d{DEFAULT} | %-5level | %c{1} | %msg | %throwable{3} %n
+appender.console.layout.pattern = %-5level | %c{1} | %msg%n
Review Comment:
Why was the pattern for both audit loggers changed? Can we use the original pattern for both?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/upgrade/OMFinalizeUpgradeRequest.java:
##########
@@ -95,6 +96,9 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
omMetadataManager.getMetaTable().addCacheEntry(
new CacheKey<>(APPARENT_VERSION_KEY),
CacheValue.get(context.getIndex(), String.valueOf(apparentVersion)));
+ // Clear the finalization_in_progress key from the cache
+ omMetadataManager.getMetaTable().addCacheEntry(
+ new CacheKey<>(OzoneConsts.FINALIZATION_IN_PROGRESS_KEY), CacheValue.get(System.nanoTime()));
Review Comment:
Looking at other examples it looks like the `epoch` parameter is the Ratis transaction index.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMRatisSnapshots.java:
##########
@@ -297,7 +297,7 @@ public void testInstallSnapshot(@TempDir Path tempDir) throws Exception {
String toMatch = String.format(
"op=DB_CHECKPOINT_INSTALL {\"leaderId\":\"%s\",\"term\":\"%d\",\"lastAppliedIndex\":\"%d\"}",
leaderOMNodeId, leaderOMSnapshotTermIndex, followerOMLastAppliedIndex);
- assertTrue(AuditLogTestUtils.auditLogContains(toMatch));
+ assertTrue(AuditLogTestUtils.systemAuditLogContains(toMatch));
Review Comment:
Why was this change required? We didn't modify the `DB_CHECKPOINT_INSTALL` operation.
##########
hadoop-hdds/framework/src/test/java/org/apache/hadoop/ozone/audit/AuditLogTestUtils.java:
##########
@@ -72,6 +91,7 @@ public static boolean auditLogContains(String... strings) {
public static void truncateAuditLogFile() throws IOException {
Files.write(Paths.get(AUDITLOG_FILENAME), new byte[0]);
+ Files.write(Paths.get(SYSTEM_AUDITLOG_FILENAME), new byte[0]);
Review Comment:
We should probably add the same for `deleteAuditLogFile` below.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -1900,6 +1908,10 @@ public void start() throws IOException {
bootstrap(omNodeDetails);
}
+ if (omUpgradeFinalizeService != null) {
Review Comment:
This is temporary until it is triggered by a client command, correct?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizeService.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.om.upgrade;
+
+import static org.apache.hadoop.ozone.OzoneConsts.FINALIZATION_IN_PROGRESS_KEY;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.BackgroundService;
+import org.apache.hadoop.hdds.utils.BackgroundTask;
+import org.apache.hadoop.hdds.utils.BackgroundTaskQueue;
+import org.apache.hadoop.hdds.utils.BackgroundTaskResult;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.ScmClient;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.ratis.protocol.ClientId;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A background service that periodically checks whether SCM has completed finalization of an upgrade and, if so,
+ * finalizes the OM upgrade.
+ */
+public class OMUpgradeFinalizeService extends BackgroundService {
+
+ private static final Logger LOG = LoggerFactory.getLogger(OMUpgradeFinalizeService.class);
+
+ private static final int THREAD_POOL_SIZE = 1;
+ private static final TimeUnit INTERVAL_UNIT = TimeUnit.MILLISECONDS;
+ private static final long TIMEOUT = 60000;
+ private static final AtomicLong RUN_COUNT = new AtomicLong(0);
+
+ private final OzoneManager ozoneManager;
+ private final OMVersionManager versionManager;
+ private final ScmClient scmClient;
+ private final AtomicBoolean stopInitiated = new AtomicBoolean(false);
+
+ /**
+ * Creates an {@code OMUpgradeFinalizeService} with a custom check interval.
+ *
+ * @param ozoneManager the OzoneManager instance
+ * @param versionManager the {@link OMVersionManager} to query the finalization status
+ * @param scmClient the scmClient instance used to query SCM
+ * @param intervalMs the duration to wait between checks
+ */
+ public OMUpgradeFinalizeService(OzoneManager ozoneManager, OMVersionManager versionManager, ScmClient scmClient,
+ long intervalMs) {
+ super("OMUpgradeFinalizeService", intervalMs, INTERVAL_UNIT, THREAD_POOL_SIZE, TIMEOUT,
+ ozoneManager.getThreadNamePrefix());
+ this.ozoneManager = ozoneManager;
+ this.versionManager = versionManager;
+ this.scmClient = scmClient;
+ }
+
+ @Override
+ public BackgroundTaskQueue getTasks() {
+ BackgroundTaskQueue queue = new BackgroundTaskQueue();
+ if (!versionManager.needsFinalization()) {
+ // Finalization is done (or was never needed), so this service can now shutdown. To avoid deadlocking on the
+ // executor.awaitTermination by calling shutdown directly, spawn a thread to perform the shutdown which will
+ // block until this task / thread completes in the executor.
+ if (stopInitiated.compareAndSet(false, true)) {
+ LOG.info("OMUpgradeFinalizeService: finalization is no longer needed, shutting down.");
+ Thread stopper = new Thread(this::shutdown, "OMUpgradeFinalizeService-stopper");
+ stopper.setDaemon(true);
+ stopper.start();
+ }
+ return queue; // empty — PeriodicalTask.run() will return without scheduling work
+ }
+ if (ozoneManager.isLeaderReady()) {
+ queue.add(new UpgradeStatusCheckTask());
+ }
+ return queue;
+ }
+
+ /**
+ * Periodic task that checks upgrade finalization status and logs the result.
+ */
+ private class UpgradeStatusCheckTask implements BackgroundTask {
+
+ @Override
+ public BackgroundTaskResult call() {
+ if (!ozoneManager.isLeaderReady()) {
+ LOG.debug("OMUpgradeFinalizeService: skipping check — not the leader.");
+ return BackgroundTaskResult.EmptyTaskResult.newResult();
+ }
+ if (versionManager.needsFinalization()) {
+ try {
+ // To finalize OM, first finalization needs to have been started. Then SCM needs to indicate that it has
+ // completed its finalization work. Only once both of those things have happened can OM finalize.
+ String finalizationInProgress =
+ ozoneManager.getMetadataManager().getMetaTable().get(FINALIZATION_IN_PROGRESS_KEY);
+ if (finalizationInProgress == null) {
+ LOG.debug("OMUpgradeFinalizeService: skipping check — finalization is not in progress.");
+ return BackgroundTaskResult.EmptyTaskResult.newResult();
+ }
+
+ HddsProtos.UpgradeStatus upgradeStatus = scmClient.getContainerClient().queryUpgradeStatus();
+ if (upgradeStatus.getShouldFinalize()) {
+ LOG.info("The SCM Upgrade has been finalized. OM will now finalize");
+
+ OzoneManagerProtocolProtos.OMRequest omRequest = OzoneManagerProtocolProtos.OMRequest.newBuilder()
+ .setCmdType(OzoneManagerProtocolProtos.Type.FinalizeUpgrade)
+ .setClientId(ozoneManager.getOMNodeId())
Review Comment:
Looks like key deleting services use the same client ID for both the OM request and Ratis request, and it is a random one. We should probably do the same.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizeService.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.om.upgrade;
+
+import static org.apache.hadoop.ozone.OzoneConsts.FINALIZATION_IN_PROGRESS_KEY;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.BackgroundService;
+import org.apache.hadoop.hdds.utils.BackgroundTask;
+import org.apache.hadoop.hdds.utils.BackgroundTaskQueue;
+import org.apache.hadoop.hdds.utils.BackgroundTaskResult;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.ScmClient;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.ratis.protocol.ClientId;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A background service that periodically checks whether SCM has completed finalization of an upgrade and, if so,
+ * finalizes the OM upgrade.
+ */
+public class OMUpgradeFinalizeService extends BackgroundService {
+
+ private static final Logger LOG = LoggerFactory.getLogger(OMUpgradeFinalizeService.class);
+
+ private static final int THREAD_POOL_SIZE = 1;
+ private static final TimeUnit INTERVAL_UNIT = TimeUnit.MILLISECONDS;
+ private static final long TIMEOUT = 60000;
+ private static final AtomicLong RUN_COUNT = new AtomicLong(0);
+
+ private final OzoneManager ozoneManager;
+ private final OMVersionManager versionManager;
+ private final ScmClient scmClient;
+ private final AtomicBoolean stopInitiated = new AtomicBoolean(false);
+
+ /**
+ * Creates an {@code OMUpgradeFinalizeService} with a custom check interval.
+ *
+ * @param ozoneManager the OzoneManager instance
+ * @param versionManager the {@link OMVersionManager} to query the finalization status
+ * @param scmClient the scmClient instance used to query SCM
+ * @param intervalMs the duration to wait between checks
+ */
+ public OMUpgradeFinalizeService(OzoneManager ozoneManager, OMVersionManager versionManager, ScmClient scmClient,
+ long intervalMs) {
+ super("OMUpgradeFinalizeService", intervalMs, INTERVAL_UNIT, THREAD_POOL_SIZE, TIMEOUT,
+ ozoneManager.getThreadNamePrefix());
+ this.ozoneManager = ozoneManager;
+ this.versionManager = versionManager;
+ this.scmClient = scmClient;
+ }
+
+ @Override
+ public BackgroundTaskQueue getTasks() {
+ BackgroundTaskQueue queue = new BackgroundTaskQueue();
+ if (!versionManager.needsFinalization()) {
+ // Finalization is done (or was never needed), so this service can now shutdown. To avoid deadlocking on the
+ // executor.awaitTermination by calling shutdown directly, spawn a thread to perform the shutdown which will
+ // block until this task / thread completes in the executor.
+ if (stopInitiated.compareAndSet(false, true)) {
+ LOG.info("OMUpgradeFinalizeService: finalization is no longer needed, shutting down.");
+ Thread stopper = new Thread(this::shutdown, "OMUpgradeFinalizeService-stopper");
+ stopper.setDaemon(true);
+ stopper.start();
+ }
+ return queue; // empty — PeriodicalTask.run() will return without scheduling work
+ }
+ if (ozoneManager.isLeaderReady()) {
+ queue.add(new UpgradeStatusCheckTask());
+ }
+ return queue;
+ }
+
+ /**
+ * Periodic task that checks upgrade finalization status and logs the result.
+ */
+ private class UpgradeStatusCheckTask implements BackgroundTask {
+
+ @Override
+ public BackgroundTaskResult call() {
+ if (!ozoneManager.isLeaderReady()) {
+ LOG.debug("OMUpgradeFinalizeService: skipping check — not the leader.");
+ return BackgroundTaskResult.EmptyTaskResult.newResult();
+ }
+ if (versionManager.needsFinalization()) {
+ try {
+ // To finalize OM, first finalization needs to have been started. Then SCM needs to indicate that it has
+ // completed its finalization work. Only once both of those things have happened can OM finalize.
+ String finalizationInProgress =
+ ozoneManager.getMetadataManager().getMetaTable().get(FINALIZATION_IN_PROGRESS_KEY);
+ if (finalizationInProgress == null) {
+ LOG.debug("OMUpgradeFinalizeService: skipping check — finalization is not in progress.");
+ return BackgroundTaskResult.EmptyTaskResult.newResult();
+ }
+
+ HddsProtos.UpgradeStatus upgradeStatus = scmClient.getContainerClient().queryUpgradeStatus();
+ if (upgradeStatus.getShouldFinalize()) {
+ LOG.info("The SCM Upgrade has been finalized. OM will now finalize");
+
+ OzoneManagerProtocolProtos.OMRequest omRequest = OzoneManagerProtocolProtos.OMRequest.newBuilder()
+ .setCmdType(OzoneManagerProtocolProtos.Type.FinalizeUpgrade)
+ .setClientId(ozoneManager.getOMNodeId())
+ .build();
+ OzoneManagerProtocolProtos.OMResponse response = OzoneManagerRatisUtils.submitRequest(
+ ozoneManager, omRequest, ClientId.randomId(), RUN_COUNT.getAndIncrement());
Review Comment:
We should increment run count even for no-op operations and include it in a summary log message at the end of the service similar to what the deleting services do.
--
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-15208. OM should learn to finalize from SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #10236:
URL: https://github.com/apache/ozone/pull/10236#discussion_r3225292515
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java:
##########
@@ -234,6 +234,9 @@ private void init() throws Exception {
// Enable filesystem snapshot feature for the test regardless of the default
conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
conf.setInt(OMStorage.TESTING_INIT_APPARENT_VERSION_KEY, OMLayoutFeature.BUCKET_LAYOUT_SUPPORT.layoutVersion());
+ conf.setInt(SCMStorageConfig.TESTING_INIT_LAYOUT_VERSION_KEY,
+ HDDSLayoutFeature.HADOOP_PRC_PORTS_IN_DATANODEDETAILS.layoutVersion());
Review Comment:
If SCM does not need finalized then OM will immediately finalize on the first poll of SCM and it makes it difficult to control the flow in the test. Therefore we need to make it so that SCM is unfinalized, then we finalize it and that triggers the OM finalize.
--
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-15208. OM should learn to finalize from SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #10236:
URL: https://github.com/apache/ozone/pull/10236#discussion_r3225302620
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSyncUpgrade.java:
##########
@@ -217,20 +215,8 @@ private void finalizeOMUpgrade() throws Exception {
// Trigger OM upgrade finalization. Ref: FinalizeUpgradeSubCommand#call
final OzoneManagerProtocol omClient = client.getObjectStore()
.getClientProxy().getOzoneManagerClient();
- final String upgradeClientID = "Test-Upgrade-Client-" + UUID.randomUUID();
- UpgradeFinalization.StatusAndMessages finalizationResponse =
- omClient.finalizeUpgrade(upgradeClientID);
-
- // The status should transition as soon as the client call above returns
- assertTrue(isStarting(finalizationResponse.status()));
- // Wait for the finalization to be marked as done.
- // 10s timeout should be plenty.
- await(POLL_MAX_WAIT_MILLIS, POLL_INTERVAL_MILLIS, () -> {
- final UpgradeFinalization.StatusAndMessages progress =
- omClient.queryUpgradeFinalizationProgress(
- upgradeClientID, false, false);
- return isDone(progress.status());
- });
+ cluster.getStorageContainerLocationClient().finalizeUpgrade();
+ OMUpgradeTestUtils.waitForFinalization(omClient);
Review Comment:
I don't know - the discoverability of these sort of methods isn't great and there may be places we want to have something in between.
--
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-15208. OM should learn to finalize from SCM [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #10236:
URL: https://github.com/apache/ozone/pull/10236#discussion_r3222325913
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java:
##########
@@ -234,6 +234,9 @@ private void init() throws Exception {
// Enable filesystem snapshot feature for the test regardless of the default
conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
conf.setInt(OMStorage.TESTING_INIT_APPARENT_VERSION_KEY, OMLayoutFeature.BUCKET_LAYOUT_SUPPORT.layoutVersion());
+ conf.setInt(SCMStorageConfig.TESTING_INIT_LAYOUT_VERSION_KEY,
+ HDDSLayoutFeature.HADOOP_PRC_PORTS_IN_DATANODEDETAILS.layoutVersion());
Review Comment:
Why do we need to change the HDDS version in this and other OM tests?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizeService.java:
##########
@@ -0,0 +1,127 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.om.upgrade;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.BackgroundService;
+import org.apache.hadoop.hdds.utils.BackgroundTask;
+import org.apache.hadoop.hdds.utils.BackgroundTaskQueue;
+import org.apache.hadoop.hdds.utils.BackgroundTaskResult;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.ScmClient;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils;
+import org.apache.hadoop.ozone.om.request.OMClientRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.ratis.protocol.ClientId;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A background service that periodically checks whether SCM has completed finalization of an upgrade and, if so,
+ * finalizes the OM upgrade.
+ */
+public class OMUpgradeFinalizeService extends BackgroundService {
+
+ private static final Logger LOG = LoggerFactory.getLogger(OMUpgradeFinalizeService.class);
+
+ private static final int THREAD_POOL_SIZE = 1;
+ private static final TimeUnit INTERVAL_UNIT = TimeUnit.MILLISECONDS;
+ private static final long TIMEOUT = 60000;
+ private static final AtomicLong RUN_COUNT = new AtomicLong(0);
+
+ private final OzoneManager ozoneManager;
+ private final OMVersionManager versionManager;
+ private final ScmClient scmClient;
+ private final AtomicBoolean stopInitiated = new AtomicBoolean(false);
+
+ /**
+ * Creates an {@code OMUpgradeFinalizeService} with a custom check interval.
+ * Primarily intended for testing.
Review Comment:
This is the prod constructor too.
```suggestion
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMUpgradeFinalization.java:
##########
@@ -93,12 +99,10 @@ void testOMUpgradeFinalizationWithOneOMDown() throws Exception {
long prepareIndex = omClient.prepareOzoneManager(120L, 5L);
assertClusterPrepared(prepareIndex, runningOms);
AuditLogTestUtils.verifyAuditLog(OMAction.UPGRADE_PREPARE, AuditEventStatus.SUCCESS);
-
omClient.cancelOzoneManagerPrepare();
AuditLogTestUtils.verifyAuditLog(OMAction.UPGRADE_CANCEL, AuditEventStatus.SUCCESS);
- StatusAndMessages response =
- omClient.finalizeUpgrade("finalize-test");
- System.out.println("Finalization Messages : " + response.msgs());
+ // Send the finalize command to SCM which triggers the OM finalize when SCM reports it is complete.
+ cluster.getStorageContainerLocationClient().finalizeUpgrade();
Review Comment:
The audit log check on the next line should be after we have waited for OM to finish finalizing. Also I think we want to switch this to the system audit log instead of the user audit log since it's triggered by a background process, not a user call to the OM.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSyncUpgrade.java:
##########
@@ -117,7 +112,10 @@ public void init() throws Exception {
conf.setTimeDuration(OZONE_OM_LEASE_HARD_LIMIT,
EXPIRE_THRESHOLD_MS, TimeUnit.MILLISECONDS);
conf.set(OzoneConfigKeys.OZONE_OM_LEASE_SOFT_LIMIT, "0s");
- conf.setInt(OMStorage.TESTING_INIT_APPARENT_VERSION_KEY, OMLayoutFeature.MULTITENANCY_SCHEMA.layoutVersion());
+ conf.setInt(OMStorage.TESTING_INIT_APPARENT_VERSION_KEY, OzoneManagerVersion.ATOMIC_REWRITE_KEY.serialize());
+ conf.setInt(SCMStorageConfig.TESTING_INIT_LAYOUT_VERSION_KEY,
+ HDDSLayoutFeature.HADOOP_PRC_PORTS_IN_DATANODEDETAILS.layoutVersion());
+ conf.set(OMConfigKeys.OZONE_OM_UPGRADE_FINALIZATION_CHECK_INTERVAL, "10ms");
Review Comment:
Why were these versions changed? The OM version used here is older than the unified versioning framework but not an `OMLayoutFeature` so it is not meant to be written to the version file.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/TestOMUpgradeFinalizeService.java:
##########
@@ -0,0 +1,244 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.om.upgrade;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyLong;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+
+import com.google.protobuf.ServiceException;
+import java.io.IOException;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.scm.protocol.StorageContainerLocationProtocol;
+import org.apache.hadoop.ozone.OzoneManagerVersion;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.ScmClient;
+import org.apache.hadoop.ozone.om.ratis.OzoneManagerRatisServer;
+import org.apache.ratis.protocol.ClientId;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Unit tests for {@link OMUpgradeFinalizeService}.
+ * Uses {@link org.apache.hadoop.hdds.utils.BackgroundService#runPeriodicalTaskNow()} to execute
+ * tasks synchronously on the test thread, avoiding timing-dependent polling.
+ */
+public class TestOMUpgradeFinalizeService {
+
+ // A long interval so the scheduler never fires automatically during tests;
+ // we drive execution manually via runPeriodicalTaskNow().
+ private static final long INTERVAL_MS = 60_000;
+
+ private OzoneManager ozoneManager;
+ private OMVersionManager versionManager;
+ private ScmClient scmClient;
+ private StorageContainerLocationProtocol containerClient;
+ private OzoneManagerRatisServer omRatisServer;
+ private OMUpgradeFinalizeService service;
+
+ @BeforeEach
+ void setUp() throws Exception {
Review Comment:
```suggestion
void setUp() {
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizeService.java:
##########
@@ -0,0 +1,127 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.om.upgrade;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.BackgroundService;
+import org.apache.hadoop.hdds.utils.BackgroundTask;
+import org.apache.hadoop.hdds.utils.BackgroundTaskQueue;
+import org.apache.hadoop.hdds.utils.BackgroundTaskResult;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.ScmClient;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils;
+import org.apache.hadoop.ozone.om.request.OMClientRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.ratis.protocol.ClientId;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A background service that periodically checks whether SCM has completed finalization of an upgrade and, if so,
+ * finalizes the OM upgrade.
+ */
+public class OMUpgradeFinalizeService extends BackgroundService {
+
+ private static final Logger LOG = LoggerFactory.getLogger(OMUpgradeFinalizeService.class);
+
+ private static final int THREAD_POOL_SIZE = 1;
+ private static final TimeUnit INTERVAL_UNIT = TimeUnit.MILLISECONDS;
+ private static final long TIMEOUT = 60000;
+ private static final AtomicLong RUN_COUNT = new AtomicLong(0);
+
+ private final OzoneManager ozoneManager;
+ private final OMVersionManager versionManager;
+ private final ScmClient scmClient;
+ private final AtomicBoolean stopInitiated = new AtomicBoolean(false);
+
+ /**
+ * Creates an {@code OMUpgradeFinalizeService} with a custom check interval.
+ * Primarily intended for testing.
+ *
+ * @param ozoneManager the OzoneManager instance
+ * @param versionManager the {@link OMVersionManager} to query the finalization status
+ * @param scmClient the scmClient instance used to query SCM
+ * @param intervalMs the duration to wait between checks
+ */
+ public OMUpgradeFinalizeService(OzoneManager ozoneManager, OMVersionManager versionManager, ScmClient scmClient,
+ long intervalMs) {
+ super("OMUpgradeFinalizeService", intervalMs, INTERVAL_UNIT, THREAD_POOL_SIZE, TIMEOUT,
+ ozoneManager.getThreadNamePrefix());
+ this.ozoneManager = ozoneManager;
+ this.versionManager = versionManager;
+ this.scmClient = scmClient;
+ }
+
+ @Override
+ public BackgroundTaskQueue getTasks() {
+ BackgroundTaskQueue queue = new BackgroundTaskQueue();
+ if (!versionManager.needsFinalization()) {
+ // Finalization is done (or was never needed), so this service can now shutdown. To avoid deadlocking on the
+ // executor.awaitTermination by calling shutdown directly, spawn a thread to perform the shutdown which will
+ // block until this task / thread completes in the executor.
+ if (stopInitiated.compareAndSet(false, true)) {
+ LOG.info("OMUpgradeFinalizeService: finalization is no longer needed, shutting down.");
+ Thread stopper = new Thread(this::shutdown, "OMUpgradeFinalizeService-stopper");
+ stopper.setDaemon(true);
+ stopper.start();
+ }
+ return queue; // empty — PeriodicalTask.run() will return without scheduling work
+ }
+ if (ozoneManager.isLeaderReady()) {
+ queue.add(new UpgradeStatusCheckTask());
+ }
+ return queue;
+ }
+
+ /**
+ * Periodic task that checks upgrade finalization status and logs the result.
+ */
+ private class UpgradeStatusCheckTask implements BackgroundTask {
+
+ @Override
+ public BackgroundTaskResult call() {
+ if (!ozoneManager.isLeaderReady()) {
+ LOG.debug("OMUpgradeFinalizeService: skipping check — not the leader.");
+ return BackgroundTaskResult.EmptyTaskResult.newResult();
+ }
+ if (versionManager.needsFinalization()) {
+ try {
+ HddsProtos.UpgradeStatus upgradeStatus = scmClient.getContainerClient().queryUpgradeStatus();
+ if (upgradeStatus.getShouldFinalize()) {
+ LOG.info("The SCM Upgrade has been finalized. OM will now finalize");
+
+ OzoneManagerProtocolProtos.OMRequest omRequest = OzoneManagerProtocolProtos.OMRequest.newBuilder()
+ .setCmdType(OzoneManagerProtocolProtos.Type.FinalizeUpgrade)
+ .setClientId("todo")
Review Comment:
We should decide if we want to make new protos for the finalize request. Since we are removing the ability for the client to call it and we can still make breaking changes without versioning before ZDU is released I'm inclined to think we should. Doesn't have to be in this PR though.
```suggestion
.setClientId(ozoneManager.getOMNodeId())
```
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSyncUpgrade.java:
##########
@@ -217,20 +215,8 @@ private void finalizeOMUpgrade() throws Exception {
// Trigger OM upgrade finalization. Ref: FinalizeUpgradeSubCommand#call
final OzoneManagerProtocol omClient = client.getObjectStore()
.getClientProxy().getOzoneManagerClient();
- final String upgradeClientID = "Test-Upgrade-Client-" + UUID.randomUUID();
- UpgradeFinalization.StatusAndMessages finalizationResponse =
- omClient.finalizeUpgrade(upgradeClientID);
-
- // The status should transition as soon as the client call above returns
- assertTrue(isStarting(finalizationResponse.status()));
- // Wait for the finalization to be marked as done.
- // 10s timeout should be plenty.
- await(POLL_MAX_WAIT_MILLIS, POLL_INTERVAL_MILLIS, () -> {
- final UpgradeFinalization.StatusAndMessages progress =
- omClient.queryUpgradeFinalizationProgress(
- upgradeClientID, false, false);
- return isDone(progress.status());
- });
+ cluster.getStorageContainerLocationClient().finalizeUpgrade();
+ OMUpgradeTestUtils.waitForFinalization(omClient);
Review Comment:
I'm pretty sure these will always be called together and only in an integration test scenario. Should we combine them into a single method in `MiniOzoneCluster` to finalize and wait, similar to `waitForClusterToBeReady`?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizeService.java:
##########
@@ -0,0 +1,127 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.om.upgrade;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.BackgroundService;
+import org.apache.hadoop.hdds.utils.BackgroundTask;
+import org.apache.hadoop.hdds.utils.BackgroundTaskQueue;
+import org.apache.hadoop.hdds.utils.BackgroundTaskResult;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.ScmClient;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils;
+import org.apache.hadoop.ozone.om.request.OMClientRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.ratis.protocol.ClientId;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A background service that periodically checks whether SCM has completed finalization of an upgrade and, if so,
+ * finalizes the OM upgrade.
+ */
+public class OMUpgradeFinalizeService extends BackgroundService {
+
+ private static final Logger LOG = LoggerFactory.getLogger(OMUpgradeFinalizeService.class);
+
+ private static final int THREAD_POOL_SIZE = 1;
+ private static final TimeUnit INTERVAL_UNIT = TimeUnit.MILLISECONDS;
+ private static final long TIMEOUT = 60000;
+ private static final AtomicLong RUN_COUNT = new AtomicLong(0);
+
+ private final OzoneManager ozoneManager;
+ private final OMVersionManager versionManager;
+ private final ScmClient scmClient;
+ private final AtomicBoolean stopInitiated = new AtomicBoolean(false);
+
+ /**
+ * Creates an {@code OMUpgradeFinalizeService} with a custom check interval.
+ * Primarily intended for testing.
+ *
+ * @param ozoneManager the OzoneManager instance
+ * @param versionManager the {@link OMVersionManager} to query the finalization status
+ * @param scmClient the scmClient instance used to query SCM
+ * @param intervalMs the duration to wait between checks
+ */
+ public OMUpgradeFinalizeService(OzoneManager ozoneManager, OMVersionManager versionManager, ScmClient scmClient,
+ long intervalMs) {
+ super("OMUpgradeFinalizeService", intervalMs, INTERVAL_UNIT, THREAD_POOL_SIZE, TIMEOUT,
+ ozoneManager.getThreadNamePrefix());
+ this.ozoneManager = ozoneManager;
+ this.versionManager = versionManager;
+ this.scmClient = scmClient;
+ }
+
+ @Override
+ public BackgroundTaskQueue getTasks() {
+ BackgroundTaskQueue queue = new BackgroundTaskQueue();
+ if (!versionManager.needsFinalization()) {
+ // Finalization is done (or was never needed), so this service can now shutdown. To avoid deadlocking on the
+ // executor.awaitTermination by calling shutdown directly, spawn a thread to perform the shutdown which will
+ // block until this task / thread completes in the executor.
+ if (stopInitiated.compareAndSet(false, true)) {
+ LOG.info("OMUpgradeFinalizeService: finalization is no longer needed, shutting down.");
+ Thread stopper = new Thread(this::shutdown, "OMUpgradeFinalizeService-stopper");
+ stopper.setDaemon(true);
+ stopper.start();
+ }
+ return queue; // empty — PeriodicalTask.run() will return without scheduling work
+ }
+ if (ozoneManager.isLeaderReady()) {
+ queue.add(new UpgradeStatusCheckTask());
+ }
+ return queue;
+ }
+
+ /**
+ * Periodic task that checks upgrade finalization status and logs the result.
+ */
+ private class UpgradeStatusCheckTask implements BackgroundTask {
+
+ @Override
+ public BackgroundTaskResult call() {
+ if (!ozoneManager.isLeaderReady()) {
+ LOG.debug("OMUpgradeFinalizeService: skipping check — not the leader.");
+ return BackgroundTaskResult.EmptyTaskResult.newResult();
+ }
+ if (versionManager.needsFinalization()) {
+ try {
+ HddsProtos.UpgradeStatus upgradeStatus = scmClient.getContainerClient().queryUpgradeStatus();
+ if (upgradeStatus.getShouldFinalize()) {
+ LOG.info("The SCM Upgrade has been finalized. OM will now finalize");
+
+ OzoneManagerProtocolProtos.OMRequest omRequest = OzoneManagerProtocolProtos.OMRequest.newBuilder()
+ .setCmdType(OzoneManagerProtocolProtos.Type.FinalizeUpgrade)
+ .setClientId("todo")
+ .build();
+ OMClientRequest omClientRequest = OzoneManagerRatisUtils.createClientRequest(omRequest, ozoneManager);
+ omRequest = omClientRequest.preExecute(ozoneManager);
Review Comment:
These will be called automatically when the request is submitted on the next line.
```suggestion
```
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizeService.java:
##########
@@ -0,0 +1,127 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.om.upgrade;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.BackgroundService;
+import org.apache.hadoop.hdds.utils.BackgroundTask;
+import org.apache.hadoop.hdds.utils.BackgroundTaskQueue;
+import org.apache.hadoop.hdds.utils.BackgroundTaskResult;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.ScmClient;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils;
+import org.apache.hadoop.ozone.om.request.OMClientRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.ratis.protocol.ClientId;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A background service that periodically checks whether SCM has completed finalization of an upgrade and, if so,
+ * finalizes the OM upgrade.
+ */
+public class OMUpgradeFinalizeService extends BackgroundService {
+
+ private static final Logger LOG = LoggerFactory.getLogger(OMUpgradeFinalizeService.class);
+
+ private static final int THREAD_POOL_SIZE = 1;
+ private static final TimeUnit INTERVAL_UNIT = TimeUnit.MILLISECONDS;
+ private static final long TIMEOUT = 60000;
+ private static final AtomicLong RUN_COUNT = new AtomicLong(0);
+
+ private final OzoneManager ozoneManager;
+ private final OMVersionManager versionManager;
+ private final ScmClient scmClient;
+ private final AtomicBoolean stopInitiated = new AtomicBoolean(false);
+
+ /**
+ * Creates an {@code OMUpgradeFinalizeService} with a custom check interval.
+ * Primarily intended for testing.
+ *
+ * @param ozoneManager the OzoneManager instance
+ * @param versionManager the {@link OMVersionManager} to query the finalization status
+ * @param scmClient the scmClient instance used to query SCM
+ * @param intervalMs the duration to wait between checks
+ */
+ public OMUpgradeFinalizeService(OzoneManager ozoneManager, OMVersionManager versionManager, ScmClient scmClient,
+ long intervalMs) {
+ super("OMUpgradeFinalizeService", intervalMs, INTERVAL_UNIT, THREAD_POOL_SIZE, TIMEOUT,
+ ozoneManager.getThreadNamePrefix());
+ this.ozoneManager = ozoneManager;
+ this.versionManager = versionManager;
+ this.scmClient = scmClient;
+ }
+
+ @Override
+ public BackgroundTaskQueue getTasks() {
+ BackgroundTaskQueue queue = new BackgroundTaskQueue();
+ if (!versionManager.needsFinalization()) {
+ // Finalization is done (or was never needed), so this service can now shutdown. To avoid deadlocking on the
+ // executor.awaitTermination by calling shutdown directly, spawn a thread to perform the shutdown which will
+ // block until this task / thread completes in the executor.
+ if (stopInitiated.compareAndSet(false, true)) {
+ LOG.info("OMUpgradeFinalizeService: finalization is no longer needed, shutting down.");
+ Thread stopper = new Thread(this::shutdown, "OMUpgradeFinalizeService-stopper");
+ stopper.setDaemon(true);
+ stopper.start();
+ }
+ return queue; // empty — PeriodicalTask.run() will return without scheduling work
+ }
+ if (ozoneManager.isLeaderReady()) {
+ queue.add(new UpgradeStatusCheckTask());
+ }
+ return queue;
+ }
+
+ /**
+ * Periodic task that checks upgrade finalization status and logs the result.
+ */
+ private class UpgradeStatusCheckTask implements BackgroundTask {
+
+ @Override
+ public BackgroundTaskResult call() {
+ if (!ozoneManager.isLeaderReady()) {
+ LOG.debug("OMUpgradeFinalizeService: skipping check — not the leader.");
+ return BackgroundTaskResult.EmptyTaskResult.newResult();
+ }
+ if (versionManager.needsFinalization()) {
+ try {
+ HddsProtos.UpgradeStatus upgradeStatus = scmClient.getContainerClient().queryUpgradeStatus();
+ if (upgradeStatus.getShouldFinalize()) {
+ LOG.info("The SCM Upgrade has been finalized. OM will now finalize");
+
+ OzoneManagerProtocolProtos.OMRequest omRequest = OzoneManagerProtocolProtos.OMRequest.newBuilder()
+ .setCmdType(OzoneManagerProtocolProtos.Type.FinalizeUpgrade)
+ .setClientId("todo")
+ .build();
+ OMClientRequest omClientRequest = OzoneManagerRatisUtils.createClientRequest(omRequest, ozoneManager);
+ omRequest = omClientRequest.preExecute(ozoneManager);
+ OzoneManagerRatisUtils.submitRequest(ozoneManager, omRequest, ClientId.randomId(),
Review Comment:
We should probably get the response and log if it fails.
--
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-15208. OM should learn to finalize from SCM after finalization command [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on PR #10236:
URL: https://github.com/apache/ozone/pull/10236#issuecomment-4614811483
@errose28 If the CI goes green I think all comments have been addressed. Please have a look when you have time.
--
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-15208. OM should learn to finalize from SCM [ozone]
Posted by "errose28 (via GitHub)" <gi...@apache.org>.
errose28 commented on code in PR #10236:
URL: https://github.com/apache/ozone/pull/10236#discussion_r3319785788
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/TestOMUpgradeFinalizeService.java:
##########
Review Comment:
Since request processing is mocked in this test, we should have a unit test for OMFinalizeUpgradeRequest/Response classes as well, similar to other OM requests. Looks like we didn't have those previously but they would be good to add.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java:
##########
@@ -234,6 +234,9 @@ private void init() throws Exception {
// Enable filesystem snapshot feature for the test regardless of the default
conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
conf.setInt(OMStorage.TESTING_INIT_APPARENT_VERSION_KEY, OMLayoutFeature.BUCKET_LAYOUT_SUPPORT.layoutVersion());
+ conf.setInt(SCMStorageConfig.TESTING_INIT_LAYOUT_VERSION_KEY,
+ HDDSLayoutFeature.HADOOP_PRC_PORTS_IN_DATANODEDETAILS.layoutVersion());
Review Comment:
In the updated flow SCM will finalize when instructed by OM from a user command, and then OM will follow. This prevents OM from finalizing before the user command just because a release had no HDDS version increase. Therefore the original change that was commented on was removed.
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMRatisSnapshots.java:
##########
@@ -297,7 +297,7 @@ public void testInstallSnapshot(@TempDir Path tempDir) throws Exception {
String toMatch = String.format(
"op=DB_CHECKPOINT_INSTALL {\"leaderId\":\"%s\",\"term\":\"%d\",\"lastAppliedIndex\":\"%d\"}",
leaderOMNodeId, leaderOMSnapshotTermIndex, followerOMLastAppliedIndex);
- assertTrue(AuditLogTestUtils.auditLogContains(toMatch));
+ assertTrue(AuditLogTestUtils.systemAuditLogContains(toMatch));
Review Comment:
This change is because we split the audit log configuration so that system audit logs go to their own logger, which is an improvement since the test can now distinguish between audit and system audit log messages and not accept one when it should have only accepted the other.
##########
hadoop-ozone/integration-test/src/test/resources/auditlog.properties:
##########
@@ -52,27 +52,34 @@ filter.write.onMismatch = NEUTRAL
# TRACE (least specific, a lot of data)
# ALL (least specific, all data)
-appenders = console, audit
+appenders = console, audit, systemaudit
appender.console.type = Console
appender.console.name = STDOUT
appender.console.layout.type = PatternLayout
-appender.console.layout.pattern = %d{DEFAULT} | %-5level | %c{1} | %msg | %throwable{3} %n
+appender.console.layout.pattern = %-5level | %c{1} | %msg%n
Review Comment:
This is removing the exception and timestamp from being logged to the console. IMO we should restore this to what it was before since it is easer to trace log messages in a test env from the console than opening dedicated files. `additivity` was also set to `false` for the audit logs which means they won't go to the console anymore. I think we should remove that so all log messages are searchable in one place.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java:
##########
@@ -1900,6 +1908,10 @@ public void start() throws IOException {
bootstrap(omNodeDetails);
}
+ if (omUpgradeFinalizeService != null) {
Review Comment:
Ok this works. We could alternatively start the thread when the command is received (in the next PR) if it is not too difficult to connect the pieces.
--
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-15208. OM should learn to finalize from SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #10236:
URL: https://github.com/apache/ozone/pull/10236#discussion_r3225499476
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizeService.java:
##########
@@ -0,0 +1,127 @@
+/*
+ * 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
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * 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.om.upgrade;
+
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
+import org.apache.hadoop.hdds.utils.BackgroundService;
+import org.apache.hadoop.hdds.utils.BackgroundTask;
+import org.apache.hadoop.hdds.utils.BackgroundTaskQueue;
+import org.apache.hadoop.hdds.utils.BackgroundTaskResult;
+import org.apache.hadoop.ozone.om.OzoneManager;
+import org.apache.hadoop.ozone.om.ScmClient;
+import org.apache.hadoop.ozone.om.ratis.utils.OzoneManagerRatisUtils;
+import org.apache.hadoop.ozone.om.request.OMClientRequest;
+import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos;
+import org.apache.ratis.protocol.ClientId;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A background service that periodically checks whether SCM has completed finalization of an upgrade and, if so,
+ * finalizes the OM upgrade.
+ */
+public class OMUpgradeFinalizeService extends BackgroundService {
+
+ private static final Logger LOG = LoggerFactory.getLogger(OMUpgradeFinalizeService.class);
+
+ private static final int THREAD_POOL_SIZE = 1;
+ private static final TimeUnit INTERVAL_UNIT = TimeUnit.MILLISECONDS;
+ private static final long TIMEOUT = 60000;
+ private static final AtomicLong RUN_COUNT = new AtomicLong(0);
+
+ private final OzoneManager ozoneManager;
+ private final OMVersionManager versionManager;
+ private final ScmClient scmClient;
+ private final AtomicBoolean stopInitiated = new AtomicBoolean(false);
+
+ /**
+ * Creates an {@code OMUpgradeFinalizeService} with a custom check interval.
+ * Primarily intended for testing.
+ *
+ * @param ozoneManager the OzoneManager instance
+ * @param versionManager the {@link OMVersionManager} to query the finalization status
+ * @param scmClient the scmClient instance used to query SCM
+ * @param intervalMs the duration to wait between checks
+ */
+ public OMUpgradeFinalizeService(OzoneManager ozoneManager, OMVersionManager versionManager, ScmClient scmClient,
+ long intervalMs) {
+ super("OMUpgradeFinalizeService", intervalMs, INTERVAL_UNIT, THREAD_POOL_SIZE, TIMEOUT,
+ ozoneManager.getThreadNamePrefix());
+ this.ozoneManager = ozoneManager;
+ this.versionManager = versionManager;
+ this.scmClient = scmClient;
+ }
+
+ @Override
+ public BackgroundTaskQueue getTasks() {
+ BackgroundTaskQueue queue = new BackgroundTaskQueue();
+ if (!versionManager.needsFinalization()) {
+ // Finalization is done (or was never needed), so this service can now shutdown. To avoid deadlocking on the
+ // executor.awaitTermination by calling shutdown directly, spawn a thread to perform the shutdown which will
+ // block until this task / thread completes in the executor.
+ if (stopInitiated.compareAndSet(false, true)) {
+ LOG.info("OMUpgradeFinalizeService: finalization is no longer needed, shutting down.");
+ Thread stopper = new Thread(this::shutdown, "OMUpgradeFinalizeService-stopper");
+ stopper.setDaemon(true);
+ stopper.start();
+ }
+ return queue; // empty — PeriodicalTask.run() will return without scheduling work
+ }
+ if (ozoneManager.isLeaderReady()) {
+ queue.add(new UpgradeStatusCheckTask());
+ }
+ return queue;
+ }
+
+ /**
+ * Periodic task that checks upgrade finalization status and logs the result.
+ */
+ private class UpgradeStatusCheckTask implements BackgroundTask {
+
+ @Override
+ public BackgroundTaskResult call() {
+ if (!ozoneManager.isLeaderReady()) {
+ LOG.debug("OMUpgradeFinalizeService: skipping check — not the leader.");
+ return BackgroundTaskResult.EmptyTaskResult.newResult();
+ }
+ if (versionManager.needsFinalization()) {
+ try {
+ HddsProtos.UpgradeStatus upgradeStatus = scmClient.getContainerClient().queryUpgradeStatus();
+ if (upgradeStatus.getShouldFinalize()) {
+ LOG.info("The SCM Upgrade has been finalized. OM will now finalize");
+
+ OzoneManagerProtocolProtos.OMRequest omRequest = OzoneManagerProtocolProtos.OMRequest.newBuilder()
+ .setCmdType(OzoneManagerProtocolProtos.Type.FinalizeUpgrade)
+ .setClientId("todo")
+ .build();
+ OMClientRequest omClientRequest = OzoneManagerRatisUtils.createClientRequest(omRequest, ozoneManager);
+ omRequest = omClientRequest.preExecute(ozoneManager);
Review Comment:
Yea, you are correct.
--
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-15208. OM should learn to finalize from SCM [ozone]
Posted by "sodonnel (via GitHub)" <gi...@apache.org>.
sodonnel commented on code in PR #10236:
URL: https://github.com/apache/ozone/pull/10236#discussion_r3225566725
##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestHSyncUpgrade.java:
##########
@@ -117,7 +112,10 @@ public void init() throws Exception {
conf.setTimeDuration(OZONE_OM_LEASE_HARD_LIMIT,
EXPIRE_THRESHOLD_MS, TimeUnit.MILLISECONDS);
conf.set(OzoneConfigKeys.OZONE_OM_LEASE_SOFT_LIMIT, "0s");
- conf.setInt(OMStorage.TESTING_INIT_APPARENT_VERSION_KEY, OMLayoutFeature.MULTITENANCY_SCHEMA.layoutVersion());
+ conf.setInt(OMStorage.TESTING_INIT_APPARENT_VERSION_KEY, OzoneManagerVersion.ATOMIC_REWRITE_KEY.serialize());
+ conf.setInt(SCMStorageConfig.TESTING_INIT_LAYOUT_VERSION_KEY,
+ HDDSLayoutFeature.HADOOP_PRC_PORTS_IN_DATANODEDETAILS.layoutVersion());
+ conf.set(OMConfigKeys.OZONE_OM_UPGRADE_FINALIZATION_CHECK_INTERVAL, "10ms");
Review Comment:
I think I was making changes to try to get this to work locally, but was failing due to the annotation / aspectj stuff and then forgot to change it back. I've reset it and will see if it passes the CI now.
--
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