From b926c359701bd2b2029ffa8d2e48e75817d709bf Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Tue, 28 Jul 2026 15:34:31 -0400 Subject: [PATCH 1/3] Add ZDU upgrade actions --- .../org/apache/hadoop/ozone/OzoneConsts.java | 6 -- ...ClearFinalizingStateScmUpgradeAction.java} | 21 ++++- ...tClearFinalizingStateScmUpgradeAction.java | 83 +++++++++++++++++ .../server/upgrade/TestScmVersionManager.java | 4 +- .../ClearPreparedStateOmUpgradeAction.java | 60 +++++++++++++ ...TestClearPreparedStateOmUpgradeAction.java | 88 +++++++++++++++++++ .../om/upgrade/TestOMVersionManager.java | 4 +- .../om/upgrade/ZduOmUpgradeActionForTest.java | 33 ------- 8 files changed, 252 insertions(+), 47 deletions(-) rename hadoop-hdds/server-scm/src/{test/java/org/apache/hadoop/hdds/scm/server/upgrade/ZduScmUpgradeActionForTest.java => main/java/org/apache/hadoop/hdds/scm/server/upgrade/ClearFinalizingStateScmUpgradeAction.java} (50%) create mode 100644 hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/upgrade/TestClearFinalizingStateScmUpgradeAction.java create mode 100644 hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/ClearPreparedStateOmUpgradeAction.java create mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/TestClearPreparedStateOmUpgradeAction.java delete mode 100644 hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/ZduOmUpgradeActionForTest.java diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java index 6e7dad0a763b..c9b8d61d7fc8 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java @@ -407,17 +407,11 @@ public final class OzoneConsts { public static final String TRANSACTION_INFO_KEY = "#TRANSACTIONINFO"; public static final String TRANSACTION_INFO_SPLIT_KEY = "#"; - public static final String PREPARE_MARKER_KEY = "#PREPAREDINFO"; - public static final String CONTAINER_DB_TYPE_ROCKSDB = "RocksDB"; // An on-disk transient marker file used when replacing DB with checkpoint public static final String DB_TRANSIENT_MARKER = "dbInconsistentMarker"; - // An on-disk marker file used to indicate that the OM is in prepare and - // should remain prepared even after a restart. - public static final String PREPARE_MARKER = "prepareMarker"; - public static final String OZONE_RATIS_SNAPSHOT_DIR = "snapshot"; public static final long DEFAULT_OM_UPDATE_ID = -1L; diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/upgrade/ZduScmUpgradeActionForTest.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/ClearFinalizingStateScmUpgradeAction.java similarity index 50% rename from hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/upgrade/ZduScmUpgradeActionForTest.java rename to hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/ClearFinalizingStateScmUpgradeAction.java index aa12c999292b..d91ec0238382 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/upgrade/ZduScmUpgradeActionForTest.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/ClearFinalizingStateScmUpgradeAction.java @@ -20,16 +20,29 @@ import static org.apache.hadoop.hdds.HDDSVersion.ZDU; import org.apache.hadoop.hdds.scm.server.OzoneStorageContainerManager; +import org.apache.hadoop.hdds.scm.server.StorageContainerManager; import org.apache.hadoop.hdds.upgrade.ScmUpgradeAction; +import org.apache.hadoop.hdds.utils.db.Table; import org.apache.hadoop.ozone.upgrade.ScmUpgradeActionForVersion; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** - * No-op upgrade action used only to verify that {@link ScmUpgradeActionForVersion} is scanned by - * {@link org.apache.hadoop.hdds.upgrade.ScmUpgradeActionProvider} in tests. + * Removes the orphan "finalizing in progress" mark written into the SCM meta table by pre-ZDU + * code. Deleting an absent key is a no-op, making the action idempotent. */ @ScmUpgradeActionForVersion(version = ZDU) -public class ZduScmUpgradeActionForTest implements ScmUpgradeAction { +public class ClearFinalizingStateScmUpgradeAction implements ScmUpgradeAction { + private static final Logger LOG = LoggerFactory.getLogger(ClearFinalizingStateScmUpgradeAction.class); + + // Orphan "finalizing in progress" mark written by pre-ZDU SCM code; removed on ZDU finalization. + private static final String LEGACY_FINALIZING_KEY = "#FINALIZING"; + @Override - public void execute(OzoneStorageContainerManager arg) { + public void execute(OzoneStorageContainerManager context) throws Exception { + StorageContainerManager scm = (StorageContainerManager) context; + Table metaTable = scm.getScmMetadataStore().getMetaTable(); + scm.getScmHAManager().getDBTransactionBuffer().removeFromBuffer(metaTable, LEGACY_FINALIZING_KEY); + LOG.info("Removed leftover SCM finalizing mark {} during ZDU finalization.", LEGACY_FINALIZING_KEY); } } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/upgrade/TestClearFinalizingStateScmUpgradeAction.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/upgrade/TestClearFinalizingStateScmUpgradeAction.java new file mode 100644 index 000000000000..d2ba66072268 --- /dev/null +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/upgrade/TestClearFinalizingStateScmUpgradeAction.java @@ -0,0 +1,83 @@ +/* + * 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.hdds.scm.server.upgrade; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.apache.hadoop.hdds.scm.ha.SCMHAManager; +import org.apache.hadoop.hdds.scm.metadata.DBTransactionBuffer; +import org.apache.hadoop.hdds.scm.metadata.SCMMetadataStore; +import org.apache.hadoop.hdds.scm.server.StorageContainerManager; +import org.apache.hadoop.hdds.utils.db.Table; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@link ClearFinalizingStateScmUpgradeAction}. + */ +class TestClearFinalizingStateScmUpgradeAction { + + private static final String LEGACY_FINALIZING_KEY = "#FINALIZING"; + + @Test + void testRemovesFinalizingKey() throws Exception { + @SuppressWarnings("unchecked") + Table metaTable = mock(Table.class); + DBTransactionBuffer buffer = mock(DBTransactionBuffer.class); + + SCMMetadataStore metadataStore = mock(SCMMetadataStore.class); + when(metadataStore.getMetaTable()).thenReturn(metaTable); + + SCMHAManager haManager = mock(SCMHAManager.class); + when(haManager.getDBTransactionBuffer()).thenReturn(buffer); + + StorageContainerManager scm = mock(StorageContainerManager.class); + when(scm.getScmMetadataStore()).thenReturn(metadataStore); + when(scm.getScmHAManager()).thenReturn(haManager); + + new ClearFinalizingStateScmUpgradeAction().execute(scm); + + verify(buffer).removeFromBuffer(metaTable, LEGACY_FINALIZING_KEY); + } + + @Test + void testIdempotent() throws Exception { + // removeFromBuffer on an absent key is a no-op; call execute twice and expect no exception. + @SuppressWarnings("unchecked") + Table metaTable = mock(Table.class); + DBTransactionBuffer buffer = mock(DBTransactionBuffer.class); + + SCMMetadataStore metadataStore = mock(SCMMetadataStore.class); + when(metadataStore.getMetaTable()).thenReturn(metaTable); + + SCMHAManager haManager = mock(SCMHAManager.class); + when(haManager.getDBTransactionBuffer()).thenReturn(buffer); + + StorageContainerManager scm = mock(StorageContainerManager.class); + when(scm.getScmMetadataStore()).thenReturn(metadataStore); + when(scm.getScmHAManager()).thenReturn(haManager); + + ClearFinalizingStateScmUpgradeAction action = new ClearFinalizingStateScmUpgradeAction(); + action.execute(scm); + action.execute(scm); + + // Called twice, once per execution. + verify(buffer, org.mockito.Mockito.times(2)).removeFromBuffer(metaTable, LEGACY_FINALIZING_KEY); + } +} diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/upgrade/TestScmVersionManager.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/upgrade/TestScmVersionManager.java index 61dcae5826d6..5c32cd8f71be 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/upgrade/TestScmVersionManager.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/upgrade/TestScmVersionManager.java @@ -140,7 +140,7 @@ public void testClasspathScanDiscoversUpgradeActions() throws Exception { ScmUpgradeAction upgradeAction = versionManager.getUpgradeActionsForTesting().get(DATANODE_SCHEMA_V2); assertInstanceOf(ScmOnFinalizeActionForDatanodeSchemaV2.class, upgradeAction); ScmUpgradeAction zduAction = versionManager.getUpgradeActionsForTesting().get(HDDSVersion.ZDU); - assertInstanceOf(ZduScmUpgradeActionForTest.class, zduAction); + assertInstanceOf(ClearFinalizingStateScmUpgradeAction.class, zduAction); } try (ScmVersionManager versionManager = createManager(HDDSVersion.SOFTWARE_VERSION.serialize(), @@ -149,7 +149,7 @@ public void testClasspathScanDiscoversUpgradeActions() throws Exception { ScmUpgradeAction upgradeAction = versionManager.getUpgradeActionsForTesting().get(DATANODE_SCHEMA_V2); assertInstanceOf(ScmOnFinalizeActionForDatanodeSchemaV2.class, upgradeAction); ScmUpgradeAction zduAction = versionManager.getUpgradeActionsForTesting().get(HDDSVersion.ZDU); - assertInstanceOf(ZduScmUpgradeActionForTest.class, zduAction); + assertInstanceOf(ClearFinalizingStateScmUpgradeAction.class, zduAction); } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/ClearPreparedStateOmUpgradeAction.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/ClearPreparedStateOmUpgradeAction.java new file mode 100644 index 000000000000..5ab4b0933031 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/ClearPreparedStateOmUpgradeAction.java @@ -0,0 +1,60 @@ +/* + * 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.OzoneManagerVersion.ZDU; + +import java.io.File; +import java.io.IOException; +import org.apache.hadoop.hdds.server.ServerUtils; +import org.apache.hadoop.ozone.common.Storage; +import org.apache.hadoop.ozone.om.OzoneManager; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Removes leftover OM "prepare for upgrade" state written by pre-ZDU code. + * It is idempotent (delete-if-exists), so it is a no-op on clusters that were never prepared and + * on fresh ZDU clusters that never finalize. + */ +@OmUpgradeActionForVersion(version = ZDU) +public class ClearPreparedStateOmUpgradeAction implements OmUpgradeAction { + private static final Logger LOG = LoggerFactory.getLogger(ClearPreparedStateOmUpgradeAction.class); + + // On-disk marker written by pre-ZDU OM prepare code; removed on ZDU finalization. + private static final String LEGACY_PREPARE_MARKER = "prepareMarker"; + // transactionInfoTable key written by pre-ZDU OM prepare code. + private static final String LEGACY_PREPARE_MARKER_KEY = "#PREPAREDINFO"; + + @Override + public void execute(OzoneManager om) throws Exception { + // Reproduces the removed getPrepareMarkerFile() logic: /current/prepareMarker. + File markerDir = new File(ServerUtils.getOzoneMetaDirPath(om.getConfiguration()), + Storage.STORAGE_DIR_CURRENT); + File marker = new File(markerDir, LEGACY_PREPARE_MARKER); + if (marker.exists()) { + if (!marker.delete()) { + throw new IOException("Failed to delete leftover OM prepare marker file " + marker); + } + LOG.info("Deleted leftover OM prepare marker file {}", marker); + } + + // Direct RocksDB delete of the orphan prepare key, mirroring the removed startup cleanup. + om.getMetadataManager().getTransactionInfoTable().delete(LEGACY_PREPARE_MARKER_KEY); + } +} diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/TestClearPreparedStateOmUpgradeAction.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/TestClearPreparedStateOmUpgradeAction.java new file mode 100644 index 000000000000..18ecd7f50301 --- /dev/null +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/TestClearPreparedStateOmUpgradeAction.java @@ -0,0 +1,88 @@ +/* + * 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.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.io.File; +import java.nio.file.Path; +import org.apache.hadoop.hdds.HddsConfigKeys; +import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.utils.TransactionInfo; +import org.apache.hadoop.hdds.utils.db.Table; +import org.apache.hadoop.ozone.om.OMMetadataManager; +import org.apache.hadoop.ozone.om.OzoneManager; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +/** + * Unit tests for {@link ClearPreparedStateOmUpgradeAction}. + */ +class TestClearPreparedStateOmUpgradeAction { + + private static final String PREPARE_MARKER_KEY = "#PREPAREDINFO"; + + @TempDir + private Path tempDir; + + private OzoneManager mockOm(OzoneConfiguration conf) { + OMMetadataManager metadataManager = mock(OMMetadataManager.class); + @SuppressWarnings("unchecked") + Table txTable = mock(Table.class); + when(metadataManager.getTransactionInfoTable()).thenReturn(txTable); + + OzoneManager om = mock(OzoneManager.class); + when(om.getConfiguration()).thenReturn(conf); + when(om.getMetadataManager()).thenReturn(metadataManager); + return om; + } + + @Test + void testDeletesMarkerFileAndDbKey() throws Exception { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(org.apache.hadoop.hdds.HddsConfigKeys.OZONE_METADATA_DIRS, tempDir.toString()); + + // Create the "current" dir and the marker file that pre-ZDU code would leave behind. + File currentDir = new File(tempDir.toFile(), "current"); + assertTrue(currentDir.mkdirs()); + File marker = new File(currentDir, "prepareMarker"); + assertTrue(marker.createNewFile()); + + OzoneManager om = mockOm(conf); + new ClearPreparedStateOmUpgradeAction().execute(om); + + assertFalse(marker.exists(), "prepare marker file should be deleted"); + verify(om.getMetadataManager().getTransactionInfoTable()).delete(PREPARE_MARKER_KEY); + } + + @Test + void testIdempotentWhenNoMarkerPresent() throws Exception { + OzoneConfiguration conf = new OzoneConfiguration(); + conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, tempDir.toString()); + + OzoneManager om = mockOm(conf); + // Should succeed without error even though the marker file and DB key are absent. + new ClearPreparedStateOmUpgradeAction().execute(om); + + verify(om.getMetadataManager().getTransactionInfoTable()).delete(PREPARE_MARKER_KEY); + } +} diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/TestOMVersionManager.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/TestOMVersionManager.java index 009ff9f2020d..5a927fa3614f 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/TestOMVersionManager.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/TestOMVersionManager.java @@ -133,7 +133,7 @@ public void testClasspathScanDiscoversUpgradeActions() throws Exception { OmUpgradeAction quotaAction = versionManager.getUpgradeActionsForTesting().get(QUOTA); assertInstanceOf(QuotaRepairUpgradeAction.class, quotaAction); OmUpgradeAction zduAction = versionManager.getUpgradeActionsForTesting().get(ZDU); - assertInstanceOf(ZduOmUpgradeActionForTest.class, zduAction); + assertInstanceOf(ClearPreparedStateOmUpgradeAction.class, zduAction); } try (OMVersionManager versionManager = createManager(SOFTWARE_VERSION.serialize(), new OMUpgradeActionProvider())) { @@ -141,7 +141,7 @@ public void testClasspathScanDiscoversUpgradeActions() throws Exception { OmUpgradeAction quotaAction = versionManager.getUpgradeActionsForTesting().get(QUOTA); assertInstanceOf(QuotaRepairUpgradeAction.class, quotaAction); OmUpgradeAction zduAction = versionManager.getUpgradeActionsForTesting().get(ZDU); - assertInstanceOf(ZduOmUpgradeActionForTest.class, zduAction); + assertInstanceOf(ClearPreparedStateOmUpgradeAction.class, zduAction); } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/ZduOmUpgradeActionForTest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/ZduOmUpgradeActionForTest.java deleted file mode 100644 index b46099926512..000000000000 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/upgrade/ZduOmUpgradeActionForTest.java +++ /dev/null @@ -1,33 +0,0 @@ -/* - * 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.OzoneManagerVersion.ZDU; - -import org.apache.hadoop.ozone.om.OzoneManager; - -/** - * No-op upgrade action used only to verify that {@link OmUpgradeActionForVersion} is scanned by - * {@link OMUpgradeActionProvider} in tests. - */ -@OmUpgradeActionForVersion(version = ZDU) -public class ZduOmUpgradeActionForTest implements OmUpgradeAction { - @Override - public void execute(OzoneManager arg) { - } -} From bf2c615d3c4b7da9b96eadf3119c112188587fde Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Tue, 28 Jul 2026 15:39:46 -0400 Subject: [PATCH 2/3] Update comment --- .../server/upgrade/ClearFinalizingStateScmUpgradeAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/ClearFinalizingStateScmUpgradeAction.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/ClearFinalizingStateScmUpgradeAction.java index d91ec0238382..70b6641ec1bd 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/ClearFinalizingStateScmUpgradeAction.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/ClearFinalizingStateScmUpgradeAction.java @@ -28,7 +28,7 @@ import org.slf4j.LoggerFactory; /** - * Removes the orphan "finalizing in progress" mark written into the SCM meta table by pre-ZDU + * Removes the "finalizing in progress" mark, which may be left over in SCM meta table from pre-ZDU * code. Deleting an absent key is a no-op, making the action idempotent. */ @ScmUpgradeActionForVersion(version = ZDU) From 9a98e4da787bcf2fe91567e2ddce1acf75985a9d Mon Sep 17 00:00:00 2001 From: Ethan Rose Date: Thu, 30 Jul 2026 20:10:37 -0400 Subject: [PATCH 3/3] Add tests asserting crash behavior when upgrade actions fail to run or load --- .../AbstractUpgradeActionProvider.java | 3 +- .../TestAbstractUpgradeActionProvider.java | 75 +++++++++++++++++++ .../hdds/scm/ha/TestSCMStateMachine.java | 58 +++++++++++++- .../upgrade/TestOMFinalizeUpgradeRequest.java | 35 +++++++++ 4 files changed, 167 insertions(+), 4 deletions(-) create mode 100644 hadoop-hdds/framework/src/test/java/org/apache/hadoop/ozone/upgrade/TestAbstractUpgradeActionProvider.java diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/ozone/upgrade/AbstractUpgradeActionProvider.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/ozone/upgrade/AbstractUpgradeActionProvider.java index f80514ec5e90..a95dcfda2e1f 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/ozone/upgrade/AbstractUpgradeActionProvider.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/ozone/upgrade/AbstractUpgradeActionProvider.java @@ -80,8 +80,7 @@ public Map load() { LOG.info("Registering Upgrade Action : {}", action.name()); upgradeActions.put(version, action); } catch (Exception e) { - LOG.error("Cannot instantiate Upgrade Action class {}", - clazz.getSimpleName(), e); + throw new IllegalStateException("Cannot instantiate Upgrade Action class " + clazz.getName(), e); } } else { LOG.warn("Found upgrade action class not of type {} : {}", diff --git a/hadoop-hdds/framework/src/test/java/org/apache/hadoop/ozone/upgrade/TestAbstractUpgradeActionProvider.java b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/ozone/upgrade/TestAbstractUpgradeActionProvider.java new file mode 100644 index 000000000000..f69ab94d69d2 --- /dev/null +++ b/hadoop-hdds/framework/src/test/java/org/apache/hadoop/ozone/upgrade/TestAbstractUpgradeActionProvider.java @@ -0,0 +1,75 @@ +/* + * 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.upgrade; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +import com.google.common.collect.ImmutableSet; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import org.apache.hadoop.hdds.ComponentVersion; +import org.apache.hadoop.hdds.upgrade.HDDSLayoutFeature; +import org.junit.jupiter.api.Test; + +/** + * Tests for {@link AbstractUpgradeActionProvider#load()}. + */ +class TestAbstractUpgradeActionProvider { + + /** + * An upgrade action whose no-arg constructor fails to instantiate must abort {@code load()} rather than + * silently drop the action from the returned map, which would finalize its version as a no-op. + */ + @Test + public void testLoadFailsWhenActionCannotBeInstantiated() { + assertThrows(IllegalStateException.class, () -> new ThrowingActionProvider().load()); + } + + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.TYPE) + private @interface TestUpgradeAction { + } + + private interface TestAction extends UpgradeAction { + } + + @TestUpgradeAction + public static class ThrowingTestAction implements TestAction { + ThrowingTestAction() { + throw new IllegalArgumentException("cannot construct test upgrade action"); + } + + @Override + public void execute(Object arg) { + } + } + + private static final class ThrowingActionProvider extends AbstractUpgradeActionProvider { + ThrowingActionProvider() { + super(ImmutableSet.of(TestUpgradeAction.class), TestAction.class, + TestAbstractUpgradeActionProvider.class.getPackage().getName()); + } + + @Override + protected ComponentVersion extractVersion(Class clazz) { + return HDDSLayoutFeature.INITIAL_VERSION; + } + } +} diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMStateMachine.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMStateMachine.java index 828606f2d424..cb8196dd1c38 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMStateMachine.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMStateMachine.java @@ -17,15 +17,24 @@ package org.apache.hadoop.hdds.scm.ha; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol.RequestType; import org.apache.hadoop.hdds.scm.container.placement.metrics.SCMMetrics; +import org.apache.hadoop.hdds.scm.ha.invoker.ScmInvoker; import org.apache.hadoop.hdds.scm.server.StorageContainerManager; import org.apache.hadoop.hdds.utils.TransactionInfo; +import org.apache.hadoop.ozone.upgrade.UpgradeException; import org.apache.ratis.proto.RaftProtos; +import org.apache.ratis.proto.RaftProtos.LogEntryProto; +import org.apache.ratis.proto.RaftProtos.StateMachineLogEntryProto; import org.apache.ratis.server.protocol.TermIndex; +import org.apache.ratis.statemachine.TransactionContext; +import org.apache.ratis.util.ExitUtils; import org.junit.jupiter.api.Test; /** @@ -42,11 +51,56 @@ public void testRatisEventsRecording() throws Exception { SCMHADBTransactionBuffer buffer = mock(SCMHADBTransactionBuffer.class); when(buffer.getLatestTrxInfo()).thenReturn(TransactionInfo.valueOf(TermIndex.valueOf(0, 0))); - SCMStateMachine stateMachine = new SCMStateMachine(scm, buffer); + try (SCMStateMachine stateMachine = new SCMStateMachine(scm, buffer)) { + stateMachine.notifyConfigurationChanged(1, 1, RaftProtos.RaftConfigurationProto.getDefaultInstance()); + } - stateMachine.notifyConfigurationChanged(1, 1, RaftProtos.RaftConfigurationProto.getDefaultInstance()); assertTrue(metrics.getRatisEvents().contains("Configuration changed at term index")); metrics.unRegister(); } + + /** + * A finalization step that throws an UpgradeException (an IOException, not an SCMException) must + * crash SCM rather than be returned to the Ratis client. UpgradeException skips the inner + * catch (SCMException) and hits the outer catch (Exception) -> ExitUtils.terminate. + */ + @Test + public void testUpgradeExceptionDuringApplyTerminates() throws Exception { + ExitUtils.disableSystemExit(); + + StorageContainerManager scm = mock(StorageContainerManager.class); + SCMMetrics metrics = SCMMetrics.create(); + when(scm.getMetrics()).thenReturn(metrics); + + SCMHADBTransactionBuffer buffer = mock(SCMHADBTransactionBuffer.class); + when(buffer.getLatestTrxInfo()).thenReturn(TransactionInfo.valueOf(TermIndex.valueOf(0, 0))); + + try (SCMStateMachine stateMachine = new SCMStateMachine(scm, buffer)) { + ScmInvoker invoker = mock(ScmInvoker.class); + when(invoker.invokeLocal(any(), any())).thenThrow( + new UpgradeException(UpgradeException.ResultCodes.FINALIZE_UPGRADE_ACTION_FAILED)); + stateMachine.registerInvoker(RequestType.FINALIZE, invoker); + + SCMRatisRequest request = SCMRatisRequest.of( + RequestType.FINALIZE, "finalize", new Class[]{}); + StateMachineLogEntryProto smLogEntry = StateMachineLogEntryProto.newBuilder() + .setLogData(request.encode().getContent()) + .build(); + LogEntryProto logEntry = LogEntryProto.newBuilder() + .setTerm(1) + .setIndex(1) + .setStateMachineLogEntry(smLogEntry) + .build(); + TransactionContext trx = mock(TransactionContext.class); + when(trx.getStateMachineLogEntry()).thenReturn(smLogEntry); + when(trx.getLogEntry()).thenReturn(logEntry); + + // terminate throws ExitException when system exit is disabled + assertThrows(ExitUtils.ExitException.class, + () -> stateMachine.applyTransaction(trx)); + } + + metrics.unRegister(); + } } diff --git a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/upgrade/TestOMFinalizeUpgradeRequest.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/upgrade/TestOMFinalizeUpgradeRequest.java index 2879a4e3d960..e282ce8dc0ca 100644 --- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/upgrade/TestOMFinalizeUpgradeRequest.java +++ b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/upgrade/TestOMFinalizeUpgradeRequest.java @@ -18,6 +18,7 @@ package org.apache.hadoop.ozone.om.request.upgrade; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -32,9 +33,14 @@ import org.apache.hadoop.ozone.OzoneConsts; import org.apache.hadoop.ozone.OzoneManagerVersion; import org.apache.hadoop.ozone.om.execution.flowcontrol.ExecutionContext; +import org.apache.hadoop.ozone.om.ratis.TestOzoneManagerStateMachine; import org.apache.hadoop.ozone.om.request.key.OMKeyRequestTests; +import org.apache.hadoop.ozone.om.response.OMClientResponse; import org.apache.hadoop.ozone.om.upgrade.OMVersionManager; import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.OMResponse; +import org.apache.hadoop.ozone.protocol.proto.OzoneManagerProtocolProtos.Status; +import org.apache.hadoop.ozone.upgrade.UpgradeException; import org.apache.hadoop.ozone.upgrade.UpgradeFinalization; import org.apache.ratis.protocol.ClientId; import org.apache.ratis.server.protocol.TermIndex; @@ -71,6 +77,35 @@ public void testFinalizationInProgressKeyRemoved() throws IOException { "metric should be 0 after finalizing"); } + /** + * A failed upgrade step (e.g. an upgrade action) surfaces as an UpgradeException, + * which is an IOException but not an OMException. exceptionToResponseStatus therefore + * maps it to INTERNAL_ERROR, and OzoneManagerStateMachine.processResponse terminates the + * process on INTERNAL_ERROR. The INTERNAL_ERROR -> terminate half of the chain is covered by + * {@link TestOzoneManagerStateMachine#testProcessResponseInternalErrorTerminates}. + */ + @Test + public void testFinalizeFailureMapsToInternalError() throws IOException { + OMVersionManager omVersionManager = mock(OMVersionManager.class); + when(omVersionManager.getApparentVersion()).thenReturn(OzoneManagerVersion.DEFAULT_VERSION); + when(ozoneManager.getVersionManager()).thenReturn(omVersionManager); + when(ozoneManager.finalizeUpgrade(any())).thenThrow( + new UpgradeException(UpgradeException.ResultCodes.FINALIZE_UPGRADE_ACTION_FAILED)); + + OzoneManagerProtocolProtos.OMRequest omRequest = OzoneManagerProtocolProtos.OMRequest.newBuilder() + .setCmdType(OzoneManagerProtocolProtos.Type.FinalizeUpgrade) + .setClientId(ClientId.randomId().toString()) + .build(); + OMFinalizeUpgradeRequest request = new OMFinalizeUpgradeRequest(omRequest); + ExecutionContext context = ExecutionContext.of(1, TermIndex.INITIAL_VALUE); + request.preExecute(ozoneManager); + + OMClientResponse response = request.validateAndUpdateCache(ozoneManager, context); + OMResponse omResponse = response.getOMResponse(); + assertFalse(omResponse.getSuccess()); + assertEquals(Status.INTERNAL_ERROR, omResponse.getStatus()); + } + private void submitRequest() throws IOException { OzoneManagerProtocolProtos.OMRequest omRequest = OzoneManagerProtocolProtos.OMRequest.newBuilder() .setCmdType(OzoneManagerProtocolProtos.Type.FinalizeUpgrade)