From 877243ca5d6034723c93241f04f19462026f048e Mon Sep 17 00:00:00 2001 From: Aravindan Vijayan Date: Wed, 14 Apr 2021 23:35:49 -0700 Subject: [PATCH 1/8] HDDS-5108. Cleanup *UpgradeFinalizer classes. --- .../ozone/upgrade/BasicUpgradeFinalizer.java | 193 +++++++++--------- .../DefaultUpgradeFinalizationExecutor.java | 39 ++-- .../ozone/upgrade/UpgradeException.java | 6 +- .../ozone/upgrade/UpgradeFinalizer.java | 4 - .../upgrade/TestHDDSLayoutVersionManager.java | 10 + .../InjectedUpgradeFinalizationExecutor.java | 34 ++- .../upgrade/TestBasicUpgradeFinalizer.java | 126 ++++++++++++ ...estDefaultUpgradeFinalizationExecutor.java | 101 +++++++++ ....java => TestUpgradeFinalizerActions.java} | 15 +- .../statemachine/DatanodeStateMachine.java | 39 +--- .../upgrade/DataNodeUpgradeFinalizer.java | 74 +++---- .../scm/server/StorageContainerManager.java | 128 ------------ .../server/upgrade/SCMUpgradeFinalizer.java | 132 +++++++++--- .../ozone/om/upgrade/OMUpgradeFinalizer.java | 46 +---- .../om/upgrade/TestOMVersionManager.java | 3 +- .../ozone/admin/om/PrepareSubCommand.java | 17 +- 16 files changed, 531 insertions(+), 436 deletions(-) create mode 100644 hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestBasicUpgradeFinalizer.java create mode 100644 hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestDefaultUpgradeFinalizationExecutor.java rename hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/{TestUpgradeFinalizer.java => TestUpgradeFinalizerActions.java} (94%) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java index 1c0e3d328093..7fec8d4f623a 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java @@ -18,13 +18,14 @@ package org.apache.hadoop.ozone.upgrade; +import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.ON_FINALIZE; import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.ON_FIRST_UPGRADE_START; import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.UNFINALIZED_STATE_VALIDATION; import static org.apache.hadoop.ozone.upgrade.UpgradeException.ResultCodes.FIRST_UPGRADE_START_ACTION_FAILED; import static org.apache.hadoop.ozone.upgrade.UpgradeException.ResultCodes.INVALID_REQUEST; import static org.apache.hadoop.ozone.upgrade.UpgradeException.ResultCodes.LAYOUT_FEATURE_FINALIZATION_FAILED; import static org.apache.hadoop.ozone.upgrade.UpgradeException.ResultCodes.PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED; -import static org.apache.hadoop.ozone.upgrade.UpgradeException.ResultCodes.PREFINALIZE_STATE_VALIDATION_FAILED; +import static org.apache.hadoop.ozone.upgrade.UpgradeException.ResultCodes.PREFINALIZE_ACTION_VALIDATION_FAILED; import static org.apache.hadoop.ozone.upgrade.UpgradeException.ResultCodes.REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED; import static org.apache.hadoop.ozone.upgrade.UpgradeException.ResultCodes.UPDATE_LAYOUT_VERSION_FAILED; import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE; @@ -39,6 +40,7 @@ import java.util.concurrent.ConcurrentLinkedQueue; import java.util.function.Function; import java.util.concurrent.TimeUnit; +import java.util.function.Supplier; import org.apache.hadoop.ozone.common.Storage; import org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeAction; @@ -50,37 +52,86 @@ /** * UpgradeFinalizer implementation for the Storage Container Manager service. */ -@SuppressWarnings("checkstyle:VisibilityModifier") public abstract class BasicUpgradeFinalizer implements UpgradeFinalizer { - protected V versionManager; - protected String clientID; - protected T component; - protected DefaultUpgradeFinalizationExecutor finalizationExecutor; + private V versionManager; + private String clientID; + private T component; + private DefaultUpgradeFinalizationExecutor finalizationExecutor; private Queue msgs = new ConcurrentLinkedQueue<>(); - protected boolean isDone = false; + private boolean isDone = false; public BasicUpgradeFinalizer(V versionManager) { this.versionManager = versionManager; - this.finalizationExecutor = - new DefaultUpgradeFinalizationExecutor(); + this.finalizationExecutor = new DefaultUpgradeFinalizationExecutor<>(); } - /** - * Sets the Finalization Executor driver. - * @param executor FinalizationExecutor. - */ + public StatusAndMessages finalize(String upgradeClientID, T service) + throws IOException { + StatusAndMessages response = preFinalize(upgradeClientID, service); + if (response.status() != FINALIZATION_REQUIRED) { + return response; + } + finalizationExecutor.execute(service, this); + return STARTING_MSG; + } - public void setFinalizationExecutor(DefaultUpgradeFinalizationExecutor - executor) { - finalizationExecutor = executor; + public synchronized StatusAndMessages reportStatus( + String upgradeClientID, boolean takeover) throws UpgradeException { + if (takeover) { + clientID = upgradeClientID; + } + assertClientId(upgradeClientID); + List returningMsgs = new ArrayList<>(msgs.size() + 10); + Status status = versionManager.getUpgradeState(); + while (msgs.size() > 0) { + returningMsgs.add(msgs.poll()); + } + return new StatusAndMessages(status, returningMsgs); } + protected abstract void preFinalizeUpgrade(T service) throws IOException; + + protected abstract void postFinalizeUpgrade(T service) throws IOException; + + public abstract void finalizeUpgrade(T service) throws UpgradeException; + @Override - public DefaultUpgradeFinalizationExecutor getFinalizationExecutor() { - return finalizationExecutor; + public void finalizeAndWaitForCompletion( + String upgradeClientID, T service, long maxTimeToWaitInSeconds) + throws IOException { + + StatusAndMessages response = finalize(upgradeClientID, service); + LOG.info("Finalization Messages : {} ", response.msgs()); + if (isFinalized(response.status())) { + return; + } + + boolean success = false; + long endTime = System.currentTimeMillis() + + TimeUnit.SECONDS.toMillis(maxTimeToWaitInSeconds); + while (System.currentTimeMillis() < endTime) { + try { + response = reportStatus(upgradeClientID, false); + LOG.info("Finalization Messages : {} ", response.msgs()); + if (isFinalized(response.status())) { + success = true; + break; + } + Thread.sleep(2000); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new IOException("Finalization Wait thread interrupted!"); + } + } + if (!success) { + LOG.error("Unable to finalize after waiting for {} seconds", + maxTimeToWaitInSeconds); + } else { + updateLayoutVersionInDB(versionManager, service); + } } public boolean isFinalizationDone() { @@ -95,9 +146,8 @@ public V getVersionManager() { return versionManager; } - public synchronized StatusAndMessages preFinalize(String upgradeClientID, - T id) - throws UpgradeException { + private synchronized StatusAndMessages preFinalize( + String upgradeClientID, T id) throws UpgradeException { switch (versionManager.getUpgradeState()) { case STARTING_FINALIZATION: return STARTING_MSG; @@ -119,46 +169,12 @@ public synchronized StatusAndMessages preFinalize(String upgradeClientID, } versionManager.setUpgradeState(STARTING_FINALIZATION); - clientID = upgradeClientID; + this.clientID = upgradeClientID; this.component = id; return FINALIZATION_REQUIRED_MSG; } } - /* - * This method must be overriden by the component implementing the - * finalization logic. - */ - public StatusAndMessages finalize(String upgradeClientID, T id) - throws IOException { - StatusAndMessages response = preFinalize(upgradeClientID, id); - if (response.status() != FINALIZATION_REQUIRED) { - return response; - } - - /** - * Overriding class should schedule actual finalization logic - * in a separate thread here. - */ - return STARTING_MSG; - } - - @Override - public synchronized StatusAndMessages reportStatus( - String upgradeClientID, boolean takeover - ) throws UpgradeException { - if (takeover) { - clientID = upgradeClientID; - } - assertClientId(upgradeClientID); - List returningMsgs = new ArrayList<>(msgs.size()+10); - Status status = versionManager.getUpgradeState(); - while (msgs.size() > 0) { - returningMsgs.add(msgs.poll()); - } - return new StatusAndMessages(status, returningMsgs); - } - private void assertClientId(String id) throws UpgradeException { if (this.clientID == null || !this.clientID.equals(id)) { throw new UpgradeException("Unknown client tries to get finalization " + @@ -168,44 +184,22 @@ private void assertClientId(String id) throws UpgradeException { } } - public void finalizeAndWaitForCompletion(String upgradeClientID, T service, - long maxTimeToWaitInSeconds) - throws IOException { - - StatusAndMessages response = finalize(upgradeClientID, service); - LOG.info("Finalization Messages : {} ", response.msgs()); - if (isFinalized(response.status())) { - return; - } - - boolean success = false; - long endTime = System.currentTimeMillis() + - TimeUnit.SECONDS.toMillis(maxTimeToWaitInSeconds); - while (System.currentTimeMillis() < endTime) { - try { - response = reportStatus(upgradeClientID, false); - LOG.info("Finalization Messages : {} ", response.msgs()); - if (isFinalized(response.status())) { - success = true; - break; - } - Thread.sleep(2000); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new IOException("Finalization Wait thread interrupted!"); - } - } - if (!success) { - LOG.error("Unable to finalize after waiting for {} seconds", - maxTimeToWaitInSeconds); - } else { - updateLayoutVersionInDB(versionManager, component); - } + private static boolean isFinalized(Status status) { + return status.equals(Status.ALREADY_FINALIZED) + || status.equals(FINALIZATION_DONE); } - private static boolean isFinalized(UpgradeFinalizer.Status status) { - return status.equals(UpgradeFinalizer.Status.ALREADY_FINALIZED) - || status.equals(FINALIZATION_DONE); + protected void finalizeUpgrade(Supplier storageSuppplier) + throws UpgradeException { + for (Object obj : versionManager.unfinalizedFeatures()) { + LayoutFeature lf = (LayoutFeature) obj; + Storage layoutStorage = storageSuppplier.get(); + Optional action = lf.action(ON_FINALIZE); + finalizeFeature(lf, layoutStorage, action); + updateLayoutVersionInVersionFile(lf, layoutStorage); + versionManager.finalized(lf); + } + versionManager.completeFinalization(); } protected void finalizeFeature(LayoutFeature feature, Storage config, @@ -283,7 +277,7 @@ private void runValidationAction(LayoutFeature f, UpgradeAction action) LOG.error(String.format(msg, f.name())); throw new UpgradeException( String.format(msg, f.name()), ex, - PREFINALIZE_STATE_VALIDATION_FAILED); + PREFINALIZE_ACTION_VALIDATION_FAILED); } } @@ -458,14 +452,13 @@ private void logAndThrow(Exception e, String msg, ResultCodes resultCode) throw new UpgradeException(msg, e, resultCode); } + @VisibleForTesting + public void setFinalizationExecutor(DefaultUpgradeFinalizationExecutor + executor) { + finalizationExecutor = executor; + } + protected void updateLayoutVersionInDB(V vm, T comp) throws IOException { throw new UnsupportedOperationException(); } - - protected abstract void postFinalizeUpgrade() throws IOException; - - protected abstract void finalizeUpgrade(Storage storageConfig) - throws UpgradeException; - - protected abstract boolean preFinalizeUpgrade() throws IOException; } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java index 14c1f2df8107..3cfb9c4e39fa 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java @@ -21,7 +21,8 @@ import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS; import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED; -import org.apache.hadoop.ozone.common.Storage; +import java.io.IOException; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -30,48 +31,36 @@ * Unit/Integration tests can override this to provide error injected version * of this class. */ - -@SuppressWarnings("checkstyle:VisibilityModifier") -public class DefaultUpgradeFinalizationExecutor { +public class DefaultUpgradeFinalizationExecutor { static final Logger LOG = LoggerFactory.getLogger(DefaultUpgradeFinalizationExecutor.class); public DefaultUpgradeFinalizationExecutor() { } - public Void execute(Storage storageConfig, - BasicUpgradeFinalizer basicUpgradeFinalizer) - throws Exception { + public Void execute(T component, BasicUpgradeFinalizer finalizer) + throws IOException { try { - basicUpgradeFinalizer.emitStartingMsg(); - basicUpgradeFinalizer.getVersionManager() + finalizer.emitStartingMsg(); + finalizer.getVersionManager() .setUpgradeState(FINALIZATION_IN_PROGRESS); - /* - * Before we can call finalize the feature, we need to make sure that - * all existing pipelines are closed and pipeline Manger would freeze - * all new pipeline creation. - */ - if(!basicUpgradeFinalizer.preFinalizeUpgrade()) { - return null; - } + finalizer.preFinalizeUpgrade(component); - basicUpgradeFinalizer.finalizeUpgrade(storageConfig); + finalizer.finalizeUpgrade(component); - basicUpgradeFinalizer.postFinalizeUpgrade(); + finalizer.postFinalizeUpgrade(component); - basicUpgradeFinalizer.emitFinishedMsg(); + finalizer.emitFinishedMsg(); + finalizer.markFinalizationDone(); return null; } catch (Exception e) { LOG.warn("Upgrade Finalization failed with following Exception:"); - e.printStackTrace(); - if (basicUpgradeFinalizer.getVersionManager().needsFinalization()) { - basicUpgradeFinalizer.getVersionManager() + if (finalizer.getVersionManager().needsFinalization()) { + finalizer.getVersionManager() .setUpgradeState(FINALIZATION_REQUIRED); throw (e); } - } finally { - basicUpgradeFinalizer.markFinalizationDone(); } return null; } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/UpgradeException.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/UpgradeException.java index dc3a656f3c9d..816c33364190 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/UpgradeException.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/UpgradeException.java @@ -112,7 +112,9 @@ public enum ResultCodes { REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED, UPDATE_LAYOUT_VERSION_FAILED, LAYOUT_FEATURE_FINALIZATION_FAILED, - PREFINALIZE_STATE_VALIDATION_FAILED, - FIRST_UPGRADE_START_ACTION_FAILED; + PREFINALIZE_ACTION_VALIDATION_FAILED, + FIRST_UPGRADE_START_ACTION_FAILED, + + PREFINALIZE_VALIDATION_FAILED; } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/UpgradeFinalizer.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/UpgradeFinalizer.java index 59387b374076..a710e3b8ebf5 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/UpgradeFinalizer.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/UpgradeFinalizer.java @@ -187,8 +187,4 @@ StatusAndMessages reportStatus(String upgradeClientId, boolean takeover) void runPrefinalizeStateActions(Storage storage, T service) throws IOException; - /** - * get the Finalization Executor driver. - */ - DefaultUpgradeFinalizationExecutor getFinalizationExecutor(); } diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSLayoutVersionManager.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSLayoutVersionManager.java index e3ebae8a640a..06683b2a170d 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSLayoutVersionManager.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSLayoutVersionManager.java @@ -89,4 +89,14 @@ public void testUpgradeActionsRegistered() throws Exception { verify(mockObj, times(0)).mockMethodScm(); verify(mockObj, times(1)).mockMethodDn(); } + + @Test + public void testHDDSLayoutFeaturesHaveIncreasingLayoutVersion() { + HDDSLayoutFeature[] values = HDDSLayoutFeature.values(); + int currVersion = Integer.MIN_VALUE; + for (HDDSLayoutFeature lf : values) { + assertTrue(currVersion < lf.layoutVersion()); + currVersion = lf.layoutVersion(); + } + } } \ No newline at end of file diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java index 6b512b690b63..afe177a54416 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java @@ -25,9 +25,9 @@ import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_IN_PROGRESS; import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED; +import java.io.IOException; import java.util.concurrent.Callable; -import org.apache.hadoop.ozone.common.Storage; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,9 +35,8 @@ * Failure injected extension of DefaultUpgradeFinalizationExecutor that * can be used by Unit/Integration Tests. */ -@SuppressWarnings("checkstyle:VisibilityModifier") -public class InjectedUpgradeFinalizationExecutor extends - DefaultUpgradeFinalizationExecutor { +public class InjectedUpgradeFinalizationExecutor extends + DefaultUpgradeFinalizationExecutor { static final Logger LOG = LoggerFactory.getLogger(InjectedUpgradeFinalizationExecutor.class); @@ -66,38 +65,33 @@ static class UpgradeTestInjectionAbort extends Exception { } @Override - public Void execute(Storage storageConfig, - BasicUpgradeFinalizer basicUpgradeFinalizer) - throws Exception { + public Void execute(T component, BasicUpgradeFinalizer finalizer) + throws IOException { try { injectTestFunctionAtThisPoint(BEFORE_PRE_FINALIZE_UPGRADE); - basicUpgradeFinalizer.emitStartingMsg(); - basicUpgradeFinalizer.getVersionManager() + finalizer.emitStartingMsg(); + finalizer.getVersionManager() .setUpgradeState(FINALIZATION_IN_PROGRESS); - if(!basicUpgradeFinalizer.preFinalizeUpgrade()) { - return null; - } + finalizer.preFinalizeUpgrade(component); injectTestFunctionAtThisPoint(AFTER_PRE_FINALIZE_UPGRADE); - basicUpgradeFinalizer.finalizeUpgrade(storageConfig); - + finalizer.finalizeUpgrade(component); injectTestFunctionAtThisPoint(AFTER_COMPLETE_FINALIZATION); - basicUpgradeFinalizer.postFinalizeUpgrade(); + finalizer.postFinalizeUpgrade(component); injectTestFunctionAtThisPoint(AFTER_POST_FINALIZE_UPGRADE); - basicUpgradeFinalizer.emitFinishedMsg(); + finalizer.emitFinishedMsg(); + finalizer.markFinalizationDone(); return null; } catch (Exception e) { LOG.warn("Upgrade Finalization failed with following Exception:"); e.printStackTrace(); - if (basicUpgradeFinalizer.getVersionManager().needsFinalization()) { - basicUpgradeFinalizer.getVersionManager() + if (finalizer.getVersionManager().needsFinalization()) { + finalizer.getVersionManager() .setUpgradeState(FINALIZATION_REQUIRED); } - } finally { - basicUpgradeFinalizer.markFinalizationDone(); } return null; } diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestBasicUpgradeFinalizer.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestBasicUpgradeFinalizer.java new file mode 100644 index 000000000000..ebf7158fd9ae --- /dev/null +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestBasicUpgradeFinalizer.java @@ -0,0 +1,126 @@ +/** + * 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.apache.hadoop.ozone.upgrade.TestUpgradeFinalizerActions.MockLayoutFeature.VERSION_1; +import static org.apache.hadoop.ozone.upgrade.TestUpgradeFinalizerActions.MockLayoutFeature.VERSION_3; +import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.ALREADY_FINALIZED; +import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_DONE; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.doCallRealMethod; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; + +import java.io.IOException; + +import org.apache.hadoop.ozone.common.Storage; +import org.apache.hadoop.ozone.upgrade.TestUpgradeFinalizerActions.MockLayoutVersionManager; +import org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.StatusAndMessages; +import org.junit.Test; +import org.mockito.ArgumentMatchers; +import org.mockito.InOrder; + +/** + * Test for BasicUpgradeFinalizer. + */ +public class TestBasicUpgradeFinalizer { + + @Test + public void testFinalizerPhasesAreInvokedInOrder() throws IOException { + SimpleTestFinalizer finalizer = spy(SimpleTestFinalizer.class); + InOrder inOrder = inOrder(finalizer); + + Object mockObj = mock(Object.class); + doCallRealMethod().when(finalizer).finalize(anyString(), + ArgumentMatchers.eq(mockObj)); + + finalizer.finalize("test-client-1", mockObj); + + StatusAndMessages res = finalizer.reportStatus("test-client-1", false); + assertEquals(FINALIZATION_DONE, res.status()); + + inOrder.verify(finalizer).preFinalizeUpgrade(ArgumentMatchers.eq(mockObj)); + inOrder.verify(finalizer).finalizeUpgrade(ArgumentMatchers.eq(mockObj)); + inOrder.verify(finalizer).postFinalizeUpgrade(ArgumentMatchers.eq(mockObj)); + + assertTrue(finalizer.isFinalizationDone()); + assertTrue(finalizer.preCalled && finalizer.finalizeCalled && + finalizer.postCalled); + } + + + @Test + public void testAlreadyFinalizedDoesNotTriggerNewFinalization() + throws IOException { + SimpleTestFinalizer finalizer = new SimpleTestFinalizer( + new MockLayoutVersionManager(VERSION_3.layoutVersion())); + + Object mockObj = mock(Object.class); + StatusAndMessages res = + finalizer.finalize("test-client-1", mockObj); + + assertEquals(ALREADY_FINALIZED, res.status()); + assertFalse(finalizer.preCalled || finalizer.finalizeCalled || + finalizer.postCalled); + } + + /** + * Yet another mock finalizer. + */ + static class SimpleTestFinalizer extends BasicUpgradeFinalizer { + + private boolean preCalled = false; + private boolean finalizeCalled = false; + private boolean postCalled = false; + + SimpleTestFinalizer() throws IOException { + super(new MockLayoutVersionManager(VERSION_1.layoutVersion())); + } + + SimpleTestFinalizer(MockLayoutVersionManager lvm) { + super(lvm); + } + + @Override + protected void preFinalizeUpgrade(Object service) { + preCalled = true; + } + + @Override + protected void postFinalizeUpgrade(Object service) { + postCalled = true; + } + + @Override + public void finalizeUpgrade(Object service) { + finalizeCalled = true; + getVersionManager().completeFinalization(); + } + + @Override + public void runPrefinalizeStateActions(Storage storage, Object service) { + + } + } +} diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestDefaultUpgradeFinalizationExecutor.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestDefaultUpgradeFinalizationExecutor.java new file mode 100644 index 000000000000..6db707aa1043 --- /dev/null +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestDefaultUpgradeFinalizationExecutor.java @@ -0,0 +1,101 @@ +/** + * 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.Assert.assertFalse; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.IOException; + +import org.apache.hadoop.ozone.common.Storage; +import org.apache.hadoop.test.LambdaTestUtils; +import org.junit.Test; + +/** + * Test for DefaultUpgradeFinalizationExecutor. + */ +public class TestDefaultUpgradeFinalizationExecutor { + + @Test + public void testPreFinalizeFailureThrowsException() throws Exception { + AbstractLayoutVersionManager mockLvm = + mock(AbstractLayoutVersionManager.class); + when(mockLvm.needsFinalization()).thenReturn(true); + + BasicUpgradeFinalizer uf = + new BasicUpgradeFinalizer(mockLvm) { + @Override + protected void preFinalizeUpgrade(Object service) throws IOException { + throw new IOException("Failure!"); + } + + @Override + protected void postFinalizeUpgrade(Object service) { + } + + @Override + public void finalizeUpgrade(Object service) { + } + + @Override + public void runPrefinalizeStateActions(Storage storage, Object service) { + } + }; + + DefaultUpgradeFinalizationExecutor executor = + new DefaultUpgradeFinalizationExecutor(); + LambdaTestUtils.intercept(IOException.class, + "Failure!", () -> executor.execute(new Object(), uf)); + } + + @Test + public void testPostFinalizeFailureDoesNotThrowException() throws Exception { + AbstractLayoutVersionManager mockLvm = + mock(AbstractLayoutVersionManager.class); + when(mockLvm.needsFinalization()).thenReturn(false); + + BasicUpgradeFinalizer uf = + new BasicUpgradeFinalizer(mockLvm) { + @Override + protected void preFinalizeUpgrade(Object service) { + } + + @Override + protected void postFinalizeUpgrade(Object service) + throws IOException { + throw new IOException("Failure!"); + } + + @Override + public void finalizeUpgrade(Object service) { + } + + @Override + public void runPrefinalizeStateActions(Storage storage, + Object service) { + } + }; + + DefaultUpgradeFinalizationExecutor executor = + new DefaultUpgradeFinalizationExecutor(); + executor.execute(new Object(), uf); + assertFalse(uf.isFinalizationDone()); + } +} \ No newline at end of file diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestUpgradeFinalizer.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestUpgradeFinalizerActions.java similarity index 94% rename from hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestUpgradeFinalizer.java rename to hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestUpgradeFinalizerActions.java index a6d6d570f1ba..a72aaf74b7fa 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestUpgradeFinalizer.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestUpgradeFinalizerActions.java @@ -21,8 +21,8 @@ import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeType.SCM; import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.ON_FIRST_UPGRADE_START; import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.UNFINALIZED_STATE_VALIDATION; -import static org.apache.hadoop.ozone.upgrade.TestUpgradeFinalizer.MockLayoutFeature.VERSION_2; -import static org.apache.hadoop.ozone.upgrade.TestUpgradeFinalizer.MockLayoutFeature.VERSION_3; +import static org.apache.hadoop.ozone.upgrade.TestUpgradeFinalizerActions.MockLayoutFeature.VERSION_2; +import static org.apache.hadoop.ozone.upgrade.TestUpgradeFinalizerActions.MockLayoutFeature.VERSION_3; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -48,7 +48,7 @@ /** * Class to test upgrade related actions. */ -public class TestUpgradeFinalizer { +public class TestUpgradeFinalizerActions { @Rule public TemporaryFolder folder = new TemporaryFolder(); @@ -130,19 +130,18 @@ static class MockUpgradeFinalizer extends } @Override - protected void postFinalizeUpgrade() throws IOException { + public void postFinalizeUpgrade(MockComponent c) { return; } @Override - protected void finalizeUpgrade(Storage storageConfig) - throws UpgradeException { + public void finalizeUpgrade(MockComponent c) { return; } @Override - protected boolean preFinalizeUpgrade() throws IOException { - return false; + public void preFinalizeUpgrade(MockComponent c) { + return; } @Override diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java index 45cf48e30e74..352117569c18 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java @@ -18,7 +18,6 @@ import java.io.Closeable; import java.io.IOException; -import java.util.Iterator; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; @@ -30,7 +29,6 @@ import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; -import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerDataProto.State; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.CommandStatusReportsProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.ContainerReportsProto; import org.apache.hadoop.hdds.protocol.proto.StorageContainerDatanodeProtocolProtos.NodeReportProto; @@ -40,7 +38,6 @@ import org.apache.hadoop.hdds.utils.LegacyHadoopConfigurationSource; import org.apache.hadoop.ozone.HddsDatanodeStopService; import org.apache.hadoop.ozone.container.common.DatanodeLayoutStorage; -import org.apache.hadoop.ozone.container.common.interfaces.Container; import org.apache.hadoop.ozone.container.common.report.ReportManager; import org.apache.hadoop.ozone.container.common.statemachine.commandhandler.CloseContainerCommandHandler; import org.apache.hadoop.ozone.container.common.statemachine.commandhandler.ClosePipelineCommandHandler; @@ -133,8 +130,7 @@ public DatanodeStateMachine(DatanodeDetails datanodeDetails, layoutVersionManager = new HDDSLayoutVersionManager( layoutStorage.getLayoutVersion()); - upgradeFinalizer = new DataNodeUpgradeFinalizer(layoutVersionManager, - datanodeDetails.getUuidString()); + upgradeFinalizer = new DataNodeUpgradeFinalizer(layoutVersionManager); DatanodeMetadataFeatures.initialize(layoutVersionManager); executorService = Executors.newFixedThreadPool( @@ -603,39 +599,6 @@ public DatanodeLayoutStorage getLayoutStorage() { return layoutStorage; } - private boolean canFinalizeDataNode() { - // Lets be sure that we do not have any open container before we return - // from here. This function should be called in its own finalizer thread - // context. - Iterator> containerIt = - getContainer().getController().getContainers(); - while (containerIt.hasNext()) { - Container ctr = containerIt.next(); - State state = ctr.getContainerState(); - switch (state) { - case OPEN: - case CLOSING: - case UNHEALTHY: - LOG.warn("FinalizeUpgrade : Waiting for container to close, current " + - "state is: {}", state); - return false; - default: - continue; - } - } - return true; - } - - @VisibleForTesting - public boolean preFinalizeUpgrade() { - return canFinalizeDataNode(); - } - - - @VisibleForTesting - public void postFinalizeUpgrade() { - } - public StatusAndMessages finalizeUpgrade() throws IOException{ return upgradeFinalizer.finalize(datanodeDetails.getUuidString(), this); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeFinalizer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeFinalizer.java index f4e70269142a..e5f40d693a7d 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeFinalizer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeFinalizer.java @@ -18,18 +18,19 @@ package org.apache.hadoop.ozone.container.upgrade; -import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.ON_FINALIZE; +import static org.apache.hadoop.ozone.upgrade.UpgradeException.ResultCodes.PREFINALIZE_VALIDATION_FAILED; import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED; import java.io.IOException; -import java.util.Optional; +import java.util.Iterator; +import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos; import org.apache.hadoop.hdds.upgrade.HDDSLayoutFeature; import org.apache.hadoop.hdds.upgrade.HDDSLayoutVersionManager; import org.apache.hadoop.ozone.common.Storage; +import org.apache.hadoop.ozone.container.common.interfaces.Container; import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine; import org.apache.hadoop.ozone.upgrade.BasicUpgradeFinalizer; -import org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeAction; import org.apache.hadoop.ozone.upgrade.UpgradeException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,63 +42,64 @@ public class DataNodeUpgradeFinalizer extends BasicUpgradeFinalizer { private static final Logger LOG = LoggerFactory.getLogger(DataNodeUpgradeFinalizer.class); - private DatanodeStateMachine datanodeStateMachine; - public DataNodeUpgradeFinalizer(HDDSLayoutVersionManager versionManager, - String optionalClientID) { + public DataNodeUpgradeFinalizer(HDDSLayoutVersionManager versionManager) { super(versionManager); - clientID = optionalClientID; } @Override public StatusAndMessages finalize(String upgradeClientID, DatanodeStateMachine dsm) throws IOException { - datanodeStateMachine = dsm; - StatusAndMessages response = preFinalize(upgradeClientID, dsm); - if (response.status() != FINALIZATION_REQUIRED) { - return response; - } - try { - getFinalizationExecutor().execute(dsm.getLayoutStorage(), - this); - } catch (Exception e) { - e.printStackTrace(); - throw new IOException(e.getMessage()); - } - return STARTING_MSG; + return super.finalize(upgradeClientID, dsm); } @Override - public boolean preFinalizeUpgrade() throws IOException { - if(!datanodeStateMachine.preFinalizeUpgrade()) { + public void preFinalizeUpgrade(DatanodeStateMachine dsm) + throws IOException { + if(!canFinalizeDataNode(dsm)) { // DataNode is not yet ready to finalize. // Reset the Finalization state. - versionManager.setUpgradeState(FINALIZATION_REQUIRED); + getVersionManager().setUpgradeState(FINALIZATION_REQUIRED); String msg = "Pre Finalization checks failed on the DataNode."; logAndEmit(msg); - return false; + throw new UpgradeException(msg, PREFINALIZE_VALIDATION_FAILED); + } + } + + private boolean canFinalizeDataNode(DatanodeStateMachine dsm) { + // Lets be sure that we do not have any open container before we return + // from here. This function should be called in its own finalizer thread + // context. + Iterator> containerIt = + dsm.getContainer().getController().getContainers(); + while (containerIt.hasNext()) { + Container ctr = containerIt.next(); + ContainerProtos.ContainerDataProto.State state = ctr.getContainerState(); + switch (state) { + case OPEN: + case CLOSING: + case UNHEALTHY: + LOG.warn("FinalizeUpgrade : Waiting for container to close, current " + + "state is: {}", state); + return false; + default: + continue; + } } return true; } @Override - public void postFinalizeUpgrade() throws IOException { - datanodeStateMachine.postFinalizeUpgrade(); + public void postFinalizeUpgrade(DatanodeStateMachine dsm) + throws UpgradeException { + // Do nothing here. } @Override - protected void finalizeUpgrade(Storage storageConfig) + public void finalizeUpgrade(DatanodeStateMachine dsm) throws UpgradeException { - for (HDDSLayoutFeature f : versionManager.unfinalizedFeatures()) { - Optional action = - f.datanodeAction(ON_FINALIZE); - finalizeFeature(f, datanodeStateMachine.getLayoutStorage(), action); - updateLayoutVersionInVersionFile(f, - datanodeStateMachine.getLayoutStorage()); - versionManager.finalized(f); - } - versionManager.completeFinalization(); + super.finalizeUpgrade(dsm::getLayoutStorage); } @Override diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java index d88745b1cd07..0977026ad0d4 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java @@ -33,7 +33,6 @@ import org.apache.hadoop.hdds.annotation.InterfaceAudience; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.conf.OzoneConfiguration; -import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeState; import org.apache.hadoop.hdds.scm.PipelineChoosePolicy; @@ -50,7 +49,6 @@ import org.apache.hadoop.hdds.scm.container.ContainerActionsHandler; import org.apache.hadoop.hdds.scm.container.ContainerID; import org.apache.hadoop.hdds.scm.container.ContainerInfo; -import org.apache.hadoop.hdds.scm.container.ContainerNotFoundException; import org.apache.hadoop.hdds.scm.container.ContainerManagerImpl; import org.apache.hadoop.hdds.scm.container.ContainerManagerV2; import org.apache.hadoop.hdds.scm.container.ContainerReportHandler; @@ -88,8 +86,6 @@ import org.apache.hadoop.hdds.scm.node.ReadOnlyHealthyToHealthyNodeHandler; import org.apache.hadoop.hdds.scm.node.SCMNodeManager; import org.apache.hadoop.hdds.scm.node.StaleNodeHandler; -import org.apache.hadoop.hdds.scm.node.states.NodeNotFoundException; -import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.node.NodeDecommissionManager; import org.apache.hadoop.hdds.scm.pipeline.PipelineActionHandler; import org.apache.hadoop.hdds.scm.pipeline.PipelineManager; @@ -150,7 +146,6 @@ import java.util.concurrent.TimeUnit; import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_SCM_WATCHER_TIMEOUT_DEFAULT; -import static org.apache.hadoop.hdds.scm.pipeline.Pipeline.PipelineState.CLOSED; import static org.apache.hadoop.hdds.utils.HAUtils.checkSecurityAndSCMHAEnabled; import static org.apache.hadoop.hdds.security.x509.certificate.authority.CertificateStore.CertType.VALID_CERTS; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ADMINISTRATORS_WILDCARD; @@ -1637,64 +1632,6 @@ public HDDSLayoutVersionManager getLayoutVersionManager() { return scmLayoutVersionManager; } - private void waitForAllContainersToClose() { - boolean containersFound = true; - while (containersFound) { - containersFound = false; - for (DatanodeDetails datanodeDetails : scmNodeManager.getAllNodes()) { - try { - for (ContainerID id : scmNodeManager.getContainers(datanodeDetails)) { - try { - final ContainerInfo container = containerManager.getContainer(id); - if (container.getState() == HddsProtos.LifeCycleState.OPEN || - container.getState() == HddsProtos.LifeCycleState.CLOSING) { - containersFound = true; - if (container.getState() == HddsProtos.LifeCycleState.OPEN) { - eventQueue.fireEvent(SCMEvents.CLOSE_CONTAINER, id); - } - } - } catch (ContainerNotFoundException cnfe) { - LOG.warn("Container {} is not managed by ContainerManager.", - id, cnfe); - continue; - } - } - } catch (NodeNotFoundException e) { - continue; - } - } - try { - if (containersFound) { - LOG.info("Waiting for all containers to close."); - Thread.sleep(5000); - } - } catch (InterruptedException e) { - continue; - } - } - } - - private void waitForAllPipelinesToDestroy() throws IOException { - boolean pipelineFound = true; - while (pipelineFound) { - pipelineFound = false; - for (Pipeline pipeline : pipelineManager.getPipelines()) { - if (pipeline.getPipelineState() != CLOSED) { - pipelineFound = true; - pipelineManager.closePipeline(pipeline, false); - } - } - try { - if (pipelineFound) { - LOG.info("Waiting for all pipelines to close."); - Thread.sleep(5000); - } - } catch (InterruptedException e) { - continue; - } - } - } - /** * Return the node Id of this SCM. * @return node Id. @@ -1703,71 +1640,6 @@ public String getSCMNodeId() { return scmHANodeDetails.getLocalNodeDetails().getNodeId(); } - // This should be called in the context of a separate finalize upgrade thread. - // This function can block indefinitely till the conditions are met to safely - // finalize Upgrade. - - public void preFinalizeUpgrade() throws IOException { - /** - * Ask pipeline manager to not create any new pipelines. Pipeline - * creation will remain frozen until postFinalizeUpgrade(). - */ - pipelineManager.freezePipelineCreation(); - - /** - * Ask all the existing data nodes to close any open containers and - * destroy existing pipelines - */ - waitForAllPipelinesToDestroy(); - - /** - * We can not yet move all the existing data nodes to HEALTHY-READONLY - * state since the next heartbeat will move them back to HEALTHY state. - * This has to wait till postFinalizeUpgrade, when SCM MLV version is - * already upgraded as part of finalize processing. - * While in this state, it should be safe to do finalize processing for - * all new features. This will also update ondisk mlv version. Any - * disrupting upgrade can add a hook here to make sure that SCM is in a - * consistent state while finalizing the upgrade. - */ - } - - public void postFinalizeUpgrade() { - /** - * Don't wait for next heartbeat from datanodes in order to move them to - * Healthy-Readonly state. Force them to Healthy-ReadOnly state so that - * we can resume pipeline creation right away. - */ - scmNodeManager.forceNodesToHealthyReadOnly(); - - /** - * Allow pipeline manager to create any new pipelines if it can - * find enough Healthy data nodes. - */ - pipelineManager.resumePipelineCreation(); - - // Wait for at least one pipeline to be created before finishing - // finalization, so clients can write. - boolean hasPipeline = false; - while (!hasPipeline) { - int pipelineCount = pipelineManager.getPipelines( - HddsProtos.ReplicationType.RATIS, HddsProtos.ReplicationFactor.THREE, - Pipeline.PipelineState.OPEN).size(); - - hasPipeline = (pipelineCount >= 1); - if (!hasPipeline) { - LOG.info("Waiting for at least one pipeline after SCM finalization."); - try { - Thread.sleep(5000); - } catch (InterruptedException e) { - // Try again on next loop iteration. - } - } else { - LOG.info("Pipeline found after SCM finalization"); - } - } - } - public StatusAndMessages finalizeUpgrade(String upgradeClientID) throws IOException{ return upgradeFinalizer.finalize(upgradeClientID, this); diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/SCMUpgradeFinalizer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/SCMUpgradeFinalizer.java index 1c8a3e4eb435..ef325f416250 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/SCMUpgradeFinalizer.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/SCMUpgradeFinalizer.java @@ -18,19 +18,21 @@ package org.apache.hadoop.hdds.scm.server.upgrade; -import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.ON_FINALIZE; -import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED; +import static org.apache.hadoop.hdds.scm.pipeline.Pipeline.PipelineState.CLOSED; import java.io.IOException; -import java.util.Optional; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.scm.pipeline.Pipeline; +import org.apache.hadoop.hdds.scm.pipeline.PipelineManager; import org.apache.hadoop.hdds.scm.server.StorageContainerManager; import org.apache.hadoop.hdds.upgrade.HDDSLayoutFeature; import org.apache.hadoop.hdds.upgrade.HDDSLayoutVersionManager; import org.apache.hadoop.ozone.common.Storage; import org.apache.hadoop.ozone.upgrade.BasicUpgradeFinalizer; -import org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeAction; import org.apache.hadoop.ozone.upgrade.UpgradeException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * UpgradeFinalizer for the Storage Container Manager service. @@ -38,7 +40,8 @@ public class SCMUpgradeFinalizer extends BasicUpgradeFinalizer { - private StorageContainerManager storageContainerManager; + private static final Logger LOG = + LoggerFactory.getLogger(SCMUpgradeFinalizer.class); public SCMUpgradeFinalizer(HDDSLayoutVersionManager versionManager) { super(versionManager); @@ -48,22 +51,15 @@ public SCMUpgradeFinalizer(HDDSLayoutVersionManager versionManager) { public StatusAndMessages finalize(String upgradeClientID, StorageContainerManager scm) throws IOException { - storageContainerManager = scm; - StatusAndMessages response = preFinalize(upgradeClientID, scm); - if (response.status() != FINALIZATION_REQUIRED) { - return response; - } - try { - getFinalizationExecutor().execute(scm.getScmStorageConfig(), this); - } catch (Exception e) { - e.printStackTrace(); - throw new IOException(e.getMessage()); - } - return STARTING_MSG; + return super.finalize(upgradeClientID, scm); } + // This should be called in the context of a separate finalize upgrade thread. + // This function can block indefinitely till the conditions are met to safely + // finalize Upgrade. @Override - public boolean preFinalizeUpgrade() throws IOException { + public void preFinalizeUpgrade(StorageContainerManager scm) + throws IOException { /* * Before we can call finalize the feature, we need to make sure that * all existing pipelines are closed and pipeline Manger would freeze @@ -74,25 +70,77 @@ public boolean preFinalizeUpgrade() throws IOException { msg += "\n New pipelines creation will remain frozen until Upgrade " + "is finalized."; - storageContainerManager.preFinalizeUpgrade(); + PipelineManager pipelineManager = scm.getPipelineManager(); + /** + * Ask pipeline manager to not create any new pipelines. Pipeline + * creation will remain frozen until postFinalizeUpgrade(). + */ + pipelineManager.freezePipelineCreation(); + + /** + * Ask all the existing data nodes to close any open containers and + * destroy existing pipelines + */ + waitForAllPipelinesToDestroy(pipelineManager); + + /** + * We can not yet move all the existing data nodes to HEALTHY-READONLY + * state since the next heartbeat will move them back to HEALTHY state. + * This has to wait till postFinalizeUpgrade, when SCM MLV version is + * already upgraded as part of finalize processing. + * While in this state, it should be safe to do finalize processing for + * all new features. This will also update ondisk mlv version. Any + * disrupting upgrade can add a hook here to make sure that SCM is in a + * consistent state while finalizing the upgrade. + */ + logAndEmit(msg); - return true; } @Override - protected void finalizeUpgrade(Storage storageConfig) + public void finalizeUpgrade(StorageContainerManager scm) throws UpgradeException { - for (HDDSLayoutFeature f : versionManager.unfinalizedFeatures()) { - Optional action = f.scmAction(ON_FINALIZE); - finalizeFeature(f, storageConfig, action); - updateLayoutVersionInVersionFile(f, storageConfig); - versionManager.finalized(f); - } - versionManager.completeFinalization(); + super.finalizeUpgrade(scm::getScmStorageConfig); } - public void postFinalizeUpgrade() throws IOException { - storageContainerManager.postFinalizeUpgrade(); + public void postFinalizeUpgrade(StorageContainerManager scm) + throws IOException { + /** + * Don't wait for next heartbeat from datanodes in order to move them to + * Healthy-Readonly state. Force them to Healthy-ReadOnly state so that + * we can resume pipeline creation right away. + */ + scm.getScmNodeManager().forceNodesToHealthyReadOnly(); + + /** + * Allow pipeline manager to create any new pipelines if it can + * find enough Healthy data nodes. + */ + PipelineManager pipelineManager = + scm.getPipelineManager(); + + pipelineManager.resumePipelineCreation(); + + // Wait for at least one pipeline to be created before finishing + // finalization, so clients can write. + boolean hasPipeline = false; + while (!hasPipeline) { + int pipelineCount = pipelineManager.getPipelines( + HddsProtos.ReplicationType.RATIS, HddsProtos.ReplicationFactor.THREE, + Pipeline.PipelineState.OPEN).size(); + + hasPipeline = (pipelineCount >= 1); + if (!hasPipeline) { + LOG.info("Waiting for at least one pipeline after SCM finalization."); + try { + Thread.sleep(5000); + } catch (InterruptedException e) { + // Try again on next loop iteration. + } + } else { + LOG.info("Pipeline found after SCM finalization"); + } + } emitFinishedMsg(); } @@ -100,7 +148,29 @@ public void postFinalizeUpgrade() throws IOException { public void runPrefinalizeStateActions(Storage storage, StorageContainerManager scm) throws IOException { - super.runPrefinalizeStateActions( + runPrefinalizeStateActions( lf -> ((HDDSLayoutFeature) lf)::scmAction, storage, scm); } + + private void waitForAllPipelinesToDestroy(PipelineManager pipelineManager) + throws IOException { + boolean pipelineFound = true; + while (pipelineFound) { + pipelineFound = false; + for (Pipeline pipeline : pipelineManager.getPipelines()) { + if (pipeline.getPipelineState() != CLOSED) { + pipelineFound = true; + pipelineManager.closePipeline(pipeline, false); + } + } + try { + if (pipelineFound) { + LOG.info("Waiting for all pipelines to close."); + Thread.sleep(5000); + } + } catch (InterruptedException e) { + continue; + } + } + } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java index 2b2ef7ec2835..a7208edd005b 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java @@ -18,19 +18,15 @@ package org.apache.hadoop.ozone.om.upgrade; -import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.ON_FINALIZE; import static org.apache.hadoop.ozone.OzoneConsts.LAYOUT_VERSION_KEY; -import static org.apache.hadoop.ozone.upgrade.UpgradeFinalizer.Status.FINALIZATION_REQUIRED; import org.apache.hadoop.ozone.common.Storage; import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OzoneManager; import java.io.IOException; -import java.util.Optional; import org.apache.hadoop.ozone.upgrade.BasicUpgradeFinalizer; -import org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeAction; import org.apache.hadoop.ozone.upgrade.UpgradeException; /** @@ -38,8 +34,6 @@ */ public class OMUpgradeFinalizer extends BasicUpgradeFinalizer { - private static final OmUpgradeAction NOOP = a -> {}; - private OzoneManager ozoneManager; public OMUpgradeFinalizer(OMLayoutVersionManager versionManager) { super(versionManager); @@ -48,51 +42,22 @@ public OMUpgradeFinalizer(OMLayoutVersionManager versionManager) { @Override public StatusAndMessages finalize(String upgradeClientID, OzoneManager om) throws IOException { - ozoneManager = om; - StatusAndMessages response = preFinalize(upgradeClientID, om); - if (response.status() != FINALIZATION_REQUIRED) { - return response; - } - // This requires some more investigation on how to do it properly while - // requests are on the fly, and post finalize features one by one. - // Until that is done, monitoring is not really doing anything meaningful - // but this is a tradoff we can take for the first iteration either if - // needed, as the finalization of the first few features should not take - // that long. Follow up JIRA is in HDDS-4286 - // String threadName = "OzoneManager-Upgrade-Finalizer"; - // ExecutorService executor = - // Executors.newSingleThreadExecutor(r -> new Thread(threadName)); - // executor.submit(new Worker(om)); - try { - getFinalizationExecutor().execute(ozoneManager.getOmStorage(), - this); - } catch (Exception e) { - e.printStackTrace(); - throw (IOException) e; - } - return STARTING_MSG; + return super.finalize(upgradeClientID, om); } @Override - protected void postFinalizeUpgrade() throws IOException { + public void postFinalizeUpgrade(OzoneManager om) { return; } @Override - protected void finalizeUpgrade(Storage storageConfig) + public void finalizeUpgrade(OzoneManager om) throws UpgradeException { - for (OMLayoutFeature f : versionManager.unfinalizedFeatures()) { - Optional action = f.action(ON_FINALIZE); - finalizeFeature(f, storageConfig, action); - updateLayoutVersionInVersionFile(f, storageConfig); - versionManager.finalized(f); - } - versionManager.completeFinalization(); + super.finalizeUpgrade(om::getOmStorage); } @Override - protected boolean preFinalizeUpgrade() throws IOException { - return true; + public void preFinalizeUpgrade(OzoneManager om) { } public void runPrefinalizeStateActions(Storage storage, OzoneManager om) @@ -107,6 +72,7 @@ public void runPrefinalizeStateActions(Storage storage, OzoneManager om) * @param om OM instance * @throws IOException on Error. */ + @Override public void updateLayoutVersionInDB(OMLayoutVersionManager lvm, OzoneManager om) throws IOException { 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 89521ed1792e..e7cc1295b726 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 @@ -78,7 +78,8 @@ public void testOMLayoutVersionManagerInitError() { } @Test - public void testOMLayoutFeatureCatalog() throws Exception { + public void testOMLayoutFeaturesHaveIncreasingLayoutVersion() + throws Exception { OMLayoutFeature[] values = OMLayoutFeature.values(); int currVersion = Integer.MIN_VALUE; OMLayoutFeature lastFeature = null; diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/PrepareSubCommand.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/PrepareSubCommand.java index e8e9dfb27a84..f6e3308bb15c 100644 --- a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/PrepareSubCommand.java +++ b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/admin/om/PrepareSubCommand.java @@ -27,6 +27,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.Callable; +import java.util.stream.Collectors; import org.apache.hadoop.hdds.cli.HddsVersionProvider; import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol; @@ -146,12 +147,22 @@ public Void call() throws Exception { Thread.sleep(pInterval.toMillis()); } } - if (currentNumPreparedOms < expectedNumPreparedOms) { - throw new Exception("OM Preparation failed since all OMs are not " + - "prepared yet."); + if (currentNumPreparedOms < (expectedNumPreparedOms + 1) / 2) { + throw new Exception("OM Preparation failed since a majority OMs are not" + + " prepared yet."); } else { System.out.println(); System.out.println("OM Preparation successful! "); + if (currentNumPreparedOms == expectedNumPreparedOms) { + System.out.println("All OMs are prepared"); + } else { + System.out.println("A majority of OMs are prepared. OMs that are not " + + "prepared : " + + omPreparedStatusMap.entrySet().stream() + .filter(e -> !e.getValue()) + .map(e -> e.getKey()) + .collect(Collectors.joining())); + } System.out.println("No new write requests will be allowed until " + "preparation is cancelled or upgrade/downgrade is done."); } From d5629e1dc1645457a33c3dddb2128de371d69f8a Mon Sep 17 00:00:00 2001 From: Aravindan Vijayan Date: Mon, 19 Apr 2021 15:16:52 -0700 Subject: [PATCH 2/8] Fix tests. --- .../ozone/upgrade/BasicUpgradeFinalizer.java | 18 +++++++++++------- .../DefaultUpgradeFinalizationExecutor.java | 3 ++- .../hadoop/ozone/upgrade/LayoutFeature.java | 4 ++-- .../InjectedUpgradeFinalizationExecutor.java | 2 ++ ...TestDefaultUpgradeFinalizationExecutor.java | 2 -- .../upgrade/TestUpgradeFinalizerActions.java | 6 +++--- .../statemachine/DatanodeStateMachine.java | 2 +- .../upgrade/DataNodeUpgradeFinalizer.java | 6 ------ .../docs/content/design/upgrade-dev-primer.md | 2 +- .../hadoop/hdds/upgrade/TestHDDSUpgrade.java | 1 + .../ozone/om/upgrade/OMUpgradeFinalizer.java | 9 --------- .../ozone/om/upgrade/TestOMVersionManager.java | 8 ++++---- 12 files changed, 27 insertions(+), 36 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java index 7fec8d4f623a..95734da7cdf1 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java @@ -20,7 +20,7 @@ import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.ON_FINALIZE; import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.ON_FIRST_UPGRADE_START; -import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.UNFINALIZED_STATE_VALIDATION; +import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.VALIDATE_IN_PREFINALIZE; import static org.apache.hadoop.ozone.upgrade.UpgradeException.ResultCodes.FIRST_UPGRADE_START_ACTION_FAILED; import static org.apache.hadoop.ozone.upgrade.UpgradeException.ResultCodes.INVALID_REQUEST; import static org.apache.hadoop.ozone.upgrade.UpgradeException.ResultCodes.LAYOUT_FEATURE_FINALIZATION_FAILED; @@ -70,7 +70,7 @@ public BasicUpgradeFinalizer(V versionManager) { public StatusAndMessages finalize(String upgradeClientID, T service) throws IOException { - StatusAndMessages response = preFinalize(upgradeClientID, service); + StatusAndMessages response = initFinalize(upgradeClientID, service); if (response.status() != FINALIZATION_REQUIRED) { return response; } @@ -92,9 +92,13 @@ public synchronized StatusAndMessages reportStatus( return new StatusAndMessages(status, returningMsgs); } - protected abstract void preFinalizeUpgrade(T service) throws IOException; + protected void preFinalizeUpgrade(T service) throws IOException { + // No Op by default. + }; - protected abstract void postFinalizeUpgrade(T service) throws IOException; + protected void postFinalizeUpgrade(T service) throws IOException { + // No Op by default. + } public abstract void finalizeUpgrade(T service) throws UpgradeException; @@ -146,7 +150,7 @@ public V getVersionManager() { return versionManager; } - private synchronized StatusAndMessages preFinalize( + private synchronized StatusAndMessages initFinalize( String upgradeClientID, T id) throws UpgradeException { switch (versionManager.getUpgradeState()) { case STARTING_FINALIZATION: @@ -247,7 +251,7 @@ protected void runPrefinalizeStateActions(Function> function = aFunction.apply(lf); Optional action = - function.apply(UNFINALIZED_STATE_VALIDATION); + function.apply(VALIDATE_IN_PREFINALIZE); if (action.isPresent()) { runValidationAction(lf, action.get()); } @@ -459,6 +463,6 @@ public void setFinalizationExecutor(DefaultUpgradeFinalizationExecutor } protected void updateLayoutVersionInDB(V vm, T comp) throws IOException { - throw new UnsupportedOperationException(); + // No-Op by default. } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java index 3cfb9c4e39fa..a1456b14b858 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java @@ -52,7 +52,6 @@ public Void execute(T component, BasicUpgradeFinalizer finalizer) finalizer.postFinalizeUpgrade(component); finalizer.emitFinishedMsg(); - finalizer.markFinalizationDone(); return null; } catch (Exception e) { LOG.warn("Upgrade Finalization failed with following Exception:"); @@ -61,6 +60,8 @@ public Void execute(T component, BasicUpgradeFinalizer finalizer) .setUpgradeState(FINALIZATION_REQUIRED); throw (e); } + } finally { + finalizer.markFinalizationDone(); } return null; } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/LayoutFeature.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/LayoutFeature.java index f857d3e5790e..52704c92d2e6 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/LayoutFeature.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/LayoutFeature.java @@ -53,8 +53,8 @@ default String name() { */ enum UpgradeActionType { - // Run every time an unfinalized component is started up. - UNFINALIZED_STATE_VALIDATION, + // Run every time an un-finalized component is started up. + VALIDATE_IN_PREFINALIZE, // Run exactly once when an upgraded cluster is detected with this new // layout version. diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java index afe177a54416..0bc173d52a34 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java @@ -92,6 +92,8 @@ public Void execute(T component, BasicUpgradeFinalizer finalizer) finalizer.getVersionManager() .setUpgradeState(FINALIZATION_REQUIRED); } + } finally { + finalizer.markFinalizationDone(); } return null; } diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestDefaultUpgradeFinalizationExecutor.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestDefaultUpgradeFinalizationExecutor.java index 6db707aa1043..e4a20f2c46e1 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestDefaultUpgradeFinalizationExecutor.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestDefaultUpgradeFinalizationExecutor.java @@ -18,7 +18,6 @@ package org.apache.hadoop.ozone.upgrade; -import static org.junit.Assert.assertFalse; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -96,6 +95,5 @@ public void runPrefinalizeStateActions(Storage storage, DefaultUpgradeFinalizationExecutor executor = new DefaultUpgradeFinalizationExecutor(); executor.execute(new Object(), uf); - assertFalse(uf.isFinalizationDone()); } } \ No newline at end of file diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestUpgradeFinalizerActions.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestUpgradeFinalizerActions.java index a72aaf74b7fa..eaabedbcf2de 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestUpgradeFinalizerActions.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestUpgradeFinalizerActions.java @@ -20,7 +20,7 @@ import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.NodeType.SCM; import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.ON_FIRST_UPGRADE_START; -import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.UNFINALIZED_STATE_VALIDATION; +import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.VALIDATE_IN_PREFINALIZE; import static org.apache.hadoop.ozone.upgrade.TestUpgradeFinalizerActions.MockLayoutFeature.VERSION_2; import static org.apache.hadoop.ozone.upgrade.TestUpgradeFinalizerActions.MockLayoutFeature.VERSION_3; import static org.junit.Assert.assertTrue; @@ -56,7 +56,7 @@ public class TestUpgradeFinalizerActions { @Test public void testRunPrefinalizeStateActions() throws IOException { - VERSION_2.addAction(UNFINALIZED_STATE_VALIDATION, + VERSION_2.addAction(VALIDATE_IN_PREFINALIZE, new MockScmUpgradeAction()); VERSION_3.addAction(ON_FIRST_UPGRADE_START, new MockDnUpgradeAction()); MockLayoutVersionManager lvm = new MockLayoutVersionManager(1); @@ -89,7 +89,7 @@ public void testRunPrefinalizeStateActions() throws IOException { @Test public void testValidationFailureWorks() throws Exception { - VERSION_2.addAction(UNFINALIZED_STATE_VALIDATION, + VERSION_2.addAction(VALIDATE_IN_PREFINALIZE, new MockFailingUpgradeAction()); MockLayoutVersionManager lvm = new MockLayoutVersionManager(1); MockUpgradeFinalizer uF = new MockUpgradeFinalizer(lvm); diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java index 352117569c18..95f3bace86ac 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeStateMachine.java @@ -607,7 +607,7 @@ public StatusAndMessages finalizeUpgrade() public StatusAndMessages queryUpgradeStatus() throws IOException{ return upgradeFinalizer.reportStatus(datanodeDetails.getUuidString(), - false); + true); } public UpgradeFinalizer getUpgradeFinalizer() { return upgradeFinalizer; diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeFinalizer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeFinalizer.java index e5f40d693a7d..3692320fe4f3 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeFinalizer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeFinalizer.java @@ -90,12 +90,6 @@ private boolean canFinalizeDataNode(DatanodeStateMachine dsm) { return true; } - @Override - public void postFinalizeUpgrade(DatanodeStateMachine dsm) - throws UpgradeException { - // Do nothing here. - } - @Override public void finalizeUpgrade(DatanodeStateMachine dsm) throws UpgradeException { diff --git a/hadoop-hdds/docs/content/design/upgrade-dev-primer.md b/hadoop-hdds/docs/content/design/upgrade-dev-primer.md index de160eaf5279..63ea45923337 100644 --- a/hadoop-hdds/docs/content/design/upgrade-dev-primer.md +++ b/hadoop-hdds/docs/content/design/upgrade-dev-primer.md @@ -56,7 +56,7 @@ A typical use case during upgrade is to have multiple versions of a class / meth ## Upgrade Action (UpgradeActionOm & UpgradeActionHdds) Annotation to specify upgrade action run during specific upgrade phases. Each layout feature can optionally define an upgrade action for every supported phase. These are the supported phases of action callbacks. -#### UNFINALIZED_STATE_VALIDATION +#### VALIDATE_IN_PREFINALIZE A ‘validation’ action run every time a component is started up with this layout feature being unfinalized. #### ON_FIRST_UPGRADE_START diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSUpgrade.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSUpgrade.java index 5c79d7ad5b06..6f4e8cfd862b 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSUpgrade.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSUpgrade.java @@ -286,6 +286,7 @@ private void testPostUpgradeConditionsDataNodes() { return false; } } catch (IOException e) { + LOG.error("Exception. ", e); return false; } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java index a7208edd005b..dd39c6c42f69 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java @@ -45,21 +45,12 @@ public StatusAndMessages finalize(String upgradeClientID, OzoneManager om) return super.finalize(upgradeClientID, om); } - @Override - public void postFinalizeUpgrade(OzoneManager om) { - return; - } - @Override public void finalizeUpgrade(OzoneManager om) throws UpgradeException { super.finalizeUpgrade(om::getOmStorage); } - @Override - public void preFinalizeUpgrade(OzoneManager om) { - } - public void runPrefinalizeStateActions(Storage storage, OzoneManager om) throws IOException { super.runPrefinalizeStateActions( 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 e7cc1295b726..5c329791cb89 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 @@ -23,7 +23,7 @@ import static org.apache.hadoop.ozone.om.upgrade.OMLayoutVersionManager.OM_REQUEST_CLASS_PACKAGE; import static org.apache.hadoop.ozone.om.upgrade.OMLayoutVersionManager.OM_UPGRADE_CLASS_PACKAGE; import static org.apache.hadoop.ozone.om.upgrade.OMLayoutVersionManager.getRequestClasses; -import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.UNFINALIZED_STATE_VALIDATION; +import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.VALIDATE_IN_PREFINALIZE; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -139,7 +139,7 @@ public void testOmUpgradeActionsRegistered() throws Exception { // INITIAL_VERSION is finalized, hence should not register. Optional action = - INITIAL_VERSION.action(UNFINALIZED_STATE_VALIDATION); + INITIAL_VERSION.action(VALIDATE_IN_PREFINALIZE); assertFalse(action.isPresent()); lvm = mock(OMLayoutVersionManager.class); @@ -147,7 +147,7 @@ public void testOmUpgradeActionsRegistered() throws Exception { doCallRealMethod().when(lvm).registerUpgradeActions(anyString()); lvm.registerUpgradeActions(OM_UPGRADE_CLASS_PACKAGE); - action = INITIAL_VERSION.action(UNFINALIZED_STATE_VALIDATION); + action = INITIAL_VERSION.action(VALIDATE_IN_PREFINALIZE); Assert.assertTrue(action.isPresent()); Assert.assertEquals(MockOmUpgradeAction.class, action.get().getClass()); OzoneManager omMock = mock(OzoneManager.class); @@ -155,7 +155,7 @@ public void testOmUpgradeActionsRegistered() throws Exception { verify(omMock, times(1)).getVersion(); } - @UpgradeActionOm(type = UNFINALIZED_STATE_VALIDATION, feature = + @UpgradeActionOm(type = VALIDATE_IN_PREFINALIZE, feature = INITIAL_VERSION) public static class MockOmUpgradeAction implements OmUpgradeAction { From 7a5c1943d836ea249dfd81126ce1f80c3d9bf927 Mon Sep 17 00:00:00 2001 From: Aravindan Vijayan Date: Mon, 19 Apr 2021 15:24:51 -0700 Subject: [PATCH 3/8] Fix OM finalization integration test. --- .../apache/hadoop/ozone/om/TestOMUpgradeFinalization.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMUpgradeFinalization.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMUpgradeFinalization.java index f278986168f3..9d85fa794255 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMUpgradeFinalization.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOMUpgradeFinalization.java @@ -37,7 +37,7 @@ import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.ozone.MiniOzoneCluster; -import org.apache.hadoop.ozone.MiniOzoneHAClusterImpl; +import org.apache.hadoop.ozone.MiniOzoneOMHAClusterImpl; import org.apache.hadoop.ozone.client.ObjectStore; import org.apache.hadoop.ozone.client.OzoneClientFactory; import org.apache.hadoop.ozone.client.protocol.ClientProtocol; @@ -67,7 +67,7 @@ public class TestOMUpgradeFinalization { */ @Rule public Timeout timeout = new Timeout(300000); - private MiniOzoneHAClusterImpl cluster; + private MiniOzoneOMHAClusterImpl cluster; private OzoneManager ozoneManager; private ClientProtocol clientProtocol; private int fromLayoutVersion; @@ -100,7 +100,7 @@ public void setup() throws Exception { OzoneConfiguration conf = new OzoneConfiguration(); String omServiceId = UUID.randomUUID().toString(); - cluster = (MiniOzoneHAClusterImpl) MiniOzoneCluster.newHABuilder(conf) + cluster = (MiniOzoneOMHAClusterImpl) MiniOzoneCluster.newOMHABuilder(conf) .setClusterId(UUID.randomUUID().toString()) .setScmId(UUID.randomUUID().toString()) .setOMServiceId(omServiceId) From c5e43d13aeefef894b6a69e43a13cf19a89d7315 Mon Sep 17 00:00:00 2001 From: Aravindan Vijayan Date: Mon, 19 Apr 2021 16:05:34 -0700 Subject: [PATCH 4/8] Fix compile failure. --- .../ScmHAUnfinalizedStateValidationAction.java | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/ScmHAUnfinalizedStateValidationAction.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/ScmHAUnfinalizedStateValidationAction.java index 2c28a8ff8980..3df1ffe29402 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/ScmHAUnfinalizedStateValidationAction.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/ScmHAUnfinalizedStateValidationAction.java @@ -19,7 +19,7 @@ package org.apache.hadoop.hdds.scm.server.upgrade; import static org.apache.hadoop.hdds.upgrade.HDDSLayoutFeature.SCM_HA; -import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.UNFINALIZED_STATE_VALIDATION; +import static org.apache.hadoop.ozone.upgrade.LayoutFeature.UpgradeActionType.VALIDATE_IN_PREFINALIZE; import static org.apache.hadoop.ozone.upgrade.UpgradeActionHdds.Component.SCM; import org.apache.hadoop.hdds.scm.ScmConfigKeys; @@ -27,17 +27,12 @@ import org.apache.hadoop.hdds.upgrade.HDDSUpgradeAction; import org.apache.hadoop.ozone.upgrade.UpgradeActionHdds; import org.apache.hadoop.ozone.upgrade.UpgradeException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; @UpgradeActionHdds(feature = SCM_HA, component = SCM, - type = UNFINALIZED_STATE_VALIDATION) + type = VALIDATE_IN_PREFINALIZE) public class ScmHAUnfinalizedStateValidationAction implements HDDSUpgradeAction { - private static final Logger LOG = - LoggerFactory.getLogger(ScmHAUnfinalizedStateValidationAction.class); - @Override public void execute(StorageContainerManager scm) throws Exception { boolean isHAEnabled = @@ -48,7 +43,7 @@ public void execute(StorageContainerManager scm) throws Exception { throw new UpgradeException(String.format("Configuration %s cannot be " + "used until SCM upgrade has been finalized", ScmConfigKeys.OZONE_SCM_HA_ENABLE_KEY), - UpgradeException.ResultCodes.PREFINALIZE_STATE_VALIDATION_FAILED); + UpgradeException.ResultCodes.PREFINALIZE_ACTION_VALIDATION_FAILED); } } } From da7a1a12833a1921e5191fd9121f4c74e757ceaa Mon Sep 17 00:00:00 2001 From: Aravindan Vijayan Date: Wed, 21 Apr 2021 13:13:19 -0700 Subject: [PATCH 5/8] Address review comments. --- .../ozone/upgrade/BasicUpgradeFinalizer.java | 13 ++++--------- .../DefaultUpgradeFinalizationExecutor.java | 2 +- .../upgrade/TestHDDSLayoutVersionManager.java | 4 ++-- .../InjectedUpgradeFinalizationExecutor.java | 3 +-- .../apache/hadoop/ozone/om/OzoneManager.java | 16 ++++++++++++++++ .../ozone/om/upgrade/OMLayoutFeature.java | 3 ++- .../ozone/om/upgrade/OMUpgradeFinalizer.java | 18 ------------------ 7 files changed, 26 insertions(+), 33 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java index 95734da7cdf1..c4fdd5a2291b 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/BasicUpgradeFinalizer.java @@ -94,7 +94,7 @@ public synchronized StatusAndMessages reportStatus( protected void preFinalizeUpgrade(T service) throws IOException { // No Op by default. - }; + } protected void postFinalizeUpgrade(T service) throws IOException { // No Op by default. @@ -131,10 +131,9 @@ public void finalizeAndWaitForCompletion( } } if (!success) { - LOG.error("Unable to finalize after waiting for {} seconds", - maxTimeToWaitInSeconds); - } else { - updateLayoutVersionInDB(versionManager, service); + throw new IOException( + String.format("Unable to finalize after waiting for %d seconds", + maxTimeToWaitInSeconds)); } } @@ -461,8 +460,4 @@ public void setFinalizationExecutor(DefaultUpgradeFinalizationExecutor executor) { finalizationExecutor = executor; } - - protected void updateLayoutVersionInDB(V vm, T comp) throws IOException { - // No-Op by default. - } } diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java index a1456b14b858..bcc69ce5333a 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java @@ -54,7 +54,7 @@ public Void execute(T component, BasicUpgradeFinalizer finalizer) finalizer.emitFinishedMsg(); return null; } catch (Exception e) { - LOG.warn("Upgrade Finalization failed with following Exception:"); + LOG.warn("Upgrade Finalization failed with following Exception. ", e); if (finalizer.getVersionManager().needsFinalization()) { finalizer.getVersionManager() .setUpgradeState(FINALIZATION_REQUIRED); diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSLayoutVersionManager.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSLayoutVersionManager.java index 06683b2a170d..cfca7179e191 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSLayoutVersionManager.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSLayoutVersionManager.java @@ -93,9 +93,9 @@ public void testUpgradeActionsRegistered() throws Exception { @Test public void testHDDSLayoutFeaturesHaveIncreasingLayoutVersion() { HDDSLayoutFeature[] values = HDDSLayoutFeature.values(); - int currVersion = Integer.MIN_VALUE; + int currVersion = -1; for (HDDSLayoutFeature lf : values) { - assertTrue(currVersion < lf.layoutVersion()); + assertEquals(currVersion + 1, lf.layoutVersion()); currVersion = lf.layoutVersion(); } } diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java index 0bc173d52a34..56f5ac7894bd 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java @@ -86,8 +86,7 @@ public Void execute(T component, BasicUpgradeFinalizer finalizer) finalizer.markFinalizationDone(); return null; } catch (Exception e) { - LOG.warn("Upgrade Finalization failed with following Exception:"); - e.printStackTrace(); + LOG.warn("Upgrade Finalization failed with following Exception.", e); if (finalizer.getVersionManager().needsFinalization()) { finalizer.getVersionManager() .setUpgradeState(FINALIZATION_REQUIRED); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java index 93b4d06c0151..9a6d7414fc01 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java @@ -559,6 +559,8 @@ private void instantiateServices(boolean withNewSnapshot) throws IOException { if (versionManager.getMetadataLayoutVersion() < layoutVersionInDB) { throw new IOException("Unable to finalize OM to the desired layout " + "version " + layoutVersionInDB + " present in the snapshot DB."); + } else { + updateLayoutVersionInDB(versionManager, metadataManager); } } } @@ -3892,4 +3894,18 @@ public int getMinMultipartUploadPartSize() { public void setMinMultipartUploadPartSize(int partSizeForTest) { this.minMultipartUploadPartSize = partSizeForTest; } + + + /** + * Write down Layout version of a finalized feature to DB on finalization. + * @param lvm OMLayoutVersionManager + * @param omMetadataManager omMetadataManager instance + * @throws IOException on Error. + */ + private void updateLayoutVersionInDB(OMLayoutVersionManager lvm, + OMMetadataManager omMetadataManager) + throws IOException { + omMetadataManager.getMetaTable().put(LAYOUT_VERSION_KEY, + String.valueOf(lvm.getMetadataLayoutVersion())); + } } diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java index 5338c168bcd1..f30b02cc41b9 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java @@ -29,7 +29,8 @@ */ public enum OMLayoutFeature implements LayoutFeature { ////////////////////////////// ////////////////////////////// - INITIAL_VERSION(0, "Initial Layout Version"); + INITIAL_VERSION(0, "Initial Layout Version"), + FSO(1, ""); /////////////////////////////// ///////////////////////////// diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java index dd39c6c42f69..86e490f2fab4 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java @@ -18,10 +18,7 @@ package org.apache.hadoop.ozone.om.upgrade; -import static org.apache.hadoop.ozone.OzoneConsts.LAYOUT_VERSION_KEY; - import org.apache.hadoop.ozone.common.Storage; -import org.apache.hadoop.ozone.om.OMMetadataManager; import org.apache.hadoop.ozone.om.OzoneManager; import java.io.IOException; @@ -56,19 +53,4 @@ public void runPrefinalizeStateActions(Storage storage, OzoneManager om) super.runPrefinalizeStateActions( lf -> ((OMLayoutFeature) lf)::action, storage, om); } - - /** - * Write down Layout version of a finalized feature to DB on finalization. - * @param f layout feature - * @param om OM instance - * @throws IOException on Error. - */ - @Override - public void updateLayoutVersionInDB(OMLayoutVersionManager lvm, - OzoneManager om) - throws IOException { - OMMetadataManager omMetadataManager = om.getMetadataManager(); - omMetadataManager.getMetaTable().put(LAYOUT_VERSION_KEY, - String.valueOf(lvm.getMetadataLayoutVersion())); - } } From 82c2d81849cbcd33a2494a3da2403a94ff0addc2 Mon Sep 17 00:00:00 2001 From: Aravindan Vijayan Date: Wed, 21 Apr 2021 14:55:16 -0700 Subject: [PATCH 6/8] Remove layout feature. --- .../org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java index f30b02cc41b9..5338c168bcd1 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMLayoutFeature.java @@ -29,8 +29,7 @@ */ public enum OMLayoutFeature implements LayoutFeature { ////////////////////////////// ////////////////////////////// - INITIAL_VERSION(0, "Initial Layout Version"), - FSO(1, ""); + INITIAL_VERSION(0, "Initial Layout Version"); /////////////////////////////// ///////////////////////////// From cb3f0cf49351193dc6b10fe735cfdca2da1bfe58 Mon Sep 17 00:00:00 2001 From: Aravindan Vijayan Date: Wed, 21 Apr 2021 22:18:01 -0700 Subject: [PATCH 7/8] Fix review comments. --- .../DefaultUpgradeFinalizationExecutor.java | 4 +- .../ozone/upgrade/UpgradeException.java | 4 -- .../upgrade/TestHDDSLayoutVersionManager.java | 2 +- .../InjectedUpgradeFinalizationExecutor.java | 5 +- ...estDefaultUpgradeFinalizationExecutor.java | 2 +- .../upgrade/DataNodeUpgradeFinalizer.java | 11 ---- .../hdds/scm/pipeline/PipelineManager.java | 6 ++ .../server/upgrade/SCMUpgradeFinalizer.java | 62 +++++++------------ .../hadoop/hdds/upgrade/TestHDDSUpgrade.java | 26 ++++---- .../ozone/om/upgrade/OMUpgradeFinalizer.java | 6 -- 10 files changed, 44 insertions(+), 84 deletions(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java index bcc69ce5333a..ed09d727b86b 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java @@ -38,7 +38,7 @@ public class DefaultUpgradeFinalizationExecutor { public DefaultUpgradeFinalizationExecutor() { } - public Void execute(T component, BasicUpgradeFinalizer finalizer) + public void execute(T component, BasicUpgradeFinalizer finalizer) throws IOException { try { finalizer.emitStartingMsg(); @@ -52,7 +52,6 @@ public Void execute(T component, BasicUpgradeFinalizer finalizer) finalizer.postFinalizeUpgrade(component); finalizer.emitFinishedMsg(); - return null; } catch (Exception e) { LOG.warn("Upgrade Finalization failed with following Exception. ", e); if (finalizer.getVersionManager().needsFinalization()) { @@ -63,6 +62,5 @@ public Void execute(T component, BasicUpgradeFinalizer finalizer) } finally { finalizer.markFinalizationDone(); } - return null; } } \ No newline at end of file diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/UpgradeException.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/UpgradeException.java index 816c33364190..dfc43efbe284 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/UpgradeException.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/UpgradeException.java @@ -103,18 +103,14 @@ public String toString() { * Error codes to make it easy to decode these exceptions. */ public enum ResultCodes { - OK, - INVALID_REQUEST, - PERSIST_UPGRADE_TO_LAYOUT_VERSION_FAILED, REMOVE_UPGRADE_TO_LAYOUT_VERSION_FAILED, UPDATE_LAYOUT_VERSION_FAILED, LAYOUT_FEATURE_FINALIZATION_FAILED, PREFINALIZE_ACTION_VALIDATION_FAILED, FIRST_UPGRADE_START_ACTION_FAILED, - PREFINALIZE_VALIDATION_FAILED; } } diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSLayoutVersionManager.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSLayoutVersionManager.java index cfca7179e191..4a09f3fa9456 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSLayoutVersionManager.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSLayoutVersionManager.java @@ -99,4 +99,4 @@ public void testHDDSLayoutFeaturesHaveIncreasingLayoutVersion() { currVersion = lf.layoutVersion(); } } -} \ No newline at end of file +} diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java index 56f5ac7894bd..2a9637d86ec2 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/InjectedUpgradeFinalizationExecutor.java @@ -65,7 +65,7 @@ static class UpgradeTestInjectionAbort extends Exception { } @Override - public Void execute(T component, BasicUpgradeFinalizer finalizer) + public void execute(T component, BasicUpgradeFinalizer finalizer) throws IOException { try { injectTestFunctionAtThisPoint(BEFORE_PRE_FINALIZE_UPGRADE); @@ -83,8 +83,6 @@ public Void execute(T component, BasicUpgradeFinalizer finalizer) injectTestFunctionAtThisPoint(AFTER_POST_FINALIZE_UPGRADE); finalizer.emitFinishedMsg(); - finalizer.markFinalizationDone(); - return null; } catch (Exception e) { LOG.warn("Upgrade Finalization failed with following Exception.", e); if (finalizer.getVersionManager().needsFinalization()) { @@ -94,7 +92,6 @@ public Void execute(T component, BasicUpgradeFinalizer finalizer) } finally { finalizer.markFinalizationDone(); } - return null; } /** diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestDefaultUpgradeFinalizationExecutor.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestDefaultUpgradeFinalizationExecutor.java index e4a20f2c46e1..feca5d58410a 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestDefaultUpgradeFinalizationExecutor.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/ozone/upgrade/TestDefaultUpgradeFinalizationExecutor.java @@ -96,4 +96,4 @@ public void runPrefinalizeStateActions(Storage storage, new DefaultUpgradeFinalizationExecutor(); executor.execute(new Object(), uf); } -} \ No newline at end of file +} diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeFinalizer.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeFinalizer.java index 3692320fe4f3..f8abf7cd5728 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeFinalizer.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/upgrade/DataNodeUpgradeFinalizer.java @@ -32,28 +32,17 @@ import org.apache.hadoop.ozone.container.common.statemachine.DatanodeStateMachine; import org.apache.hadoop.ozone.upgrade.BasicUpgradeFinalizer; import org.apache.hadoop.ozone.upgrade.UpgradeException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * UpgradeFinalizer for the DataNode. */ public class DataNodeUpgradeFinalizer extends BasicUpgradeFinalizer { - private static final Logger LOG = - LoggerFactory.getLogger(DataNodeUpgradeFinalizer.class); public DataNodeUpgradeFinalizer(HDDSLayoutVersionManager versionManager) { super(versionManager); } - @Override - public StatusAndMessages finalize(String upgradeClientID, - DatanodeStateMachine dsm) - throws IOException { - return super.finalize(upgradeClientID, dsm); - } - @Override public void preFinalizeUpgrade(DatanodeStateMachine dsm) throws IOException { diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManager.java index 263ed0e73a1c..7ebfc3694cdd 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManager.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManager.java @@ -130,7 +130,13 @@ default void waitPipelineReady(PipelineID pipelineID, long timeout) void reinitialize(Table pipelineStore) throws IOException; + /** + * Ask pipeline manager to not create any new pipelines. + */ void freezePipelineCreation(); + /** + * Ask pipeline manager to resume creating new pipelines. + */ void resumePipelineCreation(); } \ No newline at end of file diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/SCMUpgradeFinalizer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/SCMUpgradeFinalizer.java index ef325f416250..39ad1a415b52 100644 --- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/SCMUpgradeFinalizer.java +++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/SCMUpgradeFinalizer.java @@ -31,8 +31,6 @@ import org.apache.hadoop.ozone.common.Storage; import org.apache.hadoop.ozone.upgrade.BasicUpgradeFinalizer; import org.apache.hadoop.ozone.upgrade.UpgradeException; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * UpgradeFinalizer for the Storage Container Manager service. @@ -40,20 +38,10 @@ public class SCMUpgradeFinalizer extends BasicUpgradeFinalizer { - private static final Logger LOG = - LoggerFactory.getLogger(SCMUpgradeFinalizer.class); - public SCMUpgradeFinalizer(HDDSLayoutVersionManager versionManager) { super(versionManager); } - @Override - public StatusAndMessages finalize(String upgradeClientID, - StorageContainerManager scm) - throws IOException { - return super.finalize(upgradeClientID, scm); - } - // This should be called in the context of a separate finalize upgrade thread. // This function can block indefinitely till the conditions are met to safely // finalize Upgrade. @@ -71,28 +59,21 @@ public void preFinalizeUpgrade(StorageContainerManager scm) "is finalized."; PipelineManager pipelineManager = scm.getPipelineManager(); - /** - * Ask pipeline manager to not create any new pipelines. Pipeline - * creation will remain frozen until postFinalizeUpgrade(). - */ + + // Pipeline creation will remain frozen until postFinalizeUpgrade() pipelineManager.freezePipelineCreation(); - /** - * Ask all the existing data nodes to close any open containers and - * destroy existing pipelines - */ waitForAllPipelinesToDestroy(pipelineManager); - /** - * We can not yet move all the existing data nodes to HEALTHY-READONLY - * state since the next heartbeat will move them back to HEALTHY state. - * This has to wait till postFinalizeUpgrade, when SCM MLV version is - * already upgraded as part of finalize processing. - * While in this state, it should be safe to do finalize processing for - * all new features. This will also update ondisk mlv version. Any - * disrupting upgrade can add a hook here to make sure that SCM is in a - * consistent state while finalizing the upgrade. - */ + + // We can not yet move all the existing data nodes to HEALTHY-READONLY + // state since the next heartbeat will move them back to HEALTHY state. + // This has to wait till postFinalizeUpgrade, when SCM MLV version is + // already upgraded as part of finalize processing. + // While in this state, it should be safe to do finalize processing for + // all new features. This will also update ondisk mlv version. Any + // disrupting upgrade can add a hook here to make sure that SCM is in a + // consistent state while finalizing the upgrade. logAndEmit(msg); } @@ -105,19 +86,14 @@ public void finalizeUpgrade(StorageContainerManager scm) public void postFinalizeUpgrade(StorageContainerManager scm) throws IOException { - /** - * Don't wait for next heartbeat from datanodes in order to move them to - * Healthy-Readonly state. Force them to Healthy-ReadOnly state so that - * we can resume pipeline creation right away. - */ + + + // Don 't wait for next heartbeat from datanodes in order to move them to + // Healthy - Readonly state. Force them to Healthy ReadOnly state so that + // we can resume pipeline creation right away. scm.getScmNodeManager().forceNodesToHealthyReadOnly(); - /** - * Allow pipeline manager to create any new pipelines if it can - * find enough Healthy data nodes. - */ - PipelineManager pipelineManager = - scm.getPipelineManager(); + PipelineManager pipelineManager = scm.getPipelineManager(); pipelineManager.resumePipelineCreation(); @@ -152,6 +128,10 @@ public void runPrefinalizeStateActions(Storage storage, lf -> ((HDDSLayoutFeature) lf)::scmAction, storage, scm); } + /** + * Ask all the existing data nodes to close any open containers and + * destroy existing pipelines. + */ private void waitForAllPipelinesToDestroy(PipelineManager pipelineManager) throws IOException { boolean pipelineFound = true; diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSUpgrade.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSUpgrade.java index 6f4e8cfd862b..608b04b79655 100644 --- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSUpgrade.java +++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/hdds/upgrade/TestHDDSUpgrade.java @@ -628,7 +628,7 @@ public void testScmFailuresBeforeScmPreFinalizeUpgrade() () -> { return this.injectSCMFailureDuringSCMUpgrade(); }); - testFinalizationWithFailuerInjectionHelper(null); + testFinalizationWithFailureInjectionHelper(null); Assert.assertTrue(testPassed.get()); } @@ -653,7 +653,7 @@ public void testScmFailuresAfterScmPreFinalizeUpgrade() () -> { return this.injectSCMFailureDuringSCMUpgrade(); }); - testFinalizationWithFailuerInjectionHelper(null); + testFinalizationWithFailureInjectionHelper(null); Assert.assertTrue(testPassed.get()); } @@ -678,7 +678,7 @@ public void testScmFailuresAfterScmCompleteFinalization() () -> { return this.injectSCMFailureDuringSCMUpgrade(); }); - testFinalizationWithFailuerInjectionHelper(null); + testFinalizationWithFailureInjectionHelper(null); Assert.assertTrue(testPassed.get()); } @@ -703,7 +703,7 @@ public void testScmFailuresAfterScmPostFinalizeUpgrade() () -> { return this.injectSCMFailureDuringSCMUpgrade(); }); - testFinalizationWithFailuerInjectionHelper(null); + testFinalizationWithFailureInjectionHelper(null); Assert.assertTrue(testPassed.get()); } @@ -728,7 +728,7 @@ public void testAllDataNodeFailuresBeforeScmPreFinalizeUpgrade() () -> { return injectDataNodeFailureDuringSCMUpgrade(); }); - testFinalizationWithFailuerInjectionHelper(null); + testFinalizationWithFailureInjectionHelper(null); Assert.assertTrue(testPassed.get()); } @@ -753,7 +753,7 @@ public void testAllDataNodeFailuresAfterScmPreFinalizeUpgrade() () -> { return injectDataNodeFailureDuringSCMUpgrade(); }); - testFinalizationWithFailuerInjectionHelper(null); + testFinalizationWithFailureInjectionHelper(null); Assert.assertTrue(testPassed.get()); } @@ -778,7 +778,7 @@ public void testAllDataNodeFailuresAfterScmCompleteFinalization() () -> { return injectDataNodeFailureDuringSCMUpgrade(); }); - testFinalizationWithFailuerInjectionHelper(null); + testFinalizationWithFailureInjectionHelper(null); Assert.assertTrue(testPassed.get()); } @@ -803,7 +803,7 @@ public void testAllDataNodeFailuresAfterScmPostFinalizeUpgrade() () -> { return injectDataNodeFailureDuringSCMUpgrade(); }); - testFinalizationWithFailuerInjectionHelper(null); + testFinalizationWithFailureInjectionHelper(null); Assert.assertTrue(testPassed.get()); } @@ -839,7 +839,7 @@ public void testDataNodeFailuresDuringDataNodeUpgrade() ((BasicUpgradeFinalizer)ds.getDatanodeStateMachine() .getUpgradeFinalizer()) .setFinalizationExecutor(dataNodeFinalizationExecutor); - testFinalizationWithFailuerInjectionHelper(failureInjectionThread); + testFinalizationWithFailureInjectionHelper(failureInjectionThread); Assert.assertTrue(testPassed.get()); synchronized (cluster) { shutdown(); @@ -896,7 +896,7 @@ public void testAllPossibleDataNodeFailuresAndSCMFailures() ((BasicUpgradeFinalizer)ds.getDatanodeStateMachine() .getUpgradeFinalizer()) .setFinalizationExecutor(dataNodeFinalizationExecutor); - testFinalizationWithFailuerInjectionHelper( + testFinalizationWithFailureInjectionHelper( dataNodefailureInjectionThread); Assert.assertTrue(testPassed.get()); synchronized (cluster) { @@ -939,7 +939,7 @@ public void testDataNodeAndSCMFailuresTogetherDuringSCMUpgrade() }); ((BasicUpgradeFinalizer)scm.getUpgradeFinalizer()) .setFinalizationExecutor(scmFinalizationExecutor); - testFinalizationWithFailuerInjectionHelper(helpingFailureInjectionThread); + testFinalizationWithFailureInjectionHelper(helpingFailureInjectionThread); Assert.assertTrue(testPassed.get()); synchronized (cluster) { shutdown(); @@ -980,7 +980,7 @@ public void testDataNodeAndSCMFailuresTogetherDuringDataNodeUpgrade() ((BasicUpgradeFinalizer)ds.getDatanodeStateMachine() .getUpgradeFinalizer()) .setFinalizationExecutor(dataNodeFinalizationExecutor); - testFinalizationWithFailuerInjectionHelper(helpingFailureInjectionThread); + testFinalizationWithFailureInjectionHelper(helpingFailureInjectionThread); Assert.assertTrue(testPassed.get()); synchronized (cluster) { shutdown(); @@ -991,7 +991,7 @@ public void testDataNodeAndSCMFailuresTogetherDuringDataNodeUpgrade() } } - public void testFinalizationWithFailuerInjectionHelper( + public void testFinalizationWithFailureInjectionHelper( Thread failureInjectionThread) throws Exception { waitForPipelineCreated(); diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java index 86e490f2fab4..abbcbab1af8f 100644 --- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java +++ b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/upgrade/OMUpgradeFinalizer.java @@ -36,12 +36,6 @@ public OMUpgradeFinalizer(OMLayoutVersionManager versionManager) { super(versionManager); } - @Override - public StatusAndMessages finalize(String upgradeClientID, OzoneManager om) - throws IOException { - return super.finalize(upgradeClientID, om); - } - @Override public void finalizeUpgrade(OzoneManager om) throws UpgradeException { From aa1dce906da6fcea6604e8ffc9171bc2da368a86 Mon Sep 17 00:00:00 2001 From: Aravindan Vijayan Date: Thu, 22 Apr 2021 10:07:45 -0700 Subject: [PATCH 8/8] Add newline. --- .../ozone/upgrade/DefaultUpgradeFinalizationExecutor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java index ed09d727b86b..19c2c4c3e6e6 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/upgrade/DefaultUpgradeFinalizationExecutor.java @@ -63,4 +63,4 @@ public void execute(T component, BasicUpgradeFinalizer finalizer) finalizer.markFinalizationDone(); } } -} \ No newline at end of file +}