SOLR-10049: make collection deletion remove snapshot metadata

solr/CHANGES.txt

@@ -130,6 +130,8 @@ Bug Fixes
 
 * SOLR-9114: NPE using TermVectorComponent, MoreLikeThisComponent in combination with ExactStatsCache (Cao Manh Dat, Varun Thacker)
 
+* SOLR-10049: Collection deletion leaves behind the snapshot metadata (Hrishikesh Gadre via yonik)
+
 Optimizations
 ----------------------
 

solr/core/src/java/org/apache/solr/cloud/DeleteCollectionCmd.java

@@ -28,12 +28,14 @@ import java.util.concurrent.TimeUnit;
 import org.apache.solr.common.NonExistentCoreException;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.SolrZkClient;
 import org.apache.solr.common.cloud.ZkNodeProps;
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.params.CoreAdminParams;
 import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.Utils;
+import org.apache.solr.core.snapshots.SolrSnapshotManager;
 import org.apache.solr.util.TimeOut;
 import org.apache.zookeeper.KeeperException;
 import org.slf4j.Logger;
@@ -56,6 +58,11 @@ public class DeleteCollectionCmd implements OverseerCollectionMessageHandler.Cmd
     ZkStateReader zkStateReader = ocmh.zkStateReader;
     final String collection = message.getStr(NAME);
     try {
+      // Remove the snapshots meta-data for this collection in ZK. Deleting actual index files
+      // should be taken care of as part of collection delete operation.
+      SolrZkClient zkClient = zkStateReader.getZkClient();
+      SolrSnapshotManager.cleanupCollectionLevelSnapshots(zkClient, collection);
+
       if (zkStateReader.getClusterState().getCollectionOrNull(collection) == null) {
         if (zkStateReader.getZkClient().exists(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collection, true)) {
           // if the collection is not in the clusterstate, but is listed in zk, do nothing, it will just

solr/core/src/java/org/apache/solr/core/snapshots/SolrSnapshotsTool.java

@@ -295,6 +295,7 @@ public class SolrSnapshotsTool implements Closeable {
       Optional<String> asyncReqId) {
     try {
       CollectionAdminRequest.Backup backup = new CollectionAdminRequest.Backup(collectionName, snapshotName);
+      backup.setCommitName(snapshotName);
       backup.setIndexBackupStrategy(CollectionAdminParams.COPY_FILES_STRATEGY);
       backup.setLocation(destPath);
       if (backupRepo.isPresent()) {
@@ -350,29 +351,29 @@ public class SolrSnapshotsTool implements Closeable {
 
     if (cmd.hasOption(CREATE) || cmd.hasOption(DELETE) || cmd.hasOption(LIST) || cmd.hasOption(DESCRIBE)
         || cmd.hasOption(PREPARE_FOR_EXPORT) || cmd.hasOption(EXPORT_SNAPSHOT)) {
-      try (SolrSnapshotsTool tool = new SolrSnapshotsTool(cmd.getOptionValue(SOLR_ZK_ENSEMBLE))) {
+      try (SolrSnapshotsTool tool = new SolrSnapshotsTool(requiredArg(options, cmd, SOLR_ZK_ENSEMBLE))) {
         if (cmd.hasOption(CREATE)) {
           String snapshotName = cmd.getOptionValue(CREATE);
-          String collectionName = cmd.getOptionValue(COLLECTION);
+          String collectionName = requiredArg(options, cmd, COLLECTION);
           tool.createSnapshot(collectionName, snapshotName);
 
         } else if (cmd.hasOption(DELETE)) {
           String snapshotName = cmd.getOptionValue(DELETE);
-          String collectionName = cmd.getOptionValue(COLLECTION);
+          String collectionName = requiredArg(options, cmd, COLLECTION);
           tool.deleteSnapshot(collectionName, snapshotName);
 
         } else if (cmd.hasOption(LIST)) {
-          String collectionName = cmd.getOptionValue(COLLECTION);
+          String collectionName = requiredArg(options, cmd, COLLECTION);
           tool.listSnapshots(collectionName);
 
         } else if (cmd.hasOption(DESCRIBE)) {
           String snapshotName = cmd.getOptionValue(DESCRIBE);
-          String collectionName = cmd.getOptionValue(COLLECTION);
+          String collectionName = requiredArg(options, cmd, COLLECTION);
           tool.describeSnapshot(collectionName, snapshotName);
 
         } else if (cmd.hasOption(PREPARE_FOR_EXPORT)) {
           String snapshotName = cmd.getOptionValue(PREPARE_FOR_EXPORT);
-          String collectionName = cmd.getOptionValue(COLLECTION);
+          String collectionName = requiredArg(options, cmd, COLLECTION);
           String localFsDir = requiredArg(options, cmd, TEMP_DIR);
           String hdfsOpDir = requiredArg(options, cmd, DEST_DIR);
           Optional<String> pathPrefix = Optional.ofNullable(cmd.getOptionValue(HDFS_PATH_PREFIX));
@@ -391,7 +392,7 @@ public class SolrSnapshotsTool implements Closeable {
 
         }  else if (cmd.hasOption(EXPORT_SNAPSHOT)) {
           String snapshotName = cmd.getOptionValue(EXPORT_SNAPSHOT);
-          String collectionName = cmd.getOptionValue(COLLECTION);
+          String collectionName = requiredArg(options, cmd, COLLECTION);
           String destDir = requiredArg(options, cmd, DEST_DIR);
           Optional<String> backupRepo = Optional.ofNullable(cmd.getOptionValue(BACKUP_REPO_NAME));
           Optional<String> asyncReqId = Optional.ofNullable(cmd.getOptionValue(ASYNC_REQ_ID));

solr/core/src/test/org/apache/solr/core/snapshots/TestSolrCloudSnapshots.java

@@ -249,6 +249,24 @@ public class TestSolrCloudSnapshots extends SolrCloudTestCase {
     // Verify all core-level snapshots are deleted.
     assertTrue("The cores remaining " + snapshotByCoreName, snapshotByCoreName.isEmpty());
     assertTrue(listCollectionSnapshots(solrClient, collectionName).isEmpty());
+
+    // Verify if the collection deletion result in proper cleanup of snapshot metadata.
+    {
+      String commitName_2 = commitName + "_2";
+
+      CollectionAdminRequest.CreateSnapshot createSnap_2 = new CollectionAdminRequest.CreateSnapshot(collectionName, commitName_2);
+      assertEquals(0, createSnap_2.process(solrClient).getStatus());
+
+      Collection<CollectionSnapshotMetaData> collectionSnaps_2 = listCollectionSnapshots(solrClient, collectionName);
+      assertEquals(1, collectionSnaps.size());
+      assertEquals(commitName_2, collectionSnaps_2.iterator().next().getName());
+
+      // Delete collection
+      CollectionAdminRequest.Delete deleteCol = CollectionAdminRequest.deleteCollection(collectionName);
+      assertEquals(0, deleteCol.process(solrClient).getStatus());
+      assertTrue(SolrSnapshotManager.listSnapshots(solrClient.getZkStateReader().getZkClient(), collectionName).isEmpty());
+    }
+
   }
 
   private Collection<CollectionSnapshotMetaData> listCollectionSnapshots(SolrClient adminClient, String collectionName) throws Exception {