HDDS-11911. Return consistent error code when snapshot is not found i…

…n the DB or Snapshot Chain. (apache#7557)
aswinshakil · Dec 12, 2024 · eea5600 · eea5600
1 parent e8f3b25
commit eea5600
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 12 deletions.
diff --git a/...ne/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java b/...ne/integration-test/src/test/java/org/apache/hadoop/ozone/om/snapshot/TestOmSnapshot.java
@@ -127,6 +127,7 @@
 import static org.apache.hadoop.ozone.om.OmUpgradeConfig.ConfigStrings.OZONE_OM_INIT_DEFAULT_LAYOUT_VERSION;
 import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.BUCKET_NOT_FOUND;
 import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.CONTAINS_SNAPSHOT;
+import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.FILE_NOT_FOUND;
 import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
 import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.NOT_SUPPORTED_OPERATION_PRIOR_FINALIZATION;
 import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.VOLUME_NOT_FOUND;
@@ -1602,13 +1603,13 @@ public void testSnapDiffNoSnapshot() throws Exception {
     OMException omException = assertThrows(OMException.class,
         () -> store.snapshotDiff(volume, bucket, snap1, snap2,
             null, 0, false, disableNativeDiff));
-    assertEquals(KEY_NOT_FOUND, omException.getResult());
+    assertEquals(FILE_NOT_FOUND, omException.getResult());
     // From snapshot is invalid
     omException = assertThrows(OMException.class,
         () -> store.snapshotDiff(volume, bucket, snap2, snap1,
             null, 0, false, disableNativeDiff));
 
-    assertEquals(KEY_NOT_FOUND, omException.getResult());
+    assertEquals(FILE_NOT_FOUND, omException.getResult());
   }
 
   @Test
@@ -1699,12 +1700,12 @@ public void testSnapDiffMissingMandatoryParams() throws Exception {
     OMException omException = assertThrows(OMException.class,
         () -> store.snapshotDiff(volume, bucket, snap1, nullstr,
             null, 0, forceFullSnapshotDiff, disableNativeDiff));
-    assertEquals(KEY_NOT_FOUND, omException.getResult());
+    assertEquals(FILE_NOT_FOUND, omException.getResult());
     // From snapshot is empty
     omException = assertThrows(OMException.class,
         () -> store.snapshotDiff(volume, bucket, nullstr, snap1,
             null, 0, forceFullSnapshotDiff, disableNativeDiff));
-    assertEquals(KEY_NOT_FOUND, omException.getResult());
+    assertEquals(FILE_NOT_FOUND, omException.getResult());
     // Bucket is empty
     assertThrows(IllegalArgumentException.class,
         () -> store.snapshotDiff(volume, nullstr, snap1, snap2,

diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OmSnapshotManager.java
@@ -99,8 +99,8 @@
 import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_DIFF_DB_DIR;
 import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_DIFF_REPORT_MAX_PAGE_SIZE;
 import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_SNAPSHOT_DIFF_REPORT_MAX_PAGE_SIZE_DEFAULT;
+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;
-import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_SNAPSHOT_ERROR;
 import static org.apache.hadoop.ozone.om.snapshot.SnapshotDiffManager.getSnapshotRootPath;
 import static org.apache.hadoop.ozone.om.snapshot.SnapshotUtils.checkSnapshotActive;
 import static org.apache.hadoop.ozone.om.snapshot.SnapshotUtils.dropColumnFamilyHandle;
@@ -352,7 +352,8 @@ public OmSnapshot load(@Nonnull UUID snapshotId) throws IOException {
         // If it happens, then either snapshot has been purged in between or SnapshotChain is corrupted
         // and missing some entries which needs investigation.
         if (snapshotTableKey == null) {
-          throw new IOException("No snapshot exist with snapshotId: " + snapshotId);
+          throw new OMException("Snapshot " + snapshotId +
+              " is not found in the snapshot chain.", FILE_NOT_FOUND);
         }
 
         final SnapshotInfo snapshotInfo = getSnapshotInfo(snapshotTableKey);
@@ -745,7 +746,7 @@ private SnapshotInfo getSnapshotInfo(String snapshotKey) throws IOException {
       snapshotInfo = ozoneManager.getMetadataManager().getSnapshotInfoTable().getSkipCache(snapshotKey);
     }
     if (snapshotInfo == null) {
-      throw new OMException("Snapshot '" + snapshotKey + "' is not found.", INVALID_SNAPSHOT_ERROR);
+      throw new OMException("Snapshot '" + snapshotKey + "' is not found.", FILE_NOT_FOUND);
     }
     return snapshotInfo;
   }

diff --git a/...-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotUtils.java b/...-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/snapshot/SnapshotUtils.java
@@ -46,12 +46,10 @@
 import java.util.UUID;
 
 import static org.apache.hadoop.ozone.OzoneConsts.OM_KEY_PREFIX;
-import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.INVALID_SNAPSHOT_ERROR;
 import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.DIRECTORY_TABLE;
 import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.FILE_TABLE;
 import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.KEY_TABLE;
 import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.FILE_NOT_FOUND;
-import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.KEY_NOT_FOUND;
 import static org.apache.hadoop.ozone.om.exceptions.OMException.ResultCodes.TIMEOUT;
 
 /**
@@ -88,7 +86,7 @@ public static SnapshotInfo getSnapshotInfo(final OzoneManager ozoneManager,
     }
     if (snapshotInfo == null) {
       throw new OMException("Snapshot '" + snapshotKey + "' is not found.",
-          KEY_NOT_FOUND);
+          FILE_NOT_FOUND);
     }
     return snapshotInfo;
   }
@@ -164,7 +162,7 @@ public static SnapshotInfo getNextSnapshot(OzoneManager ozoneManager,
     // is removed in-memory but OMDoubleBuffer has not flushed yet.
     if (snapInfo == null) {
       throw new OMException("Provided Snapshot Info argument is null. Cannot get the next snapshot for a null value",
-          INVALID_SNAPSHOT_ERROR);
+          FILE_NOT_FOUND);
     }
     try {
       if (chainManager.hasNextPathSnapshot(snapInfo.getSnapshotPath(),
@@ -201,7 +199,7 @@ private static UUID getPreviousSnapshotId(SnapshotInfo snapInfo, SnapshotChainMa
     // is removed in-memory but OMDoubleBuffer has not flushed yet.
     if (snapInfo == null) {
       throw new OMException("Provided Snapshot Info argument is null. Cannot get the previous snapshot for a null " +
-          "value", INVALID_SNAPSHOT_ERROR);
+          "value", FILE_NOT_FOUND);
     }
     try {
       if (chainManager.hasPreviousPathSnapshot(snapInfo.getSnapshotPath(),

diff --git a/...p-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java b/...p-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/TestOmSnapshotManager.java
@@ -25,6 +25,7 @@
 import org.apache.hadoop.hdds.utils.db.DBStore;
 import org.apache.hadoop.hdds.utils.db.RDBBatchOperation;
 import org.apache.hadoop.hdds.utils.db.Table;
+import org.apache.hadoop.ozone.om.exceptions.OMException;
 import org.apache.hadoop.ozone.om.helpers.OmBucketInfo;
 import org.apache.hadoop.ozone.om.helpers.OmVolumeArgs;
 import org.apache.hadoop.ozone.om.helpers.SnapshotInfo;
@@ -72,6 +73,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.timeout;
 import static org.mockito.Mockito.times;
@@ -314,6 +316,29 @@ public void testHardLinkCreation() throws IOException {
         getINode(f1FileLink.toPath()), "link matches original file");
   }
 
+
+  @Test
+  public void testGetSnapshotInfo() throws IOException {
+    SnapshotInfo s1 = createSnapshotInfo("vol", "buck");
+    UUID latestGlobalSnapId =
+        ((OmMetadataManagerImpl) om.getMetadataManager()).getSnapshotChainManager()
+            .getLatestGlobalSnapshotId();
+    UUID latestPathSnapId =
+        ((OmMetadataManagerImpl) om.getMetadataManager()).getSnapshotChainManager()
+            .getLatestPathSnapshotId(String.join("/", "vol", "buck"));
+    s1.setPathPreviousSnapshotId(latestPathSnapId);
+    s1.setGlobalPreviousSnapshotId(latestGlobalSnapId);
+    ((OmMetadataManagerImpl) om.getMetadataManager()).getSnapshotChainManager()
+        .addSnapshot(s1);
+    OMException ome = assertThrows(OMException.class,
+        () -> om.getOmSnapshotManager().getSnapshot(s1.getSnapshotId()));
+    assertEquals(OMException.ResultCodes.FILE_NOT_FOUND, ome.getResult());
+    // not present in snapshot chain too
+    SnapshotInfo s2 = createSnapshotInfo("vol", "buck");
+    ome = assertThrows(OMException.class,
+        () -> om.getOmSnapshotManager().getSnapshot(s2.getSnapshotId()));
+    assertEquals(OMException.ResultCodes.FILE_NOT_FOUND, ome.getResult());
+  }
   /*
    * Test that exclude list is generated correctly.
    */