HDDS-11997. Duplicate snapshot purge request causes NPE (#7627)

apache · Dec 30, 2024 · ad108c8 · ad108c8
1 parent c3003fd
commit ad108c8
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 1 deletion.
diff --git a/...ger/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java b/...ger/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java
@@ -227,7 +227,9 @@ private SnapshotInfo getUpdatedSnapshotInfo(String snapshotTableKey, OMMetadataM
 
     if (snapshotInfo == null) {
       snapshotInfo = omMetadataManager.getSnapshotInfoTable().get(snapshotTableKey);
-      updatedSnapshotInfos.put(snapshotTableKey, snapshotInfo);
+      if (snapshotInfo != null) {
+        updatedSnapshotInfos.put(snapshotTableKey, snapshotInfo);
+      }
     }
     return snapshotInfo;
   }

diff --git a/...r/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotPurgeResponse.java b/...r/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotPurgeResponse.java
@@ -18,6 +18,7 @@
  */
 package org.apache.hadoop.ozone.om.response.snapshot;
 
+import com.google.common.annotations.VisibleForTesting;
 import org.apache.commons.io.FileUtils;
 import org.apache.hadoop.hdds.utils.db.BatchOperation;
 import org.apache.hadoop.ozone.om.OMMetadataManager;
@@ -138,4 +139,9 @@ private void deleteCheckpointDirectory(OMMetadataManager omMetadataManager,
       }
     }
   }
+
+  @VisibleForTesting
+  public Map<String, SnapshotInfo> getUpdatedSnapInfos() {
+    return updatedSnapInfos;
+  }
 }
diff --git a/...va/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotPurgeRequestAndResponse.java b/...va/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotPurgeRequestAndResponse.java
@@ -56,6 +56,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.anyString;
@@ -186,6 +187,42 @@ public void testValidateAndUpdateCache() throws Exception {
     assertEquals(initialSnapshotPurgeFailCount, getOmMetrics().getNumSnapshotPurgeFails());
   }
 
+  @Test
+  public void testDuplicateSnapshotPurge() throws Exception {
+    List<String> snapshotDbKeysToPurge = createSnapshots(1);
+    assertFalse(getOmMetadataManager().getSnapshotInfoTable().isEmpty());
+    OMRequest snapshotPurgeRequest = createPurgeKeysRequest(
+        snapshotDbKeysToPurge);
+
+    OMSnapshotPurgeRequest omSnapshotPurgeRequest = preExecute(snapshotPurgeRequest);
+
+    OMSnapshotPurgeResponse omSnapshotPurgeResponse = (OMSnapshotPurgeResponse)
+        omSnapshotPurgeRequest.validateAndUpdateCache(getOzoneManager(), 200L);
+
+    try (BatchOperation batchOperation = getOmMetadataManager().getStore().initBatchOperation()) {
+      omSnapshotPurgeResponse.checkAndUpdateDB(getOmMetadataManager(), batchOperation);
+      getOmMetadataManager().getStore().commitBatchOperation(batchOperation);
+    }
+
+    // Check if the entries are deleted.
+    assertTrue(getOmMetadataManager().getSnapshotInfoTable().isEmpty());
+
+    OMSnapshotPurgeResponse omSnapshotPurgeResponse1 = (OMSnapshotPurgeResponse)
+        omSnapshotPurgeRequest.validateAndUpdateCache(getOzoneManager(), 201L);
+
+    for (Map.Entry<String, SnapshotInfo> purgedSnapshot : omSnapshotPurgeResponse1.getUpdatedSnapInfos().entrySet()) {
+      assertNotNull(purgedSnapshot.getValue());
+    }
+    for (String snapshotTableKey: snapshotDbKeysToPurge) {
+      assertNull(getOmMetadataManager().getSnapshotInfoTable().get(snapshotTableKey));
+    }
+
+    try (BatchOperation batchOperation = getOmMetadataManager().getStore().initBatchOperation()) {
+      omSnapshotPurgeResponse1.checkAndUpdateDB(getOmMetadataManager(), batchOperation);
+      getOmMetadataManager().getStore().commitBatchOperation(batchOperation);
+    }
+  }
+
   /**
    * This test is mainly to validate metrics and error code.
    */