This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch branch_10_1 in repository https://gitbox.apache.org/repos/asf/solr.git
commit 218c409dbe11800cebecbcf0556711868cb71f8f Author: Eric Pugh <[email protected]> AuthorDate: Fri Oct 2 09:34:56 2026 -0400 SOLR-18321: Use Configsets V2 API instead of ZooKeeper cluster-state scans in bin/solr delete (#4673) Co-authored-by: Claude Sonnet 5 <[email protected]> (cherry picked from commit 427e5d5f4e30b069c4800496fea253fe4e6f99c4) (cherry picked from commit 1c459c584c29db92ca2ebe88a2e5547e536afb28) --- ...18321-delete-collection-configset-if-unused.yml | 12 ++ .../src/java/org/apache/solr/cli/DeleteTool.java | 126 +++++++-------------- .../solr/handler/admin/api/CollectionStatus.java | 13 ++- .../test/org/apache/solr/cli/DeleteToolTest.java | 116 +++++++++++++++++++ .../apache/solr/cloud/CollectionsAPISolrJTest.java | 9 ++ .../pages/collection-management.adoc | 2 + 6 files changed, 191 insertions(+), 87 deletions(-) diff --git a/changelog/unreleased/SOLR-18321-delete-collection-configset-if-unused.yml b/changelog/unreleased/SOLR-18321-delete-collection-configset-if-unused.yml new file mode 100644 index 00000000000..2ae09a673ea --- /dev/null +++ b/changelog/unreleased/SOLR-18321-delete-collection-configset-if-unused.yml @@ -0,0 +1,12 @@ +title: bin/solr delete no longer requires a direct ZooKeeper connection to safely delete a + collection's configset; it now uses the Configsets V2 API instead, relying on the API's + existing unconditional refusal to delete a configset that's still in use by another + collection. The delete tool's -f/--force flag is deprecated and no longer has any effect -- + it never actually bypassed that in-use check; it's kept only as a no-op for backward + compatibility with existing scripts. +type: changed +authors: + - name: Eric Pugh +links: + - name: SOLR-18321 + url: https://issues.apache.org/jira/browse/SOLR-18321 diff --git a/solr/core/src/java/org/apache/solr/cli/DeleteTool.java b/solr/core/src/java/org/apache/solr/cli/DeleteTool.java index 59450cf00c1..5f9c0c6e231 100644 --- a/solr/core/src/java/org/apache/solr/cli/DeleteTool.java +++ b/solr/core/src/java/org/apache/solr/cli/DeleteTool.java @@ -16,28 +16,21 @@ */ package org.apache.solr.cli; -import java.lang.invoke.MethodHandles; -import java.util.Collection; import java.util.Locale; -import java.util.Optional; -import java.util.Set; -import java.util.concurrent.TimeUnit; import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.DeprecatedAttributes; import org.apache.commons.cli.Option; import org.apache.commons.cli.Options; +import org.apache.solr.client.solrj.RemoteSolrException; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrServerException; -import org.apache.solr.client.solrj.impl.CloudSolrClient; -import org.apache.solr.client.solrj.jetty.HttpJettySolrClient; import org.apache.solr.client.solrj.request.CollectionsApi; import org.apache.solr.client.solrj.request.ConfigsetsApi; import org.apache.solr.client.solrj.request.CoresApi; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.apache.solr.common.SolrException; /** Supports delete command in the bin/solr script. */ public class DeleteTool extends ToolBase { - private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private static final Option COLLECTION_NAME_OPTION = Option.builder("c") @@ -55,11 +48,24 @@ public class DeleteTool extends ToolBase { "Flag to indicate if the underlying configuration directory for a collection should also be deleted; default is true.") .get(); + /** + * @deprecated Since Solr 11.0. No longer has any effect: the Overseer's configset-delete command + * unconditionally refuses to delete a configset that's still in use by another collection, so + * this flag was never actually able to bypass that safety check. Kept, as a no-op, for + * backward compatibility with existing scripts. + */ + @Deprecated(since = "11.0") private static final Option FORCE_OPTION = Option.builder("f") .longOpt("force") - .desc( - "Skip safety checks when deleting the configuration directory used by a collection.") + .deprecated( + DeprecatedAttributes.builder() + .setDescription( + "no longer has any effect; configset deletion is always safely skipped if" + + " the configset is still in use by another collection") + .setForRemoval(true) + .get()) + .desc("No longer has any effect; retained for backward compatibility.") .get(); public DeleteTool(ToolRuntime runtime) { @@ -94,105 +100,55 @@ public class DeleteTool extends ToolBase { public void runImpl(CommandLine cli) throws Exception { try (var solrClient = CLIUtils.getSolrClient(cli)) { if (CLIUtils.isCloudMode(solrClient)) { - deleteCollection(cli); + deleteCollection(cli, solrClient); } else { deleteCore(cli, solrClient); } } } - protected void deleteCollection(CommandLine cli) throws Exception { - var builder = - new HttpJettySolrClient.Builder() - .withIdleTimeout(30, TimeUnit.SECONDS) - .withConnectionTimeout(15, TimeUnit.SECONDS) - .withKeyStoreReloadInterval(-1, TimeUnit.SECONDS) - .withOptionalBasicAuthCredentials( - cli.getOptionValue(CommonCLIOptions.CREDENTIALS_OPTION)); - - var solrConnection = CLIUtils.getSolrConnection(cli); - try (var cloudSolrClient = CLIUtils.getCloudSolrClient(solrConnection, builder)) { - echoIfVerbose("Connecting to Solr at " + solrConnection.toString()); - deleteCollection(cloudSolrClient, cli); - } - } - - protected void deleteCollection(CloudSolrClient cloudSolrClient, CommandLine cli) - throws Exception { - Set<String> liveNodes = cloudSolrClient.getClusterState().getLiveNodes(); - if (liveNodes.isEmpty()) - throw new IllegalStateException( - "No live nodes found! Cannot delete a collection until " - + "there is at least 1 live node in the cluster."); - + protected void deleteCollection(CommandLine cli, SolrClient solrClient) throws Exception { String collectionName = cli.getOptionValue(COLLECTION_NAME_OPTION); - if (!cloudSolrClient.getClusterState().hasCollection(collectionName)) { - throw new IllegalArgumentException("Collection " + collectionName + " not found!"); - } - - String configName = - cloudSolrClient.getClusterState().getCollection(collectionName).getConfigName(); - boolean deleteConfig = cli.hasOption(DELETE_CONFIG_OPTION); - if (deleteConfig && configName != null) { - if (cli.hasOption(FORCE_OPTION)) { - log.warn( - "Skipping safety checks, configuration directory {} will be deleted with impunity.", - configName); - } else { - // need to scan all Collections to see if any are using the config - Collection<String> collections = cloudSolrClient.getClusterState().getCollectionNames(); - - // give a little note to the user if there are many collections in case it takes a while - if (collections.size() > 50) - if (log.isInfoEnabled()) { - log.info( - "Scanning {} to ensure no other collections are using config {}", - collections.size(), - configName); - } - - Optional<String> inUse = - collections.stream() - .filter(name -> !name.equals(collectionName)) // ignore this collection - .filter( - name -> - configName.equals( - cloudSolrClient.getClusterState().getCollection(name).getConfigName())) - .findFirst(); - if (inUse.isPresent()) { - deleteConfig = false; - log.warn( - "Configuration directory {} is also being used by {}{}", - configName, - inUse.get(), - "; configuration will not be deleted from ZooKeeper. You can pass the --force-delete-config flag to force delete."); - } + // Scoping the request to this one collection also serves as the existence check below, + // instead of a separate ListCollections call that would have to scan every collection in + // the cluster. + String configName; + try { + var statusReq = new CollectionsApi.GetCollectionStatus(collectionName); + var statusResponse = statusReq.process(solrClient); + configName = statusResponse.properties != null ? statusResponse.properties.configName : null; + } catch (RemoteSolrException e) { + if (e.code() == SolrException.ErrorCode.NOT_FOUND.code) { + throw new IllegalArgumentException("Collection " + collectionName + " not found!"); } + throw e; } + boolean deleteConfig = cli.hasOption(DELETE_CONFIG_OPTION); echoIfVerbose("\nDeleting collection '" + collectionName + "' using V2 Collections API"); try { var req = new CollectionsApi.DeleteCollection(collectionName); - var response = req.process(cloudSolrClient); + var response = req.process(solrClient); echoIfVerbose(response); } catch (SolrServerException sse) { throw new Exception( "Failed to delete collection '" + collectionName + "' due to: " + sse.getMessage()); } - if (deleteConfig) { + if (deleteConfig && configName != null) { try { var req = new ConfigsetsApi.DeleteConfigSet(configName); - req.process(cloudSolrClient); + req.process(solrClient); } catch (Exception exc) { + // Most commonly, this configset is still in use by another collection -- the + // configset-delete command unconditionally refuses to delete it in that case. echo( - "\nWARNING: Failed to delete configSet " + "\nWARNING: configSet " + configName - + " in solr due to: " - + exc.getMessage() - + "\nYou'll need to manually delete this znode using the bin/solr zk rm command."); + + " was not deleted. Most commonly it is still useed by another collection. Error: " + + exc.getMessage()); } } diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/CollectionStatus.java b/solr/core/src/java/org/apache/solr/handler/admin/api/CollectionStatus.java index c06842ed9e3..f0454d9763a 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/CollectionStatus.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/CollectionStatus.java @@ -22,6 +22,7 @@ import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP; import jakarta.inject.Inject; import org.apache.solr.client.api.endpoint.CollectionStatusApi; import org.apache.solr.client.api.model.CollectionStatusResponse; +import org.apache.solr.common.SolrException; import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.NamedList; @@ -73,13 +74,21 @@ public class CollectionStatus extends AdminAPIBase implements CollectionStatusAp final var nlResponse = new NamedList<>(); populateColStatusData(coreContainer, new ZkNodeProps(params), nlResponse); + final Object rawStatus = nlResponse.get(collectionName); + if (rawStatus == null) { + // ColStatus.getColStatus() silently skips any requested collection absent from cluster + // state, since v1's multi-collection listing just omits it. v2 is scoped to exactly one + // collection, so that omission means "not found" here. + throw new SolrException( + SolrException.ErrorCode.NOT_FOUND, "Collection '" + collectionName + "' not found"); + } + // v2 API does not support requesting the status of multiple collections simultaneously as its // counterpart does, and its response looks slightly different as a result. Primarily, the // v2 response eschews a level of nesting that necessitated by the multi-collection nature of // v1. These tweaks are made below before returning. final var colStatusResponse = - SolrJacksonMapper.getObjectMapper() - .convertValue(nlResponse.get(collectionName), CollectionStatusResponse.class); + SolrJacksonMapper.getObjectMapper().convertValue(rawStatus, CollectionStatusResponse.class); colStatusResponse.name = collectionName; return colStatusResponse; } diff --git a/solr/core/src/test/org/apache/solr/cli/DeleteToolTest.java b/solr/core/src/test/org/apache/solr/cli/DeleteToolTest.java index 8c36e1b661c..3c447d4a201 100644 --- a/solr/core/src/test/org/apache/solr/cli/DeleteToolTest.java +++ b/solr/core/src/test/org/apache/solr/cli/DeleteToolTest.java @@ -116,4 +116,120 @@ public class DeleteToolTest extends SolrCloudTestCase { }; assertEquals(1, CLITestHelper.runTool(args, DeleteTool.class)); } + + @Test + public void testFailsToDeleteNonexistentCollection() throws Exception { + String[] args = { + "delete", + "-c", + "testFailsToDeleteNonexistentCollection-does-not-exist", + "-z", + cluster.getZkClient().getZkServerAddress(), + "--credentials", + SecurityJson.USER_PASS, + "--verbose" + }; + assertEquals(1, CLITestHelper.runTool(args, DeleteTool.class)); + } + + @Test + public void testForceFlagNoLongerBypassesInUseCheck() throws Exception { + String sharedConfigName = "testForceFlagNoLongerBypassesInUseCheck-config"; + cluster.uploadConfigSet(configset("cloud-minimal"), sharedConfigName); + + withBasicAuth( + CollectionAdminRequest.createCollection("testForceFlagInUseA", sharedConfigName, 1, 1)) + .processAndWait(cluster.getSolrClient(), 10); + withBasicAuth( + CollectionAdminRequest.createCollection("testForceFlagInUseB", sharedConfigName, 1, 1)) + .processAndWait(cluster.getSolrClient(), 10); + waitForState( + "Expected collection to be created with 1 shard and 1 replicas", + "testForceFlagInUseA", + clusterShape(1, 1)); + waitForState( + "Expected collection to be created with 1 shard and 1 replicas", + "testForceFlagInUseB", + clusterShape(1, 1)); + + String[] args = { + "delete", + "-c", + "testForceFlagInUseA", + "--delete-config", + "--force", + "-z", + cluster.getZkClient().getZkServerAddress(), + "--credentials", + SecurityJson.USER_PASS, + "--verbose" + }; + // --force is a no-op now: still accepted (doesn't break arg parsing), but doesn't bypass the + // still-in-use check -- the collection delete itself still succeeds. + assertEquals(0, CLITestHelper.runTool(args, DeleteTool.class)); + + assertTrue( + "configset should still exist since it's still used by testForceFlagInUseB, " + + "even with --force", + cluster.getZkClient().exists("/configs/" + sharedConfigName)); + } + + @Test + public void testDeleteCollectionSkipsConfigDeleteWhileStillInUse() throws Exception { + String sharedConfigName = "testDeleteCollectionSkipsConfigDeleteWhileStillInUse-config"; + cluster.uploadConfigSet(configset("cloud-minimal"), sharedConfigName); + + withBasicAuth( + CollectionAdminRequest.createCollection( + "testDeleteConfigInUseA", sharedConfigName, 1, 1)) + .processAndWait(cluster.getSolrClient(), 10); + withBasicAuth( + CollectionAdminRequest.createCollection( + "testDeleteConfigInUseB", sharedConfigName, 1, 1)) + .processAndWait(cluster.getSolrClient(), 10); + waitForState( + "Expected collection to be created with 1 shard and 1 replicas", + "testDeleteConfigInUseA", + clusterShape(1, 1)); + waitForState( + "Expected collection to be created with 1 shard and 1 replicas", + "testDeleteConfigInUseB", + clusterShape(1, 1)); + + String[] deleteFirstArgs = { + "delete", + "-c", + "testDeleteConfigInUseA", + "--delete-config", + "-z", + cluster.getZkClient().getZkServerAddress(), + "--credentials", + SecurityJson.USER_PASS, + "--verbose" + }; + assertEquals(0, CLITestHelper.runTool(deleteFirstArgs, DeleteTool.class)); + + // still used by testDeleteConfigInUseB, so it should have been left alone + assertTrue( + "configset should still exist since it's still used by testDeleteConfigInUseB", + cluster.getZkClient().exists("/configs/" + sharedConfigName)); + + String[] deleteSecondArgs = { + "delete", + "-c", + "testDeleteConfigInUseB", + "--delete-config", + "-z", + cluster.getZkClient().getZkServerAddress(), + "--credentials", + SecurityJson.USER_PASS, + "--verbose" + }; + assertEquals(0, CLITestHelper.runTool(deleteSecondArgs, DeleteTool.class)); + + // no longer used by any collection, so it should now be gone + assertFalse( + "configset should have been deleted since no collection uses it anymore", + cluster.getZkClient().exists("/configs/" + sharedConfigName)); + } } diff --git a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java index 077d4a6f557..3e48d958c31 100644 --- a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java @@ -43,6 +43,7 @@ import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; import org.apache.lucene.tests.util.TestUtil; import org.apache.lucene.util.Version; +import org.apache.solr.client.solrj.RemoteSolrException; import org.apache.solr.client.solrj.SolrClient; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrResponse; @@ -877,6 +878,14 @@ public class CollectionsAPISolrJTest extends SolrCloudTestCase { assertNull(segmentData.segments.get("_0").fields); } + @Test + public void testV2CollectionStatusForNonExistentCollectionReturns404() { + var req = new CollectionsApi.GetCollectionStatus("doesNotExist"); + final RemoteSolrException ex = + expectThrows(RemoteSolrException.class, () -> req.process(cluster.getSolrClient())); + assertEquals(404, ex.code()); + } + private static final int NUM_DOCS = 10; @Test diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc index 45b26e0af47..a2eade1e7d7 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc +++ b/solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc @@ -1267,6 +1267,8 @@ The response will include an overview of the collection status, the number of active or inactive shards and replicas, and additional index information of shard leaders. +If the requested collection does not exist, the V1 API returns a `200 OK` with an empty body, while the V2 API returns a `404` error with an appropriate error message. + === Examples using COLSTATUS *Input*
