diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 3d89670ea93..9e4875be488 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -386,6 +386,9 @@ Other Changes * SOLR-7010: Remove facet.date client functionality. (Steve Rowe) +* SOLR-8423: DeleteShard and DeleteReplica should cleanup instance and data directory by default and add + support for optionally retaining the directories. (Anshum Gupta) + ================== 5.5.1 ================== Bug Fixes diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java b/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java index fed92bbf4b9..055d8494b5a 100644 --- a/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java +++ b/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionMessageHandler.java @@ -600,8 +600,10 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler ModifiableSolrParams params = new ModifiableSolrParams(); params.add(CoreAdminParams.ACTION, CoreAdminAction.UNLOAD.toString()); params.add(CoreAdminParams.CORE, core); - params.add(CoreAdminParams.DELETE_INSTANCE_DIR, "true"); - params.add(CoreAdminParams.DELETE_DATA_DIR, "true"); + + params.set(CoreAdminParams.DELETE_INDEX, message.getBool(CoreAdminParams.DELETE_INDEX, true)); + params.set(CoreAdminParams.DELETE_INSTANCE_DIR, message.getBool(CoreAdminParams.DELETE_INSTANCE_DIR, true)); + params.set(CoreAdminParams.DELETE_DATA_DIR, message.getBool(CoreAdminParams.DELETE_DATA_DIR, true)); sendShardRequest(replica.getNodeName(), params, shardHandler, asyncId, requestMap); @@ -1407,7 +1409,10 @@ public class OverseerCollectionMessageHandler implements OverseerMessageHandler try { ModifiableSolrParams params = new ModifiableSolrParams(); params.set(CoreAdminParams.ACTION, CoreAdminAction.UNLOAD.toString()); - params.set(CoreAdminParams.DELETE_INDEX, "true"); + params.set(CoreAdminParams.DELETE_INDEX, message.getBool(CoreAdminParams.DELETE_INDEX, true)); + params.set(CoreAdminParams.DELETE_INSTANCE_DIR, message.getBool(CoreAdminParams.DELETE_INSTANCE_DIR, true)); + params.set(CoreAdminParams.DELETE_DATA_DIR, message.getBool(CoreAdminParams.DELETE_DATA_DIR, true)); + sliceCmd(clusterState, params, null, slice, shardHandler, asyncId, requestMap); processResponses(results, shardHandler, true, "Failed to delete shard", asyncId, requestMap, Collections.emptySet()); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java index ce4eab213ee..de2104f4d07 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java @@ -45,6 +45,9 @@ import static org.apache.solr.common.params.CommonAdminParams.ASYNC; import static org.apache.solr.common.params.CommonParams.NAME; import static org.apache.solr.common.params.CommonParams.VALUE_LONG; import static org.apache.solr.common.params.CoreAdminParams.DATA_DIR; +import static org.apache.solr.common.params.CoreAdminParams.DELETE_DATA_DIR; +import static org.apache.solr.common.params.CoreAdminParams.DELETE_INDEX; +import static org.apache.solr.common.params.CoreAdminParams.DELETE_INSTANCE_DIR; import static org.apache.solr.common.params.CoreAdminParams.INSTANCE_DIR; import static org.apache.solr.common.params.ShardParams._ROUTE_; import static org.apache.solr.common.util.StrUtils.formatString; @@ -478,9 +481,14 @@ public class CollectionsHandler extends RequestHandlerBase { DELETESHARD_OP(DELETESHARD) { @Override Map call(SolrQueryRequest req, SolrQueryResponse rsp, CollectionsHandler handler) throws Exception { - return req.getParams().required().getAll(null, + Map map = req.getParams().required().getAll(null, COLLECTION_PROP, SHARD_ID_PROP); + req.getParams().getAll(map, + DELETE_INDEX, + DELETE_DATA_DIR, + DELETE_INSTANCE_DIR); + return map; } }, FORCELEADER_OP(FORCELEADER) { @@ -517,6 +525,12 @@ public class CollectionsHandler extends RequestHandlerBase { COLLECTION_PROP, SHARD_ID_PROP, REPLICA_PROP); + + req.getParams().getAll(map, + DELETE_INDEX, + DELETE_DATA_DIR, + DELETE_INSTANCE_DIR); + return req.getParams().getAll(map, ONLY_IF_DOWN); } }, diff --git a/solr/core/src/java/org/apache/solr/util/FileUtils.java b/solr/core/src/java/org/apache/solr/util/FileUtils.java index 48148f6f5bc..09db4f0f686 100644 --- a/solr/core/src/java/org/apache/solr/util/FileUtils.java +++ b/solr/core/src/java/org/apache/solr/util/FileUtils.java @@ -93,4 +93,7 @@ public class FileUtils { throw exc; } + public static boolean fileExists(String filePathString) { + return new File(filePathString).exists(); + } } diff --git a/solr/core/src/test/org/apache/solr/cloud/DeleteReplicaTest.java b/solr/core/src/test/org/apache/solr/cloud/DeleteReplicaTest.java index b1f8c49dc15..f0d64840278 100644 --- a/solr/core/src/test/org/apache/solr/cloud/DeleteReplicaTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/DeleteReplicaTest.java @@ -16,10 +16,19 @@ */ package org.apache.solr.cloud; +import java.io.File; +import java.io.IOException; +import java.lang.invoke.MethodHandles; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; + import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.client.solrj.impl.HttpSolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.request.CoreAdminRequest; import org.apache.solr.client.solrj.request.QueryRequest; import org.apache.solr.client.solrj.response.CoreAdminResponse; @@ -31,24 +40,17 @@ import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.MapSolrParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.NamedList; +import org.apache.solr.util.FileUtils; import org.apache.solr.util.TimeOut; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.File; -import java.io.IOException; -import java.lang.invoke.MethodHandles; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.concurrent.TimeUnit; - import static org.apache.solr.cloud.OverseerCollectionMessageHandler.NUM_SLICES; import static org.apache.solr.cloud.OverseerCollectionMessageHandler.ONLY_IF_DOWN; -import static org.apache.solr.common.util.Utils.makeMap; import static org.apache.solr.common.cloud.ZkStateReader.MAX_SHARDS_PER_NODE; import static org.apache.solr.common.params.CollectionParams.CollectionAction.DELETEREPLICA; +import static org.apache.solr.common.util.Utils.makeMap; public class DeleteReplicaTest extends AbstractFullDistribZkTestBase { @@ -170,4 +172,44 @@ public class DeleteReplicaTest extends AbstractFullDistribZkTestBase { Map> collectionInfos = new HashMap<>(); createCollection(collectionInfos, COLL_NAME, props, client); } + + @Test + @ShardsFixed(num = 2) + public void deleteReplicaAndVerifyDirectoryCleanup() throws IOException, SolrServerException, InterruptedException { + createCollection("deletereplica_test", 1, 2, 4); + + Replica leader = cloudClient.getZkStateReader().getLeaderRetry("deletereplica_test", "shard1"); + String baseUrl = (String) leader.get("base_url"); + String core = (String) leader.get("core"); + String leaderCoreName = leader.getName(); + + String instanceDir; + String dataDir; + + try (HttpSolrClient client = new HttpSolrClient(baseUrl)) { + CoreAdminResponse statusResp = CoreAdminRequest.getStatus(core, client); + NamedList r = statusResp.getCoreStatus().get(core); + instanceDir = (String) r.findRecursive("instanceDir"); + dataDir = (String) r.get("dataDir"); + } + + //Confirm that the instance and data directory exist + assertTrue("Instance directory doesn't exist", FileUtils.fileExists(instanceDir)); + assertTrue("DataDirectory doesn't exist", FileUtils.fileExists(dataDir)); + + new CollectionAdminRequest.DeleteReplica() + .setCollectionName("deletereplica_test") + .setShardName("shard1") + .setReplica(leaderCoreName) + .process(cloudClient); + + Replica newLeader = cloudClient.getZkStateReader().getLeaderRetry("deletereplica_test", "shard1"); + + assertFalse(leader.equals(newLeader)); + + //Confirm that the instance and data directory were deleted by default + + assertFalse("Instance directory still exists", FileUtils.fileExists(instanceDir)); + assertFalse("DataDirectory still exists", FileUtils.fileExists(dataDir)); + } } diff --git a/solr/core/src/test/org/apache/solr/cloud/DeleteShardTest.java b/solr/core/src/test/org/apache/solr/cloud/DeleteShardTest.java index ae15e229e04..101bfb98c20 100644 --- a/solr/core/src/test/org/apache/solr/cloud/DeleteShardTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/DeleteShardTest.java @@ -16,27 +16,34 @@ */ package org.apache.solr.cloud; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.impl.HttpSolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.CoreAdminRequest; import org.apache.solr.client.solrj.request.QueryRequest; +import org.apache.solr.client.solrj.response.CollectionAdminResponse; +import org.apache.solr.client.solrj.response.CoreAdminResponse; import org.apache.solr.cloud.overseer.OverseerAction; import org.apache.solr.common.SolrException; import org.apache.solr.common.cloud.ClusterState; +import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.cloud.Slice; import org.apache.solr.common.cloud.Slice.State; import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.CollectionParams; 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.util.FileUtils; import org.apache.zookeeper.KeeperException; import org.junit.Test; -import java.io.IOException; -import java.util.HashMap; -import java.util.Map; - public class DeleteShardTest extends AbstractFullDistribZkTestBase { public DeleteShardTest() { @@ -150,4 +157,67 @@ public class DeleteShardTest extends AbstractFullDistribZkTestBase { } } + @Test + public void testDirectoryCleanupAfterDeleteShard() throws InterruptedException, IOException, SolrServerException { + CollectionAdminResponse rsp = new CollectionAdminRequest.Create() + .setCollectionName("deleteshard_test") + .setRouterName("implicit") + .setShards("a,b,c") + .setReplicationFactor(1) + .setConfigName("conf1") + .process(cloudClient); + + // Get replica details + Replica leader = cloudClient.getZkStateReader().getLeaderRetry("deleteshard_test", "a"); + String baseUrl = (String) leader.get("base_url"); + String core = (String) leader.get("core"); + + String instanceDir; + String dataDir; + + try (HttpSolrClient client = new HttpSolrClient(baseUrl)) { + CoreAdminResponse statusResp = CoreAdminRequest.getStatus(core, client); + NamedList r = statusResp.getCoreStatus().get(core); + instanceDir = (String) r.findRecursive("instanceDir"); + dataDir = (String) r.get("dataDir"); + } + + assertTrue("Instance directory doesn't exist", FileUtils.fileExists(instanceDir)); + assertTrue("Data directory doesn't exist", FileUtils.fileExists(dataDir)); + + assertEquals(3, cloudClient.getZkStateReader().getClusterState().getActiveSlices("deleteshard_test").size()); + + // Delete shard 'a' + new CollectionAdminRequest.DeleteShard() + .setCollectionName("deleteshard_test") + .setShardName("a") + .process(cloudClient); + + assertEquals(2, cloudClient.getZkStateReader().getClusterState().getActiveSlices("deleteshard_test").size()); + assertFalse("Instance directory still exists", FileUtils.fileExists(instanceDir)); + assertFalse("Data directory still exists", FileUtils.fileExists(dataDir)); + + leader = cloudClient.getZkStateReader().getLeaderRetry("deleteshard_test", "b"); + baseUrl = (String) leader.get("base_url"); + core = (String) leader.get("core"); + + try (HttpSolrClient client = new HttpSolrClient(baseUrl)) { + CoreAdminResponse statusResp = CoreAdminRequest.getStatus(core, client); + NamedList r = statusResp.getCoreStatus().get(core); + instanceDir = (String) r.findRecursive("instanceDir"); + dataDir = (String) r.get("dataDir"); + } + + // Delete shard 'b' + new CollectionAdminRequest.DeleteShard() + .setCollectionName("deleteshard_test") + .setShardName("b") + .setDeleteDataDir(false) + .setDeleteInstanceDir(false) + .process(cloudClient); + + assertEquals(1, cloudClient.getZkStateReader().getClusterState().getActiveSlices("deleteshard_test").size()); + assertTrue("Instance directory still exists", FileUtils.fileExists(instanceDir)); + assertTrue("Data directory still exists", FileUtils.fileExists(dataDir)); + } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java index 9aead924968..a7d71ca5b93 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java @@ -526,6 +526,10 @@ public abstract class CollectionAdminRequest { + + private Boolean deleteInstanceDir; + private Boolean deleteDataDir; + public DeleteShard() { action = CollectionAction.DELETESHARD; } @@ -534,6 +538,36 @@ public abstract class CollectionAdminRequest { protected String replica; protected Boolean onlyIfDown; + private Boolean deleteDataDir; + private Boolean deleteInstanceDir; + private Boolean deleteIndexDir; public DeleteReplica() { action = CollectionAction.DELETEREPLICA; @@ -824,6 +861,15 @@ public abstract class CollectionAdminRequest