From c8edbe8663769392d422e84c05f45530833e48cc Mon Sep 17 00:00:00 2001 From: yonik Date: Mon, 30 Jan 2017 21:04:42 -0500 Subject: [PATCH] SOLR-10049: make collection deletion remove snapshot metadata --- solr/CHANGES.txt | 2 ++ .../apache/solr/cloud/DeleteCollectionCmd.java | 7 +++++++ .../solr/core/snapshots/SolrSnapshotsTool.java | 15 ++++++++------- .../core/snapshots/TestSolrCloudSnapshots.java | 18 ++++++++++++++++++ 4 files changed, 35 insertions(+), 7 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 21996159f08..27a9c7f3328 100644 --- a/solr/CHANGES.txt +++ b/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 ---------------------- diff --git a/solr/core/src/java/org/apache/solr/cloud/DeleteCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/DeleteCollectionCmd.java index 4c5ae007794..b891c92c98c 100644 --- a/solr/core/src/java/org/apache/solr/cloud/DeleteCollectionCmd.java +++ b/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 diff --git a/solr/core/src/java/org/apache/solr/core/snapshots/SolrSnapshotsTool.java b/solr/core/src/java/org/apache/solr/core/snapshots/SolrSnapshotsTool.java index cb1c52c1a7a..935ef638f8c 100644 --- a/solr/core/src/java/org/apache/solr/core/snapshots/SolrSnapshotsTool.java +++ b/solr/core/src/java/org/apache/solr/core/snapshots/SolrSnapshotsTool.java @@ -295,6 +295,7 @@ public class SolrSnapshotsTool implements Closeable { Optional 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 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 backupRepo = Optional.ofNullable(cmd.getOptionValue(BACKUP_REPO_NAME)); Optional asyncReqId = Optional.ofNullable(cmd.getOptionValue(ASYNC_REQ_ID)); diff --git a/solr/core/src/test/org/apache/solr/core/snapshots/TestSolrCloudSnapshots.java b/solr/core/src/test/org/apache/solr/core/snapshots/TestSolrCloudSnapshots.java index 65f74cad192..bb56a942ecf 100644 --- a/solr/core/src/test/org/apache/solr/core/snapshots/TestSolrCloudSnapshots.java +++ b/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 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 listCollectionSnapshots(SolrClient adminClient, String collectionName) throws Exception {