From ad108c86b11606d49f8ea9b739e40340a2117fe2 Mon Sep 17 00:00:00 2001
From: Swaminathan Balachandran <47532440+swamirishi@users.noreply.github.com>
Date: Mon, 30 Dec 2024 10:22:32 -0800
Subject: [PATCH] HDDS-11997. Duplicate snapshot purge request causes NPE
 (#7627)

---
 .../snapshot/OMSnapshotPurgeRequest.java      |  4 +-
 .../snapshot/OMSnapshotPurgeResponse.java     |  6 +++
 ...TestOMSnapshotPurgeRequestAndResponse.java | 37 +++++++++++++++++++
 3 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java
index 38c51d4de5c..62fbb39417b 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/snapshot/OMSnapshotPurgeRequest.java
+++ b/hadoop-ozone/ozone-manager/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/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotPurgeResponse.java b/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotPurgeResponse.java
index 81a020653f7..826293366b7 100644
--- a/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/response/snapshot/OMSnapshotPurgeResponse.java
+++ b/hadoop-ozone/ozone-manager/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/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotPurgeRequestAndResponse.java b/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotPurgeRequestAndResponse.java
index 1c44decdfda..8c1f38c01c6 100644
--- a/hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/snapshot/TestOMSnapshotPurgeRequestAndResponse.java
+++ b/hadoop-ozone/ozone-manager/src/test/java/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.
    */