HDDS-8555. [Snapshot] When snapshot feature is disabled, block OM startup if there are still snapshots in the system

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotDisabledRestart.java

@@ -0,0 +1,116 @@
+/**
+ * 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
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+
+package org.apache.hadoop.ozone.om;
+
+import org.apache.commons.lang3.RandomStringUtils;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.utils.IOUtils;
+import org.apache.hadoop.ozone.MiniOzoneCluster;
+import org.apache.hadoop.ozone.MiniOzoneHAClusterImpl;
+import org.apache.hadoop.ozone.client.ObjectStore;
+import org.apache.hadoop.ozone.client.OzoneClient;
+import org.apache.hadoop.ozone.client.OzoneVolume;
+import org.apache.ozone.test.LambdaTestUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Disabled;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Timeout;
+
+import java.util.UUID;
+
+/**
+ * Integration test to verify that if snapshot feature is disabled, OM start up
+ * will fail when there are still snapshots remaining.
+ */
+@Disabled("HDDS-8945")
+public class TestOmSnapshotDisabledRestart {
+
+  private static MiniOzoneHAClusterImpl cluster = null;
+  private static OzoneClient client;
+  private static ObjectStore store;
+
+  @BeforeAll
+  @Timeout(60)
+  public static void init() throws Exception {
+    OzoneConfiguration conf = new OzoneConfiguration();
+    String clusterId = UUID.randomUUID().toString();
+    String scmId = UUID.randomUUID().toString();
+
+    // Enable filesystem snapshot feature at the beginning
+    conf.setBoolean(OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
+
+    cluster = (MiniOzoneHAClusterImpl) MiniOzoneCluster.newOMHABuilder(conf)
+        .setClusterId(clusterId)
+        .setScmId(scmId)
+        .setOMServiceId("om-service-test2")
+        .setNumOfOzoneManagers(3)
+        .build();
+    cluster.waitForClusterToBeReady();
+    client = cluster.newClient();
+
+    OzoneManager leaderOzoneManager = cluster.getOMLeader();
+    OzoneConfiguration leaderConfig = leaderOzoneManager.getConfiguration();
+    cluster.setConf(leaderConfig);
+    store = client.getObjectStore();
+  }
+
+  @AfterAll
+  public static void tearDown() throws Exception {
+    IOUtils.closeQuietly(client);
+    if (cluster != null) {
+      cluster.shutdown();
+    }
+  }
+
+  @Test
+  public void testSnapshotFeatureFlag() throws Exception {
+    // Verify that OM start up will indeed fail when there are still snapshots
+    // while snapshot feature is disabled.
+
+    String volumeName = "vol-" + RandomStringUtils.randomNumeric(5);
+    String bucketName = "buck-" + RandomStringUtils.randomNumeric(5);
+    String snapshotName = "snap-" + RandomStringUtils.randomNumeric(5);
+
+    store.createVolume(volumeName);
+    OzoneVolume volume = store.getVolume(volumeName);
+    volume.createBucket(bucketName);
+    // Create a snapshot
+    store.createSnapshot(volumeName, bucketName, snapshotName);
+
+    cluster.getOzoneManagersList().forEach(om -> {
+      try {
+        cluster.shutdownOzoneManager(om);
+        // Disable snapshot feature
+        om.getConfiguration().setBoolean(
+            OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, false);
+        // Restart OM, expect OM start up failure
+        LambdaTestUtils.intercept(RuntimeException.class, "snapshots remaining",
+            () -> cluster.restartOzoneManager(om, true));
+        // Enable snapshot feature again
+        om.getConfiguration().setBoolean(
+            OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY, true);
+        // Should pass this time
+        cluster.restartOzoneManager(om, true);
+      } catch (Exception e) {
+        Assertions.fail("Failed to restart OM", e);
+      }
+    });
+  }
+}

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java

@@ -60,8 +60,6 @@
 import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
 import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo;
 import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
-import org.apache.hadoop.ozone.om.request.key.OMDirectoriesPurgeRequestWithFSO;
-import org.apache.hadoop.ozone.om.request.key.OMKeyPurgeRequest;
 import org.apache.hadoop.ozone.om.service.SnapshotDiffCleanupService;
 import org.apache.hadoop.ozone.om.snapshot.SnapshotDiffObject;
 import org.apache.hadoop.ozone.om.helpers.SnapshotDiffJob;
@@ -170,17 +168,25 @@ public final class OmSnapshotManager implements AutoCloseable {
   // Soft limit of the snapshot cache size.
   private final int softCacheSize;
 
-  /**
-   * TODO: [SNAPSHOT] HDDS-8529: Refactor the constructor in a way that when
-   *  ozoneManager.isFilesystemSnapshotEnabled() returns false,
-   *  no snapshot-related background job or initialization would run,
-   *  except for applying previously committed Ratis transactions in e.g.:
-   *  1. {@link OMKeyPurgeRequest#validateAndUpdateCache}
-   *  2. {@link OMDirectoriesPurgeRequestWithFSO#validateAndUpdateCache}
-   */
   public OmSnapshotManager(OzoneManager ozoneManager) {
+
+    boolean isFilesystemSnapshotEnabled =
+        ozoneManager.isFilesystemSnapshotEnabled();
     LOG.info("Ozone filesystem snapshot feature is {}.",
-        ozoneManager.isFilesystemSnapshotEnabled() ? "enabled" : "disabled");
+        isFilesystemSnapshotEnabled ? "enabled" : "disabled");
+
+    // Confirm that snapshot feature can be safely disabled.
+    // Throw unchecked exception if that is not the case.
+    if (!isFilesystemSnapshotEnabled &&
+        !canDisableFsSnapshot(ozoneManager.getMetadataManager())) {
+      throw new RuntimeException("Ozone Manager is refusing to start up" +
+          "because filesystem snapshot feature is disabled in config while" +
+          "there are still snapshots remaining in the system (including the " +
+          "ones that are marked as deleted but not yet cleaned up by the " +
+          "background worker thread). " +
+          "Please set config ozone.filesystem.snapshot.enabled to true and " +
+          "try to start this Ozone Manager again.");
+    }
 
     this.options = new ManagedDBOptions();
     this.options.setCreateIfMissing(true);
@@ -302,6 +308,27 @@ public OmSnapshotManager(OzoneManager ozoneManager) {
     }
   }
 
+  /**
+   * Help reject OM startup if snapshot feature is disabled
+   * but there are snapshots remaining in this OM. Note: snapshots that are
+   * already deleted but not cleaned up yet still counts.
+   * @param ommm OMMetadataManager
+   * @return true if SnapshotInfoTable is empty, false otherwise.
+   */
+  @VisibleForTesting
+  public boolean canDisableFsSnapshot(OMMetadataManager ommm) {
+
+    boolean isSnapshotInfoTableEmpty;
+    try {
+      isSnapshotInfoTableEmpty = ommm.getSnapshotInfoTable().isEmpty();
+    } catch (IOException e) {
+      throw new IllegalStateException(
+          "Unable to check SnapshotInfoTable emptiness", e);
+    }
+
+    return isSnapshotInfoTableEmpty;
+  }
+
   private CacheLoader<String, OmSnapshot> createCacheLoader() {
     return new CacheLoader<String, OmSnapshot>() {
 

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java

@@ -441,7 +441,7 @@ public final class OzoneManager extends ServiceRuntimeInfoImpl
   private final OzoneLockProvider ozoneLockProvider;
   private final OMPerformanceMetrics perfMetrics;
 
-  private final boolean fsSnapshotEnabled;
+  private boolean fsSnapshotEnabled;
 
   /**
    * OM Startup mode.
@@ -553,10 +553,6 @@ private OzoneManager(OzoneConfiguration conf, StartupOption startupOption)
     // Ratis server comes with JvmPauseMonitor, no need to start another
     jvmPauseMonitor = !isRatisEnabled ? newJvmPauseMonitor(omId) : null;
 
-    fsSnapshotEnabled = configuration.getBoolean(
-        OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY,
-        OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_DEFAULT);
-
     String defaultBucketLayoutString =
         configuration.getTrimmed(OZONE_DEFAULT_BUCKET_LAYOUT,
             OZONE_DEFAULT_BUCKET_LAYOUT_DEFAULT);
@@ -786,6 +782,7 @@ private void instantiateServices(boolean withNewSnapshot) throws IOException {
       multiTenantManager = new OMMultiTenantManagerImpl(this, configuration);
       OzoneAclUtils.setOMMultiTenantManager(multiTenantManager);
     }
+
     volumeManager = new VolumeManagerImpl(metadataManager);
 
     bucketManager = new BucketManagerImpl(this, metadataManager);
@@ -823,7 +820,10 @@ private void instantiateServices(boolean withNewSnapshot) throws IOException {
     omMetadataReader = new OmMetadataReader(keyManager, prefixManager,
         this, LOG, AUDIT, metrics);
 
-    // TODO: [SNAPSHOT] Revisit this in HDDS-8529.
+    // Reload snapshot feature config flag
+    fsSnapshotEnabled = configuration.getBoolean(
+        OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_KEY,
+        OMConfigKeys.OZONE_FILESYSTEM_SNAPSHOT_ENABLED_DEFAULT);
     omSnapshotManager = new OmSnapshotManager(this);
 
     // Snapshot metrics

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java

@@ -115,6 +115,24 @@ public void cleanup() throws Exception {
     FileUtils.deleteDirectory(testDir);
   }
 
+  @Test
+  public void testSnapshotFeatureFlagSafetyCheck() throws IOException {
+    // Verify that the snapshot feature config safety check method
+    // is returning the expected value.
+
+    Table<String, SnapshotInfo> snapshotInfoTable = mock(Table.class);
+    HddsWhiteboxTestUtils.setInternalState(
+        om.getMetadataManager(), SNAPSHOT_INFO_TABLE, snapshotInfoTable);
+
+    when(snapshotInfoTable.isEmpty()).thenReturn(false);
+    Assert.assertFalse(om.getOmSnapshotManager()
+        .canDisableFsSnapshot(om.getMetadataManager()));
+
+    when(snapshotInfoTable.isEmpty()).thenReturn(true);
+    Assert.assertTrue(om.getOmSnapshotManager()
+        .canDisableFsSnapshot(om.getMetadataManager()));
+  }
+
   @Test
   public void testCloseOnEviction() throws IOException {