Re: [PR] HDDS-15208. OM should learn to finalize from SCM [ozone]
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: [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]
Re: [PR] HDDS-15208. OM should learn to finalize from SCM [ozone]
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: [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]
Re: [PR] HDDS-15208. OM should learn to finalize from SCM [ozone]
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 com
Re: [PR] HDDS-15208. OM should learn to finalize from SCM [ozone]
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: [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]
Re: [PR] HDDS-15208. OM should learn to finalize from SCM [ozone]
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: [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]
Re: [PR] HDDS-15208. OM should learn to finalize from SCM [ozone]
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 = 6;
+ 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();
+ }
+
Re: [PR] HDDS-15208. OM should learn to finalize from SCM [ozone]
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 = 6;
+ 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();
+ }
+
Re: [PR] HDDS-15208. OM should learn to finalize from SCM [ozone]
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 = 6;
+ 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: [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]
Re: [PR] HDDS-15208. OM should learn to finalize from SCM [ozone]
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: [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]
Re: [PR] HDDS-15208. OM should learn to finalize from SCM [ozone]
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: [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]
Re: [PR] HDDS-15208. OM should learn to finalize from SCM [ozone]
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: [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]
Re: [PR] HDDS-15208. OM should learn to finalize from SCM [ozone]
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: [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]
Re: [PR] HDDS-15208. OM should learn to finalize from SCM [ozone]
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 = 6;
+ 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 in
