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)