HDDS-7914. [Snapshot] Block FS API access to deleted (non-active) sna…

…pshots (#4419)

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

@@ -39,16 +39,19 @@
 import org.apache.hadoop.ozone.client.OzoneVolume;
 import org.apache.hadoop.ozone.client.io.OzoneInputStream;
 import org.apache.hadoop.ozone.client.io.OzoneOutputStream;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
 import org.apache.hadoop.ozone.om.helpers.BucketLayout;
 import org.apache.hadoop.ozone.om.helpers.OmKeyArgs;
 import org.apache.hadoop.ozone.om.helpers.OpenKeySession;
 import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
 import org.apache.hadoop.ozone.om.protocol.OzoneManagerProtocol;
 import org.apache.ozone.test.GenericTestUtils;
+import org.apache.ozone.test.LambdaTestUtils;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.jupiter.api.Assertions;
 import org.junit.rules.Timeout;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
@@ -134,11 +137,13 @@ public TestOmSnapshotFileSystem(BucketLayout newBucketLayout,
       init();
     }
   }
+
   private static void setConfig(BucketLayout newBucketLayout,
       boolean newEnableFileSystemPaths) {
     TestOmSnapshotFileSystem.enabledFileSystemPaths = newEnableFileSystemPaths;
     TestOmSnapshotFileSystem.bucketLayout = newBucketLayout;
   }
+
   /**
    * Create a MiniDFSCluster for testing.
    */
@@ -367,6 +372,60 @@ private static void setKeyPrefix(String s) {
     keyPrefix = s;
   }
 
+  @Test
+  public void testBlockSnapshotFSAccessAfterDeletion() throws Exception {
+    Path root = new Path("/");
+    Path dir = new Path(root, "/testListKeysBeforeAfterSnapshotDeletion");
+    Path key1 = new Path(dir, "key1");
+    Path key2 = new Path(dir, "key2");
+
+    // Create 2 keys
+    ContractTestUtils.touch(fs, key1);
+    ContractTestUtils.touch(fs, key2);
+
+    // Create a snapshot
+    String snapshotName = UUID.randomUUID().toString();
+    String snapshotKeyPrefix = createSnapshot(snapshotName);
+
+    // Can list keys in snapshot
+    Path snapshotRoot = new Path(snapshotKeyPrefix + root);
+    Path snapshotParent = new Path(snapshotKeyPrefix + dir);
+    // Check dir in snapshot
+    FileStatus[] fileStatuses = o3fs.listStatus(snapshotRoot);
+    Assertions.assertEquals(1, fileStatuses.length);
+    // List keys in dir in snapshot
+    fileStatuses = o3fs.listStatus(snapshotParent);
+    Assertions.assertEquals(2, fileStatuses.length);
+
+    // Check key metadata
+    Path snapshotKey1 = new Path(snapshotKeyPrefix + key1);
+    FileStatus fsActiveKey = o3fs.getFileStatus(key1);
+    FileStatus fsSnapshotKey = o3fs.getFileStatus(snapshotKey1);
+    Assert.assertEquals(fsActiveKey.getModificationTime(),
+        fsSnapshotKey.getModificationTime());
+
+    Path snapshotKey2 = new Path(snapshotKeyPrefix + key2);
+    fsActiveKey = o3fs.getFileStatus(key2);
+    fsSnapshotKey = o3fs.getFileStatus(snapshotKey2);
+    Assert.assertEquals(fsActiveKey.getModificationTime(),
+        fsSnapshotKey.getModificationTime());
+
+    // Delete the snapshot
+    deleteSnapshot(snapshotName);
+
+    // Can't access keys in snapshot anymore with FS API. Should throw exception
+    final String errorMsg = "no longer active";
+    LambdaTestUtils.intercept(OMException.class, errorMsg,
+        () -> o3fs.listStatus(snapshotRoot));
+    LambdaTestUtils.intercept(OMException.class, errorMsg,
+        () -> o3fs.listStatus(snapshotParent));
+
+    LambdaTestUtils.intercept(OMException.class, errorMsg,
+        () -> o3fs.getFileStatus(snapshotKey1));
+    LambdaTestUtils.intercept(OMException.class, errorMsg,
+        () -> o3fs.getFileStatus(snapshotKey2));
+  }
+
   @Test
   // based on TestOzoneFileSystem:testListStatus
   public void testListStatus() throws Exception {
@@ -524,7 +583,6 @@ private static void createAndCommitKey(String keyName) throws IOException {
     writeClient.commitKey(keyArgs, session.getId());
   }
 
-
   /**
    * Tests listStatus operation on root directory.
    */
@@ -637,9 +695,13 @@ public void deleteRootDir()
 
   private String createSnapshot()
       throws IOException, InterruptedException, TimeoutException {
+    return createSnapshot(UUID.randomUUID().toString());
+  }
+
+  private String createSnapshot(String snapshotName)
+      throws IOException, InterruptedException, TimeoutException {
 
     // create snapshot
-    String snapshotName = UUID.randomUUID().toString();
     writeClient.createSnapshot(volumeName, bucketName, snapshotName);
 
     // wait till the snapshot directory exists
@@ -654,4 +716,8 @@ private String createSnapshot()
 
     return OM_KEY_PREFIX + OmSnapshotManager.getSnapshotPrefix(snapshotName);
   }
+
+  private void deleteSnapshot(String snapshotName) throws IOException {
+    writeClient.deleteSnapshot(volumeName, bucketName, snapshotName);
+  }
 }

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

@@ -67,6 +67,7 @@ public class OmSnapshot implements IOmMetadataReader, Closeable {
   private final String volumeName;
   private final String bucketName;
   private final String snapshotName;
+  // To access snapshot checkpoint DB metadata
   private final OMMetadataManager omMetadataManager;
 
   public OmSnapshot(KeyManager keyManager,
@@ -84,7 +85,6 @@ public OmSnapshot(KeyManager keyManager,
     this.omMetadataManager = keyManager.getMetadataManager();
   }
 
-
   @Override
   public OmKeyInfo lookupKey(OmKeyArgs args) throws IOException {
     return denormalizeOmKeyInfo(omMetadataReader.lookupKey(

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

@@ -51,6 +51,7 @@
 import org.apache.hadoop.ozone.om.helpers.RepeatedOmKeyInfo;
 import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
 import org.apache.hadoop.ozone.om.helpers.SnapshotInfo.SnapshotStatus;
+import org.apache.hadoop.ozone.om.service.SnapshotDeletingService;
 import org.apache.hadoop.ozone.om.snapshot.SnapshotDiffManager;
 import org.apache.hadoop.ozone.om.snapshot.SnapshotUtils;
 import org.apache.hadoop.ozone.snapshot.SnapshotDiffResponse;
@@ -69,6 +70,7 @@
 import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_DIFF_DB_NAME;
 import static org.apache.hadoop.ozone.OzoneConsts.OM_SNAPSHOT_INDICATOR;
 import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_DIFF_DB_DIR;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.FILE_NOT_FOUND;
 import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_KEY_NAME;
 
 /**
@@ -174,6 +176,28 @@ public OmSnapshot load(@Nonnull String snapshotTableKey)
         // see if the snapshot exists
         snapshotInfo = getSnapshotInfo(snapshotTableKey);
 
+        // Block snapshot from loading when it is no longer active e.g. DELETED,
+        // unless this is called from SnapshotDeletingService.
+        // TODO: [SNAPSHOT] However, snapshotCache.get() from other requests
+        //  (not from SDS) would be able to piggyback off of this because
+        //  snapshot still in cache won't trigger loader again.
+        //  This needs proper addressal in e.g. HDDS-7935
+        //  by introducing another cache just for SDS.
+        //  While the snapshotCache would host ACTIVE snapshots only.
+        if (!snapshotInfo.getSnapshotStatus().equals(
+                SnapshotStatus.SNAPSHOT_ACTIVE)) {
+          if (isCalledFromSnapshotDeletingService()) {
+            LOG.debug("Permitting {} to load snapshot {} in status: {}",
+                SnapshotDeletingService.class.getSimpleName(),
+                snapshotInfo.getTableKey(),
+                snapshotInfo.getSnapshotStatus());
+          } else {
+            throw new OMException("Unable to load snapshot. " +
+                "Snapshot with table key '" + snapshotTableKey +
+                "' is no longer active", FILE_NOT_FOUND);
+          }
+        }
+
         CacheValue<SnapshotInfo> cacheValue =
             ozoneManager.getMetadataManager().getSnapshotInfoTable()
                 .getCacheValue(new CacheKey<>(snapshotTableKey));
@@ -214,6 +238,8 @@ public OmSnapshot load(@Nonnull String snapshotTableKey)
         = notification -> {
           try {
             // close snapshot's rocksdb on eviction
+            LOG.debug("Closing snapshot: {}", notification.getKey());
+            // TODO: [SNAPSHOT] HDDS-7935.Close only when refcount reaches zero?
             notification.getValue().close();
           } catch (IOException e) {
             LOG.error("Failed to close snapshot: {} {}",
@@ -232,6 +258,32 @@ public OmSnapshot load(@Nonnull String snapshotTableKey)
         columnFamilyOptions);
   }
 
+  /**
+   * Helper method to check whether the loader is called from
+   * SnapshotDeletingTask (return true) or not (return false).
+   */
+  private boolean isCalledFromSnapshotDeletingService() {
+
+    StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace();
+    for (StackTraceElement elem : stackTrace) {
+      // Allow as long as loader is called from SDS. e.g. SnapshotDeletingTask
+      if (elem.getClassName().startsWith(
+          SnapshotDeletingService.class.getName())) {
+        return true;
+      }
+    }
+
+    return false;
+  }
+
+  /**
+   * Get snapshot instance LRU cache.
+   * @return LoadingCache
+   */
+  public LoadingCache<String, OmSnapshot> getSnapshotCache() {
+    return snapshotCache;
+  }
+
   /**
    * Creates snapshot checkpoint that corresponds to snapshotInfo.
    * @param omMetadataManager the metadata manager

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotDeleteRequest.java

@@ -184,6 +184,11 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
       omClientResponse = new OMSnapshotDeleteResponse(
           omResponse.build(), tableKey, snapshotInfo);
 
+      // Evict the snapshot entry from cache, and close the snapshot DB
+      // Nothing happens if the key doesn't exist in cache (snapshot not loaded)
+      ozoneManager.getOmSnapshotManager().getSnapshotCache()
+          .invalidate(tableKey);
+
     } catch (IOException ex) {
       exception = ex;
       omClientResponse = new OMSnapshotDeleteResponse(

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

@@ -97,7 +97,7 @@ public void testCloseOnEviction() throws IOException {
     HddsWhiteboxTestUtils.setInternalState(
         firstSnapshot.getMetadataManager(), "store", firstSnapshotStore);
 
-    //  create second snapshot checkpoint (which will be used for eviction)
+    // create second snapshot checkpoint (which will be used for eviction)
     OmSnapshotManager.createOmSnapshotCheckpoint(om.getMetadataManager(),
         second);
 
@@ -109,7 +109,6 @@ public void testCloseOnEviction() throws IOException {
         .checkForSnapshot(second.getVolumeName(),
         second.getBucketName(), getSnapshotPrefix(second.getName()));
 
-
     // confirm store was closed
     verify(firstSnapshotStore, timeout(3000).times(1)).close();
   }

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotDeleteRequest.java

@@ -20,6 +20,7 @@
 
 package org.apache.hadoop.ozone.om.request.snapshot;
 
+import com.google.common.cache.LoadingCache;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.utils.db.cache.CacheKey;
 import org.apache.hadoop.hdds.utils.db.cache.CacheValue;
@@ -28,6 +29,7 @@
 import org.apache.hadoop.ozone.om.OMConfigKeys;
 import org.apache.hadoop.ozone.om.OMMetrics;
 import org.apache.hadoop.ozone.om.OmMetadataManagerImpl;
+import org.apache.hadoop.ozone.om.OmSnapshotManager;
 import org.apache.hadoop.ozone.om.OzoneManager;
 import org.apache.hadoop.ozone.om.exceptions.OMException;
 import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
@@ -98,6 +100,11 @@ public void setup() throws Exception {
     when(ozoneManager.getAuditLogger()).thenReturn(auditLogger);
     Mockito.doNothing().when(auditLogger).logWrite(any(AuditMessage.class));
 
+    OmSnapshotManager omSnapshotManager = mock(OmSnapshotManager.class);
+    when(omSnapshotManager.getSnapshotCache())
+        .thenReturn(mock(LoadingCache.class));
+    when(ozoneManager.getOmSnapshotManager()).thenReturn(omSnapshotManager);
+
     volumeName = UUID.randomUUID().toString();
     bucketName = UUID.randomUUID().toString();
     snapshotName = UUID.randomUUID().toString();