From 70efb0f4235669b5f8292b2d3ae52dd5efaa11df Mon Sep 17 00:00:00 2001 From: SpookyJumpyBeans <116844292+SpookyJumpyBeans@users.noreply.github.com> Date: Mon, 19 Oct 2026 18:58:05 -0400 Subject: [PATCH] SOLR-18557: Restore source collection when REINDEXCOLLECTION cleanup fails If deleting the target or checkpoint collection threw during cleanup (for example because an alias references the target), the readOnly reset and the reindexing state removal were skipped, leaving the source collection read-only. Run them in a finally block so the source is always restored, and keep rethrowing the delete error. --- .../SOLR-18557-reindex-abort-readonly.yml | 12 ++ .../api/collections/ReindexCollectionCmd.java | 106 +++++++++--------- .../solr/cloud/ReindexCollectionTest.java | 36 ++++++ 3 files changed, 104 insertions(+), 50 deletions(-) create mode 100644 changelog/unreleased/SOLR-18557-reindex-abort-readonly.yml diff --git a/changelog/unreleased/SOLR-18557-reindex-abort-readonly.yml b/changelog/unreleased/SOLR-18557-reindex-abort-readonly.yml new file mode 100644 index 000000000000..b376faa572ae --- /dev/null +++ b/changelog/unreleased/SOLR-18557-reindex-abort-readonly.yml @@ -0,0 +1,12 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc + +title: > + REINDEXCOLLECTION now restores the source collection when its cleanup can't delete the target or + checkpoint collection, instead of leaving the source read-only +type: fixed +authors: + - name: Boyi Dai + nick: SpookyJumpyBeans +links: + - name: SOLR-18557 + url: https://issues.apache.org/jira/browse/SOLR-18557 diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/ReindexCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/ReindexCollectionCmd.java index 256f07726916..8859ec708713 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/ReindexCollectionCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/ReindexCollectionCmd.java @@ -900,56 +900,62 @@ private void cleanup( } ClusterState clusterState = ccc.getSolrCloudManager().getClusterState(); NamedList cmdResults = new NamedList<>(); - if (createdTarget - && !collection.equals(targetCollection) - && clusterState.hasCollection(targetCollection)) { - log.debug(" -- removing {}", targetCollection); - ZkNodeProps cmd = - new ZkNodeProps(CommonParams.NAME, targetCollection, FOLLOW_ALIASES, "false"); - new DeleteCollectionCmd(ccc) - .call( - adminCmdContext - .subRequestContext(CollectionParams.CollectionAction.DELETE, null) - .withClusterState(clusterState), - cmd, - cmdResults); - CollectionHandlingUtils.checkResults( - "CLEANUP: deleting target collection " + targetCollection, cmdResults, false); - } - // remove chk collection - if (clusterState.hasCollection(chkCollection)) { - log.debug(" -- removing {}", chkCollection); - ZkNodeProps cmd = new ZkNodeProps(CommonParams.NAME, chkCollection, FOLLOW_ALIASES, "false"); - cmdResults = new NamedList<>(); - new DeleteCollectionCmd(ccc) - .call( - adminCmdContext - .subRequestContext(CollectionParams.CollectionAction.DELETE, null) - .withClusterState(clusterState), - cmd, - cmdResults); - CollectionHandlingUtils.checkResults( - "CLEANUP: deleting checkpoint collection " + chkCollection, cmdResults, false); - } - log.debug(" -- turning readOnly mode off for {}", collection); - ZkNodeProps props = - new ZkNodeProps( - Overseer.QUEUE_OPERATION, - CollectionParams.CollectionAction.MODIFYCOLLECTION.toLower(), - ZkStateReader.COLLECTION_PROP, - collection, - ZkStateReader.READ_ONLY, - null); - if (ccc.getDistributedClusterStateUpdater().isDistributedStateUpdate()) { - ccc.getDistributedClusterStateUpdater() - .doSingleStateUpdate( - DistributedClusterStateUpdater.MutatingCommand.CollectionModifyCollection, - props, - ccc.getSolrCloudManager(), - ccc.getZkStateReader()); - } else { - ccc.offerStateUpdate(props); + try { + if (createdTarget + && !collection.equals(targetCollection) + && clusterState.hasCollection(targetCollection)) { + log.debug(" -- removing {}", targetCollection); + ZkNodeProps cmd = + new ZkNodeProps(CommonParams.NAME, targetCollection, FOLLOW_ALIASES, "false"); + new DeleteCollectionCmd(ccc) + .call( + adminCmdContext + .subRequestContext(CollectionParams.CollectionAction.DELETE, null) + .withClusterState(clusterState), + cmd, + cmdResults); + CollectionHandlingUtils.checkResults( + "CLEANUP: deleting target collection " + targetCollection, cmdResults, false); + } + // remove chk collection + if (clusterState.hasCollection(chkCollection)) { + log.debug(" -- removing {}", chkCollection); + ZkNodeProps cmd = + new ZkNodeProps(CommonParams.NAME, chkCollection, FOLLOW_ALIASES, "false"); + cmdResults = new NamedList<>(); + new DeleteCollectionCmd(ccc) + .call( + adminCmdContext + .subRequestContext(CollectionParams.CollectionAction.DELETE, null) + .withClusterState(clusterState), + cmd, + cmdResults); + CollectionHandlingUtils.checkResults( + "CLEANUP: deleting checkpoint collection " + chkCollection, cmdResults, false); + } + } finally { + // always restore the source collection, even if the temporary collections could not be + // removed (e.g. the target is referenced by an alias), otherwise it is left read-only + log.debug(" -- turning readOnly mode off for {}", collection); + ZkNodeProps props = + new ZkNodeProps( + Overseer.QUEUE_OPERATION, + CollectionParams.CollectionAction.MODIFYCOLLECTION.toLower(), + ZkStateReader.COLLECTION_PROP, + collection, + ZkStateReader.READ_ONLY, + null); + if (ccc.getDistributedClusterStateUpdater().isDistributedStateUpdate()) { + ccc.getDistributedClusterStateUpdater() + .doSingleStateUpdate( + DistributedClusterStateUpdater.MutatingCommand.CollectionModifyCollection, + props, + ccc.getSolrCloudManager(), + ccc.getZkStateReader()); + } else { + ccc.offerStateUpdate(props); + } + removeReindexingState(collection); } - removeReindexingState(collection); } } diff --git a/solr/core/src/test/org/apache/solr/cloud/ReindexCollectionTest.java b/solr/core/src/test/org/apache/solr/cloud/ReindexCollectionTest.java index aff39f246e75..ab068856feac 100644 --- a/solr/core/src/test/org/apache/solr/cloud/ReindexCollectionTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/ReindexCollectionTest.java @@ -444,6 +444,42 @@ public void testAbort() throws Exception { assertEquals(status.toString(), "aborted", status.get("state")); } + @Test + public void testAbortWithAliasedTarget() throws Exception { + final String sourceCollection = "abortAliasedReindexing"; + final String targetCollection = "abortAliasedReindexingTarget"; + final String targetAlias = "abortAliasedReindexingAlias"; + createCollection(sourceCollection, "conf1", 2, 1); + + TestInjection.reindexLatch = new CountDownLatch(1); + CollectionAdminRequest.ReindexCollection req = + CollectionAdminRequest.reindexCollection(sourceCollection).setTarget(targetCollection); + String asyncId = req.processAsync(solrClient); + waitForState( + "source collection didn't become readOnly", sourceCollection, DocCollection::isReadOnly); + + // an alias on the target makes the cleanup fail to delete it + CollectionAdminRequest.createAlias(targetAlias, targetCollection).process(solrClient); + + req = CollectionAdminRequest.reindexCollection(sourceCollection); + req.setCommand("abort"); + req.process(solrClient); + waitForReindexingState(sourceCollection, ReindexCollectionCmd.State.ABORTED); + TestInjection.reindexLatch.countDown(); + + // the source collection must still be restored + waitForState( + "source collection is in wrong state", + sourceCollection, + docCollection -> !docCollection.isReadOnly()); + waitForReindexingState(sourceCollection, null); + + // the failure to delete the aliased target is still reported + assertEquals( + RequestStatusState.FAILED, + CollectionAdminRequest.requestStatus(asyncId).waitFor(solrClient, 30)); + } + private void createCollection(String name, String config, int numShards, int numReplicas) throws Exception { CollectionAdminRequest.createCollection(name, config, numShards, numReplicas)