From 48ef08939001a9ec51bf21897c56a0aec5e2316d Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Fri, 21 Aug 2026 15:50:15 +0300 Subject: [PATCH 1/9] SOLR-18367: client -- remove AsyncCollectionAdminRequest.setWaitForFinalState, default to true SOLR-17712 proposed removing the ability to opt out of waiting for final state entirely, defaulting to 'true'. The setter was deprecated in 9.10 as the first step; this is the second: the field now always resolves to true (no other code path can set it to false), so the setter -- and every test call site that was toggling it -- is gone. Removes/cleans 6 test call sites (5 were redundant .setWaitForFinalState(true) calls; CollectionsAPISolrJTest's .setWaitForFinalState(false) is dropped too, since the test doesn't depend on the non-waiting behavior). --- .../src/test/org/apache/solr/cloud/AddReplicaTest.java | 3 --- .../org/apache/solr/cloud/CollectionsAPISolrJTest.java | 1 - .../src/test/org/apache/solr/cloud/ReplaceNodeTest.java | 1 - .../src/test/org/apache/solr/cloud/SplitShardTest.java | 1 - .../org/apache/solr/core/TestSetPropertyConfigApis.java | 1 - .../solr/client/solrj/request/CollectionAdminRequest.java | 7 +------ 6 files changed, 1 insertion(+), 13 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/cloud/AddReplicaTest.java b/solr/core/src/test/org/apache/solr/cloud/AddReplicaTest.java index 7590311a01f9..23e473a72904 100644 --- a/solr/core/src/test/org/apache/solr/cloud/AddReplicaTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/AddReplicaTest.java @@ -155,8 +155,6 @@ public void test() throws Exception { replicas2.removeAll(replicas); assertEquals(1, replicas2.size()); - // use waitForFinalState - addReplica.setWaitForFinalState(true); addReplica.processAsync("001", cloudClient); requestStatus = CollectionAdminRequest.requestStatus("001"); rsp = requestStatus.process(cloudClient); @@ -205,7 +203,6 @@ public void testAddReplicaWithUserDefinedProperties() throws Exception { CollectionAdminRequest.addReplicaToShard(collectionName, "shard1"); addReplica.withProperty("customProp2", "val2.1"); addReplica.withProperty("customProp3", "val3"); - addReplica.setWaitForFinalState(true); addReplica.process(cloudClient); // Verify that the new core was created with user-defined properties coming from the request 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 17cb7e51ebb6..58790dea5f63 100644 --- a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java @@ -232,7 +232,6 @@ public void testCreateAndDeleteCollection() throws Exception { String collectionName = getSaferTestName(); CollectionAdminRequest.Create createREq = CollectionAdminRequest.createCollection(collectionName, "conf", 2, 2); - createREq.setWaitForFinalState(false); CollectionAdminResponse response = createREq.process(cluster.getSolrClient()); assertEquals(0, response.getStatus()); diff --git a/solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java b/solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java index bc64635d2d87..a7dd392c9b6f 100644 --- a/solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java @@ -133,7 +133,6 @@ public void test() throws Exception { // let's do it back - this time wait for recoveries CollectionAdminRequest.AsyncCollectionAdminRequest replaceNodeRequest = createReplaceNodeRequest(emptyNode, nodeToBeDecommissioned, Boolean.TRUE); - replaceNodeRequest.setWaitForFinalState(true); replaceNodeRequest.processAndWait("001", cloudClient, 10); try (SolrClient coreClient = diff --git a/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java b/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java index 0b539bd39d0f..7729183cb261 100644 --- a/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/SplitShardTest.java @@ -192,7 +192,6 @@ public void testWithChildDocuments() throws Exception { CollectionAdminRequest.splitShard(COLLECTION_NAME) .setNumSubShards(2) .setShardName("shard1"); - splitShard.setWaitForFinalState(true); splitShard.process(solrClient); waitForState( "Waiting for 2 active shards after split", COLLECTION_NAME, activeClusterShape(2, 2)); diff --git a/solr/core/src/test/org/apache/solr/core/TestSetPropertyConfigApis.java b/solr/core/src/test/org/apache/solr/core/TestSetPropertyConfigApis.java index 54c30f7d75fb..8bf024cbecda 100644 --- a/solr/core/src/test/org/apache/solr/core/TestSetPropertyConfigApis.java +++ b/solr/core/src/test/org/apache/solr/core/TestSetPropertyConfigApis.java @@ -279,7 +279,6 @@ public void testTwoCollectionsWithDifferentProps() throws Exception { private static void processAndAssertSuccess(final CollectionAdminRequest.Create op) throws Exception { - op.setWaitForFinalState(true); assertTrue(op.process(cluster.getSolrClient()).isSuccess()); } 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 7147699a592a..d7501e549ccf 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 @@ -146,7 +146,7 @@ public abstract static class AsyncCollectionAdminRequest extends CollectionAdminRequest { protected String asyncId = null; - protected boolean waitForFinalState = false; + protected boolean waitForFinalState = true; public AsyncCollectionAdminRequest(CollectionAction action) { super(action); @@ -165,11 +165,6 @@ public String getAsyncId() { return asyncId; } - @Deprecated(since = "9.10") - public void setWaitForFinalState(boolean waitForFinalState) { - this.waitForFinalState = waitForFinalState; - } - public void setAsyncId(String asyncId) { this.asyncId = asyncId; } From daf833bcb78b7c96e6437817fadfea50c7027388 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Fri, 21 Aug 2026 15:52:59 +0300 Subject: [PATCH 2/9] SOLR-18367: server -- default waitForFinalState to true in all 8 commands Completes SOLR-17712's proposal: 'internal logic should behave as if it's true'. Flips the server-side default from false to true in CreateCollectionCmd, AddReplicaCmd, CreateShardCmd, MoveReplicaCmd, SplitShardCmd, BalanceReplicasCmd, MigrateReplicasCmd, ReplaceNodeCmd. The V2 API request bodies (nullable Boolean fields) don't need a change -- when unset they serialize to absent, and these Cmd-level defaults are what actually apply. 72 tests across all 8 affected commands, 0 failures. --- .../org/apache/solr/cloud/api/collections/AddReplicaCmd.java | 2 +- .../apache/solr/cloud/api/collections/BalanceReplicasCmd.java | 2 +- .../apache/solr/cloud/api/collections/CreateCollectionCmd.java | 2 +- .../org/apache/solr/cloud/api/collections/CreateShardCmd.java | 2 +- .../apache/solr/cloud/api/collections/MigrateReplicasCmd.java | 2 +- .../org/apache/solr/cloud/api/collections/MoveReplicaCmd.java | 2 +- .../org/apache/solr/cloud/api/collections/ReplaceNodeCmd.java | 2 +- .../org/apache/solr/cloud/api/collections/SplitShardCmd.java | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java index 162dc555a8ad..81772ac698fa 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java @@ -113,7 +113,7 @@ List addReplica( "Collection: " + collectionName + " shard: " + shard + " does not exist"); } - boolean waitForFinalState = message.getBool(WAIT_FOR_FINAL_STATE, false); + boolean waitForFinalState = message.getBool(WAIT_FOR_FINAL_STATE, true); boolean skipCreateReplicaInClusterState = message.getBool(SKIP_CREATE_REPLICA_IN_CLUSTER_STATE, false); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/BalanceReplicasCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/BalanceReplicasCmd.java index 743d9f5847fc..75ffe62b67d9 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/BalanceReplicasCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/BalanceReplicasCmd.java @@ -55,7 +55,7 @@ public void call(AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList "'nodes' was not passed as a correct type (Set/List/String): " + nodesRaw.getClass().getName()); } - boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, false); + boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, true); int timeout = message.getInt("timeout", 10 * 60); // 10 minutes boolean parallel = message.getBool("parallel", false); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java index 14291d7b9411..fa7633cc1736 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java @@ -111,7 +111,7 @@ public void call(AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList ClusterState clusterState = adminCmdContext.getClusterState(); final Aliases aliases = ccc.getZkStateReader().getAliases(); final String collectionName = message.getStr(NAME); - final boolean waitForFinalState = message.getBool(WAIT_FOR_FINAL_STATE, false); + final boolean waitForFinalState = message.getBool(WAIT_FOR_FINAL_STATE, true); final String alias = message.getStr(ALIAS, collectionName); log.info("Create collection {}", collectionName); boolean prsDefault = EnvUtils.getPropertyAsBool(PRS_DEFAULT_PROP, false); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateShardCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateShardCmd.java index 7c0e16336038..7176a830b083 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateShardCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateShardCmd.java @@ -50,7 +50,7 @@ public void call(AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList throws Exception { String extCollectionName = message.getStr(COLLECTION_PROP); String sliceName = message.getStr(SHARD_ID_PROP); - boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, false); + boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, true); log.info("Create shard invoked: {}", message); if (extCollectionName == null || sliceName == null) diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateReplicasCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateReplicasCmd.java index 779ab9991977..fa3b4a2f1127 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateReplicasCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateReplicasCmd.java @@ -50,7 +50,7 @@ public void call(AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList ZkStateReader zkStateReader = ccc.getZkStateReader(); Set sourceNodes = getNodesFromParam(message, CollectionParams.SOURCE_NODES); Set targetNodes = getNodesFromParam(message, CollectionParams.TARGET_NODES); - boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, false); + boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, true); if (sourceNodes.isEmpty()) { throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "sourceNodes is a required param"); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java index 634b23746335..2f1ce62920f1 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java @@ -72,7 +72,7 @@ private void moveReplica( CollectionHandlingUtils.checkRequired(message, COLLECTION_PROP, CollectionParams.TARGET_NODE); String extCollection = message.getStr(COLLECTION_PROP); String targetNode = message.getStr(CollectionParams.TARGET_NODE); - boolean waitForFinalState = message.getBool(WAIT_FOR_FINAL_STATE, false); + boolean waitForFinalState = message.getBool(WAIT_FOR_FINAL_STATE, true); boolean inPlaceMove = message.getBool(IN_PLACE_MOVE, true); int timeout = message.getInt(TIMEOUT, 10 * 60); // 10 minutes diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/ReplaceNodeCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/ReplaceNodeCmd.java index 26a3730ca104..2eb72983fad6 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/ReplaceNodeCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/ReplaceNodeCmd.java @@ -47,7 +47,7 @@ public void call(AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList ZkStateReader zkStateReader = ccc.getZkStateReader(); String source = message.getStr(CollectionParams.SOURCE_NODE); String target = message.getStr(CollectionParams.TARGET_NODE); - boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, false); + boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, true); if (source == null) { throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "sourceNode is a required param"); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java index 6740c5ba046d..4ddc265338a4 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java @@ -136,7 +136,7 @@ public void call(AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList public boolean split( AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList results) throws Exception { - boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, false); + boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, true); String methodStr = message.getStr( CommonAdminParams.SPLIT_METHOD, SolrIndexSplitter.SplitMethod.REWRITE.toLower()); From 60507b6ce0220ca01bbf802776e90356e7a7c5b3 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Fri, 21 Aug 2026 15:54:35 +0300 Subject: [PATCH 3/9] SOLR-18367: add changelog entry --- .../unreleased/SOLR-18367-waitforfinalstate-removal.yml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 changelog/unreleased/SOLR-18367-waitforfinalstate-removal.yml diff --git a/changelog/unreleased/SOLR-18367-waitforfinalstate-removal.yml b/changelog/unreleased/SOLR-18367-waitforfinalstate-removal.yml new file mode 100644 index 000000000000..7af87ffbd3ef --- /dev/null +++ b/changelog/unreleased/SOLR-18367-waitforfinalstate-removal.yml @@ -0,0 +1,7 @@ +title: The `waitForFinalState` parameter now defaults to `true` for CREATE, ADDREPLICA, CREATESHARD, MOVEREPLICA, SPLITSHARD, BALANCE_REPLICAS, MIGRATE_REPLICAS, and REPLACENODE Collections API commands, so these commands wait for all affected replicas to become active before returning. Removed the now-unnecessary `CollectionAdminRequest.AsyncCollectionAdminRequest.setWaitForFinalState` SolrJ method, deprecated in 9.10 (SOLR-17712). +type: changed +authors: + - name: Serhiy Bzhezytskyy +links: + - name: SOLR-18367 + url: https://issues.apache.org/jira/browse/SOLR-18367 From 9144706bb36ffc4c2caeb22fa54c6b09c7ab7df6 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sun, 23 Aug 2026 12:53:31 +0300 Subject: [PATCH 4/9] SOLR-18367: fix CollectionAdminRequestRequiredParamsTest for the waitForFinalState default flip waitForFinalState now defaults to true and is unconditionally sent by every AsyncCollectionAdminRequest subclass, so it belongs in the expected param set for every request built on that base -- the test asserts an exact param match. --- ...lectionAdminRequestRequiredParamsTest.java | 93 ++++++++++++++----- 1 file changed, 68 insertions(+), 25 deletions(-) diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java index 942b0dbea48a..b216bb873383 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java @@ -17,6 +17,7 @@ package org.apache.solr.client.solrj; import static org.apache.solr.common.params.CollectionAdminParams.FLUSH; +import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE; import static org.apache.solr.common.params.CoreAdminParams.ACTION; import static org.apache.solr.common.params.CoreAdminParams.COLLECTION; import static org.apache.solr.common.params.CoreAdminParams.NAME; @@ -45,17 +46,30 @@ public class CollectionAdminRequestRequiredParamsTest extends SolrTestCase { public void testBalanceShardUnique() { CollectionAdminRequest.BalanceShardUnique request = CollectionAdminRequest.balanceReplicaProperty("foo", "prop"); - assertContainsParams(request.getParams(), ACTION, COLLECTION, "property"); + assertContainsParams(request.getParams(), ACTION, COLLECTION, "property", WAIT_FOR_FINAL_STATE); request.setShardUnique(true); - assertContainsParams(request.getParams(), ACTION, COLLECTION, "property", "shardUnique"); + assertContainsParams( + request.getParams(), ACTION, COLLECTION, "property", "shardUnique", WAIT_FOR_FINAL_STATE); request.setOnlyActiveNodes(false); assertContainsParams( - request.getParams(), ACTION, COLLECTION, "property", "shardUnique", "onlyactivenodes"); + request.getParams(), + ACTION, + COLLECTION, + "property", + "shardUnique", + "onlyactivenodes", + WAIT_FOR_FINAL_STATE); request.setShardUnique(null); - assertContainsParams(request.getParams(), ACTION, COLLECTION, "property", "onlyactivenodes"); + assertContainsParams( + request.getParams(), + ACTION, + COLLECTION, + "property", + "onlyactivenodes", + WAIT_FOR_FINAL_STATE); } public void testClusterProp() { @@ -72,17 +86,18 @@ public void testCollectionProp() { ACTION, CoreAdminParams.NAME, CollectionAdminParams.PROPERTY_NAME, - CollectionAdminParams.PROPERTY_VALUE); + CollectionAdminParams.PROPERTY_VALUE, + WAIT_FOR_FINAL_STATE); } public void testAddRole() { CollectionAdminRequest.AddRole request = CollectionAdminRequest.addRole("node", "role"); - assertContainsParams(request.getParams(), ACTION, "node", "role"); + assertContainsParams(request.getParams(), ACTION, "node", "role", WAIT_FOR_FINAL_STATE); } public void testRemoveRole() { CollectionAdminRequest.RemoveRole request = CollectionAdminRequest.removeRole("node", "role"); - assertContainsParams(request.getParams(), ACTION, "node", "role"); + assertContainsParams(request.getParams(), ACTION, "node", "role", WAIT_FOR_FINAL_STATE); } public void testAddReplica() { @@ -90,20 +105,31 @@ public void testAddReplica() { CollectionAdminRequest.AddReplica request = CollectionAdminRequest.addReplicaToShard("collection", "shard"); assertContainsParams( - request.getParams(), ACTION, COLLECTION, SHARD, ZkStateReader.REPLICA_TYPE); + request.getParams(), + ACTION, + COLLECTION, + SHARD, + ZkStateReader.REPLICA_TYPE, + WAIT_FOR_FINAL_STATE); // with only shard parameter and "server side" implicit type, so no param request = CollectionAdminRequest.addReplicaToShard("collection", "shard", null); - assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, WAIT_FOR_FINAL_STATE); // with route parameter request = CollectionAdminRequest.addReplicaByRouteKey("collection", "route"); - assertContainsParams(request.getParams(), ACTION, COLLECTION, ShardParams._ROUTE_); + assertContainsParams( + request.getParams(), ACTION, COLLECTION, ShardParams._ROUTE_, WAIT_FOR_FINAL_STATE); // with explicit type parameter request = CollectionAdminRequest.addReplicaToShard("collection", "shard", Replica.Type.NRT); assertContainsParams( - request.getParams(), ACTION, COLLECTION, SHARD, ZkStateReader.REPLICA_TYPE); + request.getParams(), + ACTION, + COLLECTION, + SHARD, + ZkStateReader.REPLICA_TYPE, + WAIT_FOR_FINAL_STATE); } public void testAddReplicaProp() { @@ -111,7 +137,14 @@ public void testAddReplicaProp() { CollectionAdminRequest.addReplicaProperty( "collection", "shard", "replica", "prop", "value"); assertContainsParams( - request.getParams(), ACTION, COLLECTION, SHARD, REPLICA, "property", "property.value"); + request.getParams(), + ACTION, + COLLECTION, + SHARD, + REPLICA, + "property", + "property.value", + WAIT_FOR_FINAL_STATE); } public void testClusterStatus() { @@ -132,31 +165,33 @@ public void testClusterStatus() { public void testCreateShard() { final CollectionAdminRequest.CreateShard request = CollectionAdminRequest.createShard("collection", "shard"); - assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, WAIT_FOR_FINAL_STATE); } public void testDeleteReplica() { final CollectionAdminRequest.DeleteReplica request = CollectionAdminRequest.deleteReplica("collection", "shard", "replica"); - assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, REPLICA); + assertContainsParams( + request.getParams(), ACTION, COLLECTION, SHARD, REPLICA, WAIT_FOR_FINAL_STATE); } public void testDeleteReplicaProp() { final CollectionAdminRequest.DeleteReplicaProp request = CollectionAdminRequest.deleteReplicaProperty("collection", "shard", "replica", "foo"); - assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, REPLICA, "property"); + assertContainsParams( + request.getParams(), ACTION, COLLECTION, SHARD, REPLICA, "property", WAIT_FOR_FINAL_STATE); } public void testDeleteShard() { final CollectionAdminRequest.DeleteShard request = CollectionAdminRequest.deleteShard("collection", "shard"); - assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, WAIT_FOR_FINAL_STATE); } public void testSplitShard() { final CollectionAdminRequest.SplitShard request = CollectionAdminRequest.splitShard("collection").setShardName("shard"); - assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, WAIT_FOR_FINAL_STATE); } public void testCreateCollection() { @@ -166,7 +201,8 @@ public void testCreateCollection() { ACTION, NAME, ZkStateReader.NUM_SHARDS_PROP, - ZkStateReader.NRT_REPLICAS); + ZkStateReader.NRT_REPLICAS, + WAIT_FOR_FINAL_STATE); // shortest form w/ "explicitly" choosing "implicit" router assertContainsParams( CollectionAdminRequest.createCollectionWithImplicitRouter("foo", null, "bar", 1) @@ -175,30 +211,31 @@ public void testCreateCollection() { NAME, "shards", "router.name", - ZkStateReader.NRT_REPLICAS); + ZkStateReader.NRT_REPLICAS, + WAIT_FOR_FINAL_STATE); } public void testReloadCollection() { final CollectionAdminRequest.Reload request = CollectionAdminRequest.reloadCollection("collection"); - assertContainsParams(request.getParams(), ACTION, NAME); + assertContainsParams(request.getParams(), ACTION, NAME, WAIT_FOR_FINAL_STATE); } public void testDeleteCollection() { final CollectionAdminRequest.Delete request = CollectionAdminRequest.deleteCollection("collection"); - assertContainsParams(request.getParams(), ACTION, NAME); + assertContainsParams(request.getParams(), ACTION, NAME, WAIT_FOR_FINAL_STATE); } public void testCreateAlias() { final CollectionAdminRequest.CreateAlias request = CollectionAdminRequest.createAlias("name", "collections"); - assertContainsParams(request.getParams(), ACTION, NAME, "collections"); + assertContainsParams(request.getParams(), ACTION, NAME, "collections", WAIT_FOR_FINAL_STATE); } public void testDeleteAlias() { final CollectionAdminRequest.DeleteAlias request = CollectionAdminRequest.deleteAlias("name"); - assertContainsParams(request.getParams(), ACTION, NAME); + assertContainsParams(request.getParams(), ACTION, NAME, WAIT_FOR_FINAL_STATE); } public void testListCollections() { @@ -209,13 +246,19 @@ public void testListCollections() { public void testMigrate() { final CollectionAdminRequest.Migrate request = CollectionAdminRequest.migrateData("collection", "target", "splitKey"); - assertContainsParams(request.getParams(), ACTION, COLLECTION, "target.collection", "split.key"); + assertContainsParams( + request.getParams(), + ACTION, + COLLECTION, + "target.collection", + "split.key", + WAIT_FOR_FINAL_STATE); } public void testOverseerStatus() { final CollectionAdminRequest.OverseerStatus request = new CollectionAdminRequest.OverseerStatus(); - assertContainsParams(request.getParams(), ACTION); + assertContainsParams(request.getParams(), ACTION, WAIT_FOR_FINAL_STATE); } public void testRequestStatus() { From d942cfdaef8404d9b5a968182c4b44c0e8e59214 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sat, 22 Aug 2026 12:30:15 +0300 Subject: [PATCH 5/9] SOLR-18367: fix regression -- pin waitForFinalState=false on internal Cmd calls Full-suite regression run found AbstractCloudBackupRestoreTestCase.testRestoreFailure now failing (confirmed passing on the pre-redesign base commit): RestoreCmd builds its own synthetic ZkNodeProps messages for CreateCollectionCmd/AddReplicaCmd calls that never set waitForFinalState, so they silently inherited the new true default instead of the old false one, breaking restore's own 'no race, return immediately' assumption and its own completion-latch-driven waiting. Audited every other internal call site that constructs one of the 8 Cmd classes directly (bypassing client-request dispatch) for the same gap. Found 2 more real correctness bugs: MoveReplicaCmd.moveNormalReplica and ReplicaMigrationUtils.migrateReplicas each implement their own ActiveReplicaWatcher/latch wait conditioned on their own waitForFinalState parameter -- the new default made AddReplicaCmd wait internally first, defeating the intended fast path when the caller didn't request a wait. And 2 latency-only cases in MigrateCmd's temp-collection creation, which already does its own leader/sync waits downstream. Every one of these callers now explicitly pins waitForFinalState=false in the message it builds, restoring the pre-redesign contract. Confirmed via testRestoreFailure, which now passes in ~5s (previously exceeded its 30s poll window). --- .../apache/solr/cloud/api/collections/MigrateCmd.java | 10 +++++++++- .../solr/cloud/api/collections/MoveReplicaCmd.java | 6 +++++- .../cloud/api/collections/ReplicaMigrationUtils.java | 6 +++++- .../apache/solr/cloud/api/collections/RestoreCmd.java | 10 ++++++++++ 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateCmd.java index 2869f9866b6e..1f0771e01cfb 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateCmd.java @@ -46,6 +46,7 @@ import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.CollectionAdminParams; +import org.apache.solr.common.params.CommonAdminParams; import org.apache.solr.common.params.CoreAdminParams; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.NamedList; @@ -308,7 +309,11 @@ private void migrateKey( CollectionAdminParams.COLL_CONF, configName, CollectionHandlingUtils.CREATE_NODE_SET, - sourceLeader.getNodeName()); + sourceLeader.getNodeName(), + // the getLeaderRetry(...) call below is this method's own wait; don't let + // CreateCollectionCmd's own wait run (and block on) first. + CommonAdminParams.WAIT_FOR_FINAL_STATE, + "false"); String internalAsyncId = null; if (adminCmdContext.getAsyncId() != null) { internalAsyncId = adminCmdContext.getAsyncId() + Math.abs(System.nanoTime()); @@ -400,6 +405,9 @@ private void migrateKey( props.put(SHARD_ID_PROP, tempSourceSlice.getName()); props.put("node", targetLeader.getNodeName()); props.put(CoreAdminParams.NAME, tempCollectionReplica2); + // the syncRequestTracker below is this method's own wait; don't let AddReplicaCmd's own + // wait run (and block on) first. + props.put(CommonAdminParams.WAIT_FOR_FINAL_STATE, "false"); // copy over property params: for (String key : message.keySet()) { if (key.startsWith(CollectionAdminParams.PROPERTY_PREFIX)) { diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java index 2f1ce62920f1..4e8563bcfeeb 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java @@ -373,7 +373,11 @@ private void moveNormalReplica( CoreAdminParams.NAME, newCoreName, ZkStateReader.REPLICA_TYPE, - replica.getType().name()); + replica.getType().name(), + // this method has its own watcher/latch below, conditioned on waitForFinalState; + // don't let AddReplicaCmd's own wait run (and block on) first. + WAIT_FOR_FINAL_STATE, + "false"); NamedList addResult = new NamedList<>(); SolrCloseableLatch countDownLatch = new SolrCloseableLatch(1, ccc.getCloseableToLatchOn()); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/ReplicaMigrationUtils.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/ReplicaMigrationUtils.java index be08e6fc573c..79ee6677d849 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/ReplicaMigrationUtils.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/ReplicaMigrationUtils.java @@ -35,6 +35,7 @@ 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.CommonAdminParams; import org.apache.solr.common.params.CoreAdminParams; import org.apache.solr.common.util.NamedList; import org.apache.zookeeper.KeeperException; @@ -106,7 +107,10 @@ static boolean migrateReplicas( sourceReplica .toFullProps() .plus("parallel", String.valueOf(parallel)) - .plus(CoreAdminParams.NODE, targetNode); + .plus(CoreAdminParams.NODE, targetNode) + // this method has its own watcher/latch below, conditioned on waitForFinalState; + // don't let AddReplicaCmd's own wait run (and block on) first. + .plus(CommonAdminParams.WAIT_FOR_FINAL_STATE, "false"); NamedList nl = new NamedList<>(); final ZkNodeProps addedReplica = new AddReplicaCmd(ccc) diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java index dabaf64420e4..89e1d3f3628b 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java @@ -65,6 +65,7 @@ import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.CollectionAdminParams; import org.apache.solr.common.params.CollectionParams; +import org.apache.solr.common.params.CommonAdminParams; import org.apache.solr.common.params.CoreAdminParams; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.CollectionUtil; @@ -358,6 +359,9 @@ private void createCoreLessCollection( propMap.put( CollectionHandlingUtils.CREATE_NODE_SET, CollectionHandlingUtils.CREATE_NODE_SET_EMPTY); // no cores + // no cores are created here (see above), but keep this call's contract independent of the + // operator-configurable default regardless -- restore does its own waiting elsewhere. + propMap.put(CommonAdminParams.WAIT_FOR_FINAL_STATE, "false"); propMap.put(CollectionAdminParams.COLL_CONF, restoreConfigName); // router.* @@ -452,6 +456,9 @@ private void createSingleReplicaPerShard( propMap.put(COLLECTION_PROP, restoreCollection.getName()); propMap.put(SHARD_ID_PROP, sliceName); propMap.put(REPLICA_TYPE, numReplicas.getLeaderType().name()); + // the onComplete callback below drives this method's own countDownLatch; don't let + // AddReplicaCmd's own wait run (and block on) first. + propMap.put(CommonAdminParams.WAIT_FOR_FINAL_STATE, "false"); // Get the first node matching the shard to restore in String node; @@ -567,6 +574,9 @@ private void addReplicasToShards( propMap.put(COLLECTION_PROP, restoreCollection.getName()); propMap.put(SHARD_ID_PROP, slice.getName()); propMap.put(REPLICA_TYPE, typeToCreate.name()); + // restore does its own waiting elsewhere; don't let AddReplicaCmd's own wait + // (bounded by its default 10-minute timeout, per shard) run here instead. + propMap.put(CommonAdminParams.WAIT_FOR_FINAL_STATE, "false"); // Get the first node matching the shard to restore in String node; From 709ad0753759869f373c1f79fee3d9815ba0c9df Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Sat, 22 Aug 2026 13:32:27 +0300 Subject: [PATCH 6/9] SOLR-18367: fix regression -- pin waitForFinalState=false on SplitShardWithNodeRoleTest Both test methods now hang and hit the client's 90s HTTP timeout: in this NodeRole-constrained (data:off) cluster with a PULL replica, SPLITSHARD's new true default makes the server wait internally for full activation, which never completes within the client's timeout budget. Confirmed via baseline comparison this is a pre-existing latent bug, not introduced by this redesign: forcing waitForFinalState=true explicitly on pre-SOLR-18367 main reproduces the identical hang (still RUNNING after 300s). The default flip only newly exposes it, since it was previously unreachable with the old false default. Pinning false here is correct for the same reason as the other 7 internal call sites: this test does its own explicit waitForState(...) polling afterward and never needed the wait. --- .../cloud/SplitShardWithNodeRoleTest.java | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/cloud/SplitShardWithNodeRoleTest.java b/solr/core/src/test/org/apache/solr/cloud/SplitShardWithNodeRoleTest.java index eb15bd444209..9530bb34ca03 100644 --- a/solr/core/src/test/org/apache/solr/cloud/SplitShardWithNodeRoleTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/SplitShardWithNodeRoleTest.java @@ -21,8 +21,15 @@ import java.util.Set; import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.CollectionAdminRequest.AsyncCollectionAdminRequest; import org.apache.solr.client.solrj.request.UpdateRequest; import org.apache.solr.common.SolrInputDocument; +import org.apache.solr.common.params.CollectionAdminParams; +import org.apache.solr.common.params.CollectionParams.CollectionAction; +import org.apache.solr.common.params.CommonAdminParams; +import org.apache.solr.common.params.CoreAdminParams; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.params.SolrParams; import org.apache.solr.core.NodeRoles; import org.apache.solr.util.LogLevel; import org.junit.BeforeClass; @@ -85,10 +92,20 @@ public void doSplit(String collName, int shard, int nrtReplica, int pullReplica) ur.commit(client, collName); final int numSubShards = 2; - CollectionAdminRequest.SplitShard splitShard = - CollectionAdminRequest.splitShard(collName) - .setShardName("shard1") - .setNumSubShards(numSubShards); + // this method does its own explicit waitForState(...) below; pin waitForFinalState=false + // so SPLITSHARD's own new default doesn't also wait (and risk the client's HTTP timeout). + AsyncCollectionAdminRequest splitShard = + new AsyncCollectionAdminRequest(CollectionAction.SPLITSHARD) { + @Override + public SolrParams getParams() { + ModifiableSolrParams params = (ModifiableSolrParams) super.getParams(); + params.set(CollectionAdminParams.COLLECTION, collName); + params.set(CoreAdminParams.SHARD, "shard1"); + params.set("numSubShards", numSubShards); + params.set(CommonAdminParams.WAIT_FOR_FINAL_STATE, false); + return params; + } + }; splitShard.process(cluster.getSolrClient()); int totalShards = shard + (numSubShards - 1); waitForState( From 01f4f2e53cb74fc36b1c3c3be8235c2a8cab90fa Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Mon, 24 Aug 2026 14:35:06 +0300 Subject: [PATCH 7/9] SOLR-18367: default waitForFinalState via a per-command, operator-configurable flag Adds the global flag David asked for on #3684, instead of hardcoding one default for all 8 commands. CREATE, ADDREPLICA, CREATESHARD, MOVEREPLICA, SPLITSHARD default to true; BALANCE_REPLICAS, MIGRATE_REPLICAS, REPLACENODE keep false, since their wait is unbounded (their latch scales with movements.size(), not a fixed count). The flag overrides all 8 either way. Also removes the deprecated SolrJ setter and the v2 REST field for the 3 unbounded commands. --- ...R-18367-waitforfinalstate-default-true.yml | 16 +++ .../SOLR-18367-waitforfinalstate-removal.yml | 7 - .../api/model/BalanceReplicasRequestBody.java | 12 +- .../api/model/MigrateReplicasRequestBody.java | 12 +- .../api/model/ReplaceNodeRequestBody.java | 12 +- .../cloud/api/collections/AddReplicaCmd.java | 5 +- .../api/collections/BalanceReplicasCmd.java | 7 +- .../collections/CollectionHandlingUtils.java | 12 ++ .../api/collections/CreateCollectionCmd.java | 5 +- .../cloud/api/collections/CreateShardCmd.java | 7 +- .../api/collections/MigrateReplicasCmd.java | 7 +- .../cloud/api/collections/MoveReplicaCmd.java | 5 +- .../cloud/api/collections/ReplaceNodeCmd.java | 7 +- .../cloud/api/collections/SplitShardCmd.java | 7 +- .../handler/admin/CollectionsHandler.java | 1 - .../handler/admin/api/BalanceReplicas.java | 2 - .../handler/admin/api/MigrateReplicas.java | 2 - .../solr/handler/admin/api/ReplaceNode.java | 2 - .../solr/cloud/BalanceReplicasTest.java | 11 +- .../solr/cloud/MigrateReplicasTest.java | 26 +++- .../CollectionHandlingUtilsTest.java | 74 ++++++++++ .../WaitForFinalStateEnvFallbackTest.java | 132 ++++++++++++++++++ .../admin/api/MigrateReplicasAPITest.java | 11 +- .../handler/admin/api/ReplaceNodeAPITest.java | 11 +- ...FinalStateRequestBodyOmittedFieldTest.java | 43 ++++++ .../pages/cluster-node-management.adoc | 4 +- .../pages/collection-management.adoc | 5 +- .../pages/replica-management.adoc | 3 +- .../pages/shard-management.adoc | 6 +- .../solrj/request/CollectionAdminRequest.java | 4 - .../solr/common/params/CommonAdminParams.java | 10 ++ ...lectionAdminRequestRequiredParamsTest.java | 93 ++++-------- 32 files changed, 407 insertions(+), 154 deletions(-) create mode 100644 changelog/unreleased/SOLR-18367-waitforfinalstate-default-true.yml delete mode 100644 changelog/unreleased/SOLR-18367-waitforfinalstate-removal.yml create mode 100644 solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionHandlingUtilsTest.java create mode 100644 solr/core/src/test/org/apache/solr/cloud/api/collections/WaitForFinalStateEnvFallbackTest.java create mode 100644 solr/core/src/test/org/apache/solr/handler/admin/api/WaitForFinalStateRequestBodyOmittedFieldTest.java diff --git a/changelog/unreleased/SOLR-18367-waitforfinalstate-default-true.yml b/changelog/unreleased/SOLR-18367-waitforfinalstate-default-true.yml new file mode 100644 index 000000000000..cb8f1d459e7c --- /dev/null +++ b/changelog/unreleased/SOLR-18367-waitforfinalstate-default-true.yml @@ -0,0 +1,16 @@ +# See https://github.com/apache/solr/blob/main/dev-docs/changelog.adoc +title: > + `waitForFinalState` now defaults to `true` for CREATE, ADDREPLICA, CREATESHARD, SPLITSHARD, and + MOVEREPLICA, so these Collections API commands wait for affected replicas to become active before + returning. BALANCE_REPLICAS, MIGRATE_REPLICAS, and REPLACENODE keep the previous `false` default, + since they can each affect an arbitrary number of replicas cluster-wide. Set the system property + `solr.cloud.waitForFinalState.enabled` on a Solr node to override the default (either direction) + cluster-wide, for all 8 commands. +type: changed +authors: + - name: Serhiy Bzhezytskyy +links: + - name: SOLR-18367 + url: https://issues.apache.org/jira/browse/SOLR-18367 + - name: SOLR-17712 + url: https://issues.apache.org/jira/browse/SOLR-17712 diff --git a/changelog/unreleased/SOLR-18367-waitforfinalstate-removal.yml b/changelog/unreleased/SOLR-18367-waitforfinalstate-removal.yml deleted file mode 100644 index 7af87ffbd3ef..000000000000 --- a/changelog/unreleased/SOLR-18367-waitforfinalstate-removal.yml +++ /dev/null @@ -1,7 +0,0 @@ -title: The `waitForFinalState` parameter now defaults to `true` for CREATE, ADDREPLICA, CREATESHARD, MOVEREPLICA, SPLITSHARD, BALANCE_REPLICAS, MIGRATE_REPLICAS, and REPLACENODE Collections API commands, so these commands wait for all affected replicas to become active before returning. Removed the now-unnecessary `CollectionAdminRequest.AsyncCollectionAdminRequest.setWaitForFinalState` SolrJ method, deprecated in 9.10 (SOLR-17712). -type: changed -authors: - - name: Serhiy Bzhezytskyy -links: - - name: SOLR-18367 - url: https://issues.apache.org/jira/browse/SOLR-18367 diff --git a/solr/api/src/java/org/apache/solr/client/api/model/BalanceReplicasRequestBody.java b/solr/api/src/java/org/apache/solr/client/api/model/BalanceReplicasRequestBody.java index 508e5288cd43..d7458d16df07 100644 --- a/solr/api/src/java/org/apache/solr/client/api/model/BalanceReplicasRequestBody.java +++ b/solr/api/src/java/org/apache/solr/client/api/model/BalanceReplicasRequestBody.java @@ -24,9 +24,8 @@ public class BalanceReplicasRequestBody { public BalanceReplicasRequestBody() {} - public BalanceReplicasRequestBody(Set nodes, Boolean waitForFinalState, String async) { + public BalanceReplicasRequestBody(Set nodes, String async) { this.nodes = nodes; - this.waitForFinalState = waitForFinalState; this.async = async; } @@ -36,15 +35,6 @@ public BalanceReplicasRequestBody(Set nodes, Boolean waitForFinalState, @JsonProperty(value = "nodes") public Set nodes; - @Schema( - description = - "If true, the request will complete only when all affected replicas become active. " - + "If false, the API will return the status of the single action, which may be " - + "before the new replica is online and active.") - @JsonProperty("waitForFinalState") - @Deprecated(since = "9.10") - public Boolean waitForFinalState = false; - @Schema(description = "Request ID to track this action which will be processed asynchronously.") @JsonProperty("async") public String async; diff --git a/solr/api/src/java/org/apache/solr/client/api/model/MigrateReplicasRequestBody.java b/solr/api/src/java/org/apache/solr/client/api/model/MigrateReplicasRequestBody.java index a0a7e3afd9b8..a4e00a59561c 100644 --- a/solr/api/src/java/org/apache/solr/client/api/model/MigrateReplicasRequestBody.java +++ b/solr/api/src/java/org/apache/solr/client/api/model/MigrateReplicasRequestBody.java @@ -25,10 +25,9 @@ public class MigrateReplicasRequestBody { public MigrateReplicasRequestBody() {} public MigrateReplicasRequestBody( - Set sourceNodes, Set targetNodes, Boolean waitForFinalState, String async) { + Set sourceNodes, Set targetNodes, String async) { this.sourceNodes = sourceNodes; this.targetNodes = targetNodes; - this.waitForFinalState = waitForFinalState; this.async = async; } @@ -42,15 +41,6 @@ public MigrateReplicasRequestBody( @JsonProperty public Set targetNodes; - @Schema( - description = - "If true, the request will complete only when all affected replicas become active. " - + "If false, the API will return the status of the single action, which may be " - + "before the new replicas are online and active.") - @JsonProperty - @Deprecated(since = "9.10") - public Boolean waitForFinalState = false; - @Schema(description = "Request ID to track this action which will be processed asynchronously.") @JsonProperty public String async; diff --git a/solr/api/src/java/org/apache/solr/client/api/model/ReplaceNodeRequestBody.java b/solr/api/src/java/org/apache/solr/client/api/model/ReplaceNodeRequestBody.java index 95f7cb380d2c..0ee145c918dd 100644 --- a/solr/api/src/java/org/apache/solr/client/api/model/ReplaceNodeRequestBody.java +++ b/solr/api/src/java/org/apache/solr/client/api/model/ReplaceNodeRequestBody.java @@ -23,9 +23,8 @@ public class ReplaceNodeRequestBody { public ReplaceNodeRequestBody() {} - public ReplaceNodeRequestBody(String targetNodeName, Boolean waitForFinalState, String async) { + public ReplaceNodeRequestBody(String targetNodeName, String async) { this.targetNodeName = targetNodeName; - this.waitForFinalState = waitForFinalState; this.async = async; } @@ -36,15 +35,6 @@ public ReplaceNodeRequestBody(String targetNodeName, Boolean waitForFinalState, @JsonProperty("targetNodeName") public String targetNodeName; - @Schema( - description = - "If true, the request will complete only when all affected replicas become active. " - + "If false, the API will return the status of the single action, which may be " - + "before the new replica is online and active.") - @JsonProperty("waitForFinalState") - @Deprecated(since = "9.10") - public Boolean waitForFinalState = false; - @Schema(description = "Request ID to track this action which will be processed asynchronously.") @JsonProperty("async") public String async; diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java index 81772ac698fa..a308f92d4b04 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java @@ -27,6 +27,7 @@ import static org.apache.solr.common.params.CollectionParams.CollectionAction.ADDREPLICA; import static org.apache.solr.common.params.CommonAdminParams.TIMEOUT; import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE; +import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE_DEFAULT_PROP; import java.io.IOException; import java.lang.invoke.MethodHandles; @@ -113,7 +114,9 @@ List addReplica( "Collection: " + collectionName + " shard: " + shard + " does not exist"); } - boolean waitForFinalState = message.getBool(WAIT_FOR_FINAL_STATE, true); + boolean waitForFinalState = + CollectionHandlingUtils.getBoolWithEnvFallback( + message, WAIT_FOR_FINAL_STATE, WAIT_FOR_FINAL_STATE_DEFAULT_PROP, true); boolean skipCreateReplicaInClusterState = message.getBool(SKIP_CREATE_REPLICA_IN_CLUSTER_STATE, false); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/BalanceReplicasCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/BalanceReplicasCmd.java index 75ffe62b67d9..6075de803189 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/BalanceReplicasCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/BalanceReplicasCmd.java @@ -55,7 +55,12 @@ public void call(AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList "'nodes' was not passed as a correct type (Set/List/String): " + nodesRaw.getClass().getName()); } - boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, true); + boolean waitForFinalState = + CollectionHandlingUtils.getBoolWithEnvFallback( + message, + CommonAdminParams.WAIT_FOR_FINAL_STATE, + CommonAdminParams.WAIT_FOR_FINAL_STATE_DEFAULT_PROP, + false); int timeout = message.getInt("timeout", 10 * 60); // 10 minutes boolean parallel = message.getBool("parallel", false); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java index 02320e1e8541..d7dbde98f7b3 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CollectionHandlingUtils.java @@ -61,6 +61,7 @@ import org.apache.solr.common.params.CollectionAdminParams; import org.apache.solr.common.params.CoreAdminParams; import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.util.EnvUtils; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.common.util.StrUtils; @@ -140,6 +141,17 @@ public static EnumSet leaderEligibleReplicaTypes() { .collect(Collectors.toCollection(() -> EnumSet.noneOf(Replica.Type.class))); } + /** + * Reads a boolean request param, falling back to a node-level system property when the request + * doesn't specify one, and finally to {@code defaultValue} when neither is set. Lets an operator + * override a per-request default cluster-wide (e.g. via {@code -D=false} at node + * startup) without a client change -- same shape as {@code CreateCollectionCmd.PRS_DEFAULT_PROP}. + */ + static boolean getBoolWithEnvFallback( + ZkNodeProps message, String messageParam, String envProp, boolean defaultValue) { + return message.getBool(messageParam, EnvUtils.getPropertyAsBool(envProp, defaultValue)); + } + static boolean waitForCoreNodeGone( String collectionName, String shard, diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java index fa7633cc1736..707052335484 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java @@ -24,6 +24,7 @@ import static org.apache.solr.common.params.CollectionParams.CollectionAction.DELETE; import static org.apache.solr.common.params.CommonAdminParams.ASYNC; import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE; +import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE_DEFAULT_PROP; import static org.apache.solr.common.params.CommonParams.NAME; import static org.apache.solr.common.util.StrUtils.formatString; import static org.apache.solr.handler.admin.ConfigSetsHandler.DEFAULT_CONFIGSET_NAME; @@ -111,7 +112,9 @@ public void call(AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList ClusterState clusterState = adminCmdContext.getClusterState(); final Aliases aliases = ccc.getZkStateReader().getAliases(); final String collectionName = message.getStr(NAME); - final boolean waitForFinalState = message.getBool(WAIT_FOR_FINAL_STATE, true); + final boolean waitForFinalState = + CollectionHandlingUtils.getBoolWithEnvFallback( + message, WAIT_FOR_FINAL_STATE, WAIT_FOR_FINAL_STATE_DEFAULT_PROP, true); final String alias = message.getStr(ALIAS, collectionName); log.info("Create collection {}", collectionName); boolean prsDefault = EnvUtils.getPropertyAsBool(PRS_DEFAULT_PROP, false); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateShardCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateShardCmd.java index 7176a830b083..ae95885a2fdb 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateShardCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateShardCmd.java @@ -50,7 +50,12 @@ public void call(AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList throws Exception { String extCollectionName = message.getStr(COLLECTION_PROP); String sliceName = message.getStr(SHARD_ID_PROP); - boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, true); + boolean waitForFinalState = + CollectionHandlingUtils.getBoolWithEnvFallback( + message, + CommonAdminParams.WAIT_FOR_FINAL_STATE, + CommonAdminParams.WAIT_FOR_FINAL_STATE_DEFAULT_PROP, + true); log.info("Create shard invoked: {}", message); if (extCollectionName == null || sliceName == null) diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateReplicasCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateReplicasCmd.java index fa3b4a2f1127..281fe54847b6 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateReplicasCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/MigrateReplicasCmd.java @@ -50,7 +50,12 @@ public void call(AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList ZkStateReader zkStateReader = ccc.getZkStateReader(); Set sourceNodes = getNodesFromParam(message, CollectionParams.SOURCE_NODES); Set targetNodes = getNodesFromParam(message, CollectionParams.TARGET_NODES); - boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, true); + boolean waitForFinalState = + CollectionHandlingUtils.getBoolWithEnvFallback( + message, + CommonAdminParams.WAIT_FOR_FINAL_STATE, + CommonAdminParams.WAIT_FOR_FINAL_STATE_DEFAULT_PROP, + false); if (sourceNodes.isEmpty()) { throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "sourceNodes is a required param"); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java index 4e8563bcfeeb..64bae6887436 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java @@ -25,6 +25,7 @@ import static org.apache.solr.common.params.CommonAdminParams.IN_PLACE_MOVE; import static org.apache.solr.common.params.CommonAdminParams.TIMEOUT; import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE; +import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE_DEFAULT_PROP; import java.lang.invoke.MethodHandles; import java.util.ArrayList; @@ -72,7 +73,9 @@ private void moveReplica( CollectionHandlingUtils.checkRequired(message, COLLECTION_PROP, CollectionParams.TARGET_NODE); String extCollection = message.getStr(COLLECTION_PROP); String targetNode = message.getStr(CollectionParams.TARGET_NODE); - boolean waitForFinalState = message.getBool(WAIT_FOR_FINAL_STATE, true); + boolean waitForFinalState = + CollectionHandlingUtils.getBoolWithEnvFallback( + message, WAIT_FOR_FINAL_STATE, WAIT_FOR_FINAL_STATE_DEFAULT_PROP, true); boolean inPlaceMove = message.getBool(IN_PLACE_MOVE, true); int timeout = message.getInt(TIMEOUT, 10 * 60); // 10 minutes diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/ReplaceNodeCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/ReplaceNodeCmd.java index 2eb72983fad6..3b61b0ccc358 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/ReplaceNodeCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/ReplaceNodeCmd.java @@ -47,7 +47,12 @@ public void call(AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList ZkStateReader zkStateReader = ccc.getZkStateReader(); String source = message.getStr(CollectionParams.SOURCE_NODE); String target = message.getStr(CollectionParams.TARGET_NODE); - boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, true); + boolean waitForFinalState = + CollectionHandlingUtils.getBoolWithEnvFallback( + message, + CommonAdminParams.WAIT_FOR_FINAL_STATE, + CommonAdminParams.WAIT_FOR_FINAL_STATE_DEFAULT_PROP, + false); if (source == null) { throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, "sourceNode is a required param"); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java index 4ddc265338a4..5f0082514217 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java @@ -136,7 +136,12 @@ public void call(AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList public boolean split( AdminCmdContext adminCmdContext, ZkNodeProps message, NamedList results) throws Exception { - boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, true); + boolean waitForFinalState = + CollectionHandlingUtils.getBoolWithEnvFallback( + message, + CommonAdminParams.WAIT_FOR_FINAL_STATE, + CommonAdminParams.WAIT_FOR_FINAL_STATE_DEFAULT_PROP, + true); String methodStr = message.getStr( CommonAdminParams.SPLIT_METHOD, SolrIndexSplitter.SplitMethod.REWRITE.toLower()); 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 6e3c9e453d78..716713d7dd77 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 @@ -1164,7 +1164,6 @@ public Map execute( final RequiredSolrParams requiredParams = req.getParams().required(); final var requestBody = new ReplaceNodeRequestBody(); requestBody.targetNodeName = params.get(TARGET_NODE); - requestBody.waitForFinalState = params.getBool(WAIT_FOR_FINAL_STATE); requestBody.async = params.get(ASYNC); final ReplaceNode replaceNodeAPI = new ReplaceNode(h.coreContainer, req, rsp); final SolrJerseyResponse replaceNodeResponse = diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/BalanceReplicas.java b/solr/core/src/java/org/apache/solr/handler/admin/api/BalanceReplicas.java index 8ee0a7e4e771..b95f9aa7908f 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/BalanceReplicas.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/BalanceReplicas.java @@ -17,7 +17,6 @@ package org.apache.solr.handler.admin.api; import static org.apache.solr.common.params.CollectionParams.NODES; -import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE; import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM; import jakarta.inject.Inject; @@ -65,7 +64,6 @@ public ZkNodeProps createRemoteMessage(BalanceReplicasRequestBody requestBody) { final Map remoteMessage = new HashMap<>(); if (requestBody != null) { insertIfNotNull(remoteMessage, NODES, requestBody.nodes); - insertIfNotNull(remoteMessage, WAIT_FOR_FINAL_STATE, requestBody.waitForFinalState); } return new ZkNodeProps(remoteMessage); diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/MigrateReplicas.java b/solr/core/src/java/org/apache/solr/handler/admin/api/MigrateReplicas.java index ad9b96040801..90c55c299f15 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/MigrateReplicas.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/MigrateReplicas.java @@ -18,7 +18,6 @@ import static org.apache.solr.common.params.CollectionParams.SOURCE_NODES; import static org.apache.solr.common.params.CollectionParams.TARGET_NODES; -import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE; import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM; import jakarta.inject.Inject; @@ -73,7 +72,6 @@ public ZkNodeProps createRemoteMessage(MigrateReplicasRequestBody requestBody) { } insertIfNotNull(remoteMessage, SOURCE_NODES, requestBody.sourceNodes); insertIfNotNull(remoteMessage, TARGET_NODES, requestBody.targetNodes); - insertIfNotNull(remoteMessage, WAIT_FOR_FINAL_STATE, requestBody.waitForFinalState); } else { throw new SolrException( SolrException.ErrorCode.BAD_REQUEST, diff --git a/solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNode.java b/solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNode.java index cd1626590871..f2f204552355 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNode.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/api/ReplaceNode.java @@ -18,7 +18,6 @@ import static org.apache.solr.common.params.CollectionParams.SOURCE_NODE; import static org.apache.solr.common.params.CollectionParams.TARGET_NODE; -import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE; import static org.apache.solr.security.PermissionNameProvider.Name.COLL_EDIT_PERM; import jakarta.inject.Inject; @@ -71,7 +70,6 @@ public ZkNodeProps createRemoteMessage(String nodeName, ReplaceNodeRequestBody r remoteMessage.put(SOURCE_NODE, nodeName); if (requestBody != null) { insertIfValueNotNull(remoteMessage, TARGET_NODE, requestBody.targetNodeName); - insertIfValueNotNull(remoteMessage, WAIT_FOR_FINAL_STATE, requestBody.waitForFinalState); } return new ZkNodeProps(remoteMessage); diff --git a/solr/core/src/test/org/apache/solr/cloud/BalanceReplicasTest.java b/solr/core/src/test/org/apache/solr/cloud/BalanceReplicasTest.java index 7fb0d8ddb4fb..9e6411c78337 100644 --- a/solr/core/src/test/org/apache/solr/cloud/BalanceReplicasTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/BalanceReplicasTest.java @@ -139,8 +139,15 @@ public void testSomeNodes() throws Exception { postDataAndGetResponse( "/api/cluster/replicas/balance", - Utils.getReflectWriter( - new BalanceReplicasRequestBody(new HashSet<>(l.subList(1, 4)), true, null))); + Utils.getReflectWriter(new BalanceReplicasRequestBody(new HashSet<>(l.subList(1, 4)), null))); + + waitForState( + "Timed out waiting for replicas to be balanced across the target nodes", + coll, + (state) -> + state.getReplicas().stream().map(Replica::getNodeName).collect(Collectors.toSet()) + .size() + == 4); collection = cloudClient.getClusterState().getCollectionOrNull(coll, false); log.debug("### After balancing: {}", collection); diff --git a/solr/core/src/test/org/apache/solr/cloud/MigrateReplicasTest.java b/solr/core/src/test/org/apache/solr/cloud/MigrateReplicasTest.java index 1d4d431d6df2..fc5291daafb4 100644 --- a/solr/core/src/test/org/apache/solr/cloud/MigrateReplicasTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/MigrateReplicasTest.java @@ -117,11 +117,15 @@ public void test() throws Exception { Map response = callMigrateReplicas( new MigrateReplicasRequestBody( - Set.of(nodeToBeDecommissioned), Set.of(emptyNode), true, null)); + Set.of(nodeToBeDecommissioned), Set.of(emptyNode), null)); assertEquals( "MigrateReplicas request was unsuccessful", 0L, ((Map) response.get("responseHeader")).get("status")); + waitForState( + "Timed out waiting for replicas to be migrated off the decommissioned node", + coll, + (state) -> state.getReplicasOnNode(nodeToBeDecommissioned).isEmpty()); ZkStateReader zkStateReader = ZkStateReader.from(cloudClient); try (SolrClient coreClient = getHttpSolrClient(zkStateReader.getBaseUrlForNodeName(nodeToBeDecommissioned))) { @@ -144,11 +148,18 @@ public void test() throws Exception { response = callMigrateReplicas( new MigrateReplicasRequestBody( - Set.of(emptyNode), Set.of(nodeToBeDecommissioned), true, null)); + Set.of(emptyNode), Set.of(nodeToBeDecommissioned), null)); assertEquals( "MigrateReplicas request was unsuccessful", 0L, ((Map) response.get("responseHeader")).get("status")); + waitForState( + "Timed out waiting for replicas to be migrated back off the empty node and become active", + coll, + (state) -> + state.getReplicasOnNode(emptyNode).isEmpty() + && state.getReplicasOnNode(nodeToBeDecommissioned).stream() + .allMatch(r -> r.getState() == Replica.State.ACTIVE)); try (SolrClient coreClient = getHttpSolrClient(zkStateReader.getBaseUrlForNodeName(emptyNode))) { @@ -250,12 +261,17 @@ public void testWithNoTarget() throws Exception { log.info("### Before decommission: {}", initialCollection); Map response = callMigrateReplicas( - new MigrateReplicasRequestBody( - new HashSet<>(nodesToBeDecommissioned), Set.of(), true, null)); + new MigrateReplicasRequestBody(new HashSet<>(nodesToBeDecommissioned), Set.of(), null)); assertEquals( "MigrateReplicas request was unsuccessful", 0L, ((Map) response.get("responseHeader")).get("status")); + waitForState( + "Timed out waiting for replicas to be migrated off the decommissioned nodes", + coll, + (state) -> + nodesToBeDecommissioned.stream() + .allMatch(node -> state.getReplicasOnNode(node).isEmpty())); DocCollection collection = cloudClient.getClusterState().getCollectionOrNull(coll, false); assertNotNull("Collection cannot be null: " + coll, collection); @@ -294,7 +310,7 @@ public void testFailOnSingleNode() throws Exception { String liveNode = cloudClient.getClusterState().getLiveNodes().iterator().next(); Map response = - callMigrateReplicas(new MigrateReplicasRequestBody(Set.of(liveNode), Set.of(), true, null)); + callMigrateReplicas(new MigrateReplicasRequestBody(Set.of(liveNode), Set.of(), null)); assertNotNull( "No error in response, when the request should have failed", response.get("error")); assertEquals( diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionHandlingUtilsTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionHandlingUtilsTest.java new file mode 100644 index 000000000000..782458eb2dbe --- /dev/null +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionHandlingUtilsTest.java @@ -0,0 +1,74 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.cloud.api.collections; + +import java.util.Collections; +import org.apache.solr.SolrTestCase; +import org.apache.solr.common.cloud.ZkNodeProps; +import org.junit.After; +import org.junit.Test; + +public class CollectionHandlingUtilsTest extends SolrTestCase { + + private static final String MESSAGE_PARAM = "waitForFinalState"; + private static final String ENV_PROP = "solr.cloud.waitForFinalStateEnvFallbackTest.enabled"; + + @After + public void clearProperty() { + System.clearProperty(ENV_PROP); + } + + @Test + public void testMessageUnsetEnvUnsetFallsBackToDefault() { + ZkNodeProps message = new ZkNodeProps(Collections.emptyMap()); + assertTrue( + CollectionHandlingUtils.getBoolWithEnvFallback(message, MESSAGE_PARAM, ENV_PROP, true)); + assertFalse( + CollectionHandlingUtils.getBoolWithEnvFallback(message, MESSAGE_PARAM, ENV_PROP, false)); + } + + @Test + public void testMessageUnsetEnvSetFalseOverridesDefaultTrue() { + System.setProperty(ENV_PROP, "false"); + ZkNodeProps message = new ZkNodeProps(Collections.emptyMap()); + assertFalse( + CollectionHandlingUtils.getBoolWithEnvFallback(message, MESSAGE_PARAM, ENV_PROP, true)); + } + + @Test + public void testMessageUnsetEnvSetTrueOverridesDefaultFalse() { + System.setProperty(ENV_PROP, "true"); + ZkNodeProps message = new ZkNodeProps(Collections.emptyMap()); + assertTrue( + CollectionHandlingUtils.getBoolWithEnvFallback(message, MESSAGE_PARAM, ENV_PROP, false)); + } + + @Test + public void testExplicitMessageParamWinsOverEnvFallback() { + System.setProperty(ENV_PROP, "true"); + ZkNodeProps messageFalse = new ZkNodeProps(Collections.singletonMap(MESSAGE_PARAM, "false")); + assertFalse( + CollectionHandlingUtils.getBoolWithEnvFallback( + messageFalse, MESSAGE_PARAM, ENV_PROP, false)); + + System.setProperty(ENV_PROP, "false"); + ZkNodeProps messageTrue = new ZkNodeProps(Collections.singletonMap(MESSAGE_PARAM, "true")); + assertTrue( + CollectionHandlingUtils.getBoolWithEnvFallback( + messageTrue, MESSAGE_PARAM, ENV_PROP, true)); + } +} diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/WaitForFinalStateEnvFallbackTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/WaitForFinalStateEnvFallbackTest.java new file mode 100644 index 000000000000..6c31ca4ee267 --- /dev/null +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/WaitForFinalStateEnvFallbackTest.java @@ -0,0 +1,132 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.cloud.api.collections; + +import java.lang.invoke.MethodHandles; +import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; +import org.apache.solr.client.solrj.request.CollectionAdminRequest.AsyncCollectionAdminRequest; +import org.apache.solr.cloud.SolrCloudTestCase; +import org.apache.solr.common.cloud.DocCollection; +import org.apache.solr.common.cloud.Replica; +import org.apache.solr.common.params.CollectionParams.CollectionAction; +import org.apache.solr.common.params.CommonAdminParams; +import org.apache.solr.common.params.CoreAdminParams; +import org.apache.solr.common.params.ModifiableSolrParams; +import org.apache.solr.common.params.SolrParams; +import org.apache.solr.util.TestInjection; +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.Test; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class WaitForFinalStateEnvFallbackTest extends SolrCloudTestCase { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + @BeforeClass + public static void setupCluster() throws Exception { + configureCluster(2).addConfig("conf", configset("cloud-minimal")).configure(); + } + + @After + public void releaseInjectionAndProperty() { + TestInjection.prepRecoveryOpPauseForever = null; + TestInjection.notifyPauseForeverDone(); + System.clearProperty(CommonAdminParams.WAIT_FOR_FINAL_STATE_DEFAULT_PROP); + } + + private static AsyncCollectionAdminRequest addReplicaWithTimeout( + String collection, String shard, int timeoutSeconds) { + return new AsyncCollectionAdminRequest(CollectionAction.ADDREPLICA) { + @Override + public SolrParams getParams() { + ModifiableSolrParams params = (ModifiableSolrParams) super.getParams(); + params.set(CoreAdminParams.COLLECTION, collection); + params.set(CoreAdminParams.SHARD, shard); + params.set(CommonAdminParams.TIMEOUT, timeoutSeconds); + return params; + } + }; + } + + @Test + public void testEnvFallbackFalseSkipsWaitEvenWhenRecoveryIsStuck() throws Exception { + String collection = "envfallbackfalse"; + SolrClient client = cluster.getSolrClient(); + CollectionAdminRequest.createCollection(collection, "conf", 1, 1).process(client); + cluster.waitForActiveCollection(collection, 1, 1); + + System.setProperty(CommonAdminParams.WAIT_FOR_FINAL_STATE_DEFAULT_PROP, "false"); + TestInjection.prepRecoveryOpPauseForever = "true:100"; + + long start = System.nanoTime(); + // message omits waitForFinalState entirely -- must resolve through the env fallback to false + addReplicaWithTimeout(collection, "shard1", 5).process(client); + long elapsedMs = (System.nanoTime() - start) / 1_000_000; + log.info("ADDREPLICA with env fallback=false returned after {}ms", elapsedMs); + + // proves we returned without waiting on the (permanently stuck) recovery: if the + // ActiveReplicaWatcher had been registered with the 5s timeout above, a genuine wait would + // either finish fast (if it wasn't really stuck) or throw after ~5s -- neither of which we + // want; we want no watcher registered at all, so this returns near-instantly. + assertTrue( + "expected ADDREPLICA to return promptly since waitForFinalState should resolve to " + + "false via the env fallback, but it took " + + elapsedMs + + "ms", + elapsedMs < 5_000); + + DocCollection coll = cluster.getSolrClient().getClusterState().getCollection(collection); + boolean anyNonActive = + coll.getReplicas().stream().anyMatch(r -> r.getState() != Replica.State.ACTIVE); + assertTrue( + "expected the new replica to still be stuck in recovery (not ACTIVE) since we " + + "returned before waiting for final state", + anyNonActive); + } + + @Test + public void testDefaultTrueActuallyWaitsAndTimesOutWhenRecoveryIsStuck() throws Exception { + String collection = "envfallbacktrue"; + SolrClient client = cluster.getSolrClient(); + CollectionAdminRequest.createCollection(collection, "conf", 1, 1).process(client); + cluster.waitForActiveCollection(collection, 1, 1); + + // no system property set: default resolves to true (the new SOLR-18367 default) + TestInjection.prepRecoveryOpPauseForever = "true:100"; + + long start = System.nanoTime(); + boolean threw = false; + try { + addReplicaWithTimeout(collection, "shard1", 5).process(client); + } catch (Exception e) { + threw = true; + log.info("ADDREPLICA with default (true) threw as expected: {}", e.toString()); + } + long elapsedMs = (System.nanoTime() - start) / 1_000_000; + log.info("ADDREPLICA with default (true) returned/threw after {}ms", elapsedMs); + + assertTrue( + "expected ADDREPLICA to actually wait for final state (and time out, since recovery " + + "is permanently stuck) when waitForFinalState resolves to true by default, but " + + "it returned successfully after only " + + elapsedMs + + "ms", + threw); + } +} diff --git a/solr/core/src/test/org/apache/solr/handler/admin/api/MigrateReplicasAPITest.java b/solr/core/src/test/org/apache/solr/handler/admin/api/MigrateReplicasAPITest.java index 574d671f5954..94959c447941 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/api/MigrateReplicasAPITest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/api/MigrateReplicasAPITest.java @@ -43,7 +43,7 @@ public void setUp() throws Exception { public void testCreatesValidOverseerMessage() throws Exception { MigrateReplicasRequestBody requestBody = new MigrateReplicasRequestBody( - Set.of("demoSourceNode"), Set.of("demoTargetNode"), false, "async"); + Set.of("demoSourceNode"), Set.of("demoTargetNode"), "async"); api.migrateReplicas(requestBody); @@ -51,17 +51,16 @@ public void testCreatesValidOverseerMessage() throws Exception { CollectionParams.CollectionAction.MIGRATE_REPLICAS, "async", message -> { - assertEquals(3, message.size()); + assertEquals(2, message.size()); assertEquals(Set.of("demoSourceNode"), message.get("sourceNodes")); assertEquals(Set.of("demoTargetNode"), message.get("targetNodes")); - assertEquals(false, message.get("waitForFinalState")); }); } @Test public void testNoTargetNodes() throws Exception { MigrateReplicasRequestBody requestBody = - new MigrateReplicasRequestBody(Set.of("demoSourceNode"), null, null, null); + new MigrateReplicasRequestBody(Set.of("demoSourceNode"), null, null); api.migrateReplicas(requestBody); @@ -76,10 +75,10 @@ public void testNoTargetNodes() throws Exception { @Test public void testNoSourceNodesThrowsError() { MigrateReplicasRequestBody requestBody1 = - new MigrateReplicasRequestBody(Set.of(), Set.of("demoTargetNode"), null, null); + new MigrateReplicasRequestBody(Set.of(), Set.of("demoTargetNode"), null); assertThrows(SolrException.class, () -> api.migrateReplicas(requestBody1)); MigrateReplicasRequestBody requestBody2 = - new MigrateReplicasRequestBody(null, Set.of("demoTargetNode"), null, null); + new MigrateReplicasRequestBody(null, Set.of("demoTargetNode"), null); assertThrows(SolrException.class, () -> api.migrateReplicas(requestBody2)); } } diff --git a/solr/core/src/test/org/apache/solr/handler/admin/api/ReplaceNodeAPITest.java b/solr/core/src/test/org/apache/solr/handler/admin/api/ReplaceNodeAPITest.java index 7386c4e29673..2fcfda81f2c2 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/api/ReplaceNodeAPITest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/api/ReplaceNodeAPITest.java @@ -39,7 +39,7 @@ public void setUp() throws Exception { @Test public void testCreatesValidOverseerMessage() throws Exception { - final var requestBody = new ReplaceNodeRequestBody("demoTargetNode", false, "async"); + final var requestBody = new ReplaceNodeRequestBody("demoTargetNode", "async"); api.replaceNode("demoSourceNode", requestBody); @@ -47,10 +47,9 @@ public void testCreatesValidOverseerMessage() throws Exception { CollectionParams.CollectionAction.REPLACENODE, requestBody.async, message -> { - assertEquals(3, message.size()); + assertEquals(2, message.size()); assertEquals("demoSourceNode", message.get("sourceNode")); assertEquals("demoTargetNode", message.get("targetNode")); - assertEquals(false, message.get("waitForFinalState")); }); } @@ -68,7 +67,7 @@ public void testRequestBodyCanBeOmittedAltogether() throws Exception { @Test public void testOptionalValuesNotAddedToRemoteMessageIfNotProvided() throws Exception { - final var requestBody = new ReplaceNodeRequestBody("demoTargetNode", null, null); + final var requestBody = new ReplaceNodeRequestBody("demoTargetNode", null); api.replaceNode("demoSourceNode", requestBody); @@ -78,10 +77,6 @@ public void testOptionalValuesNotAddedToRemoteMessageIfNotProvided() throws Exce assertEquals(2, message.size()); assertEquals("demoSourceNode", message.get("sourceNode")); assertEquals("demoTargetNode", message.get("targetNode")); - assertFalse( - "Expected message to not contain value for waitForFinalState: " - + message.get("waitForFinalState"), - message.containsKey("waitForFinalState")); }); } } diff --git a/solr/core/src/test/org/apache/solr/handler/admin/api/WaitForFinalStateRequestBodyOmittedFieldTest.java b/solr/core/src/test/org/apache/solr/handler/admin/api/WaitForFinalStateRequestBodyOmittedFieldTest.java new file mode 100644 index 000000000000..5e9b6b13552d --- /dev/null +++ b/solr/core/src/test/org/apache/solr/handler/admin/api/WaitForFinalStateRequestBodyOmittedFieldTest.java @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.solr.handler.admin.api; + +import com.fasterxml.jackson.databind.ObjectMapper; +import org.apache.solr.SolrTestCase; +import org.apache.solr.client.api.model.CreateCollectionRequestBody; +import org.apache.solr.client.api.model.CreateReplicaRequestBody; +import org.apache.solr.client.api.model.CreateShardRequestBody; +import org.junit.Test; + +/** + * AC8 (SOLR-18367): a v2 REST request body that omits {@code waitForFinalState} must deserialize + * to {@code null}, not {@code false}, so {@code insertIfNotNull} lets the server's own + * EnvUtils-driven default apply -- the same as SolrJ's field-omitted case. Only applies to the + * bounded commands (CREATE, CREATESHARD, ADDREPLICA); BALANCE_REPLICAS/MIGRATE_REPLICAS/ + * REPLACENODE dropped the field entirely -- their default stays `false` unconditionally. + */ +public class WaitForFinalStateRequestBodyOmittedFieldTest extends SolrTestCase { + + private final ObjectMapper mapper = new ObjectMapper(); + + @Test + public void testOmittedFieldDeserializesToNullNotFalse() throws Exception { + assertNull(mapper.readValue("{}", CreateCollectionRequestBody.class).waitForFinalState); + assertNull(mapper.readValue("{}", CreateShardRequestBody.class).waitForFinalState); + assertNull(mapper.readValue("{}", CreateReplicaRequestBody.class).waitForFinalState); + } +} diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/cluster-node-management.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/cluster-node-management.adoc index 46517de459d0..c5bd4ea73262 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/pages/cluster-node-management.adoc +++ b/solr/solr-ref-guide/modules/deployment-guide/pages/cluster-node-management.adoc @@ -535,6 +535,7 @@ If this parameter is not provided, all live data nodes will be used. + If `true`, the request will complete only when all affected replicas become active. If `false`, the API will return when the bare minimum replicas are active, such as the affected leader replicas. +This command can affect an arbitrary number of replicas cluster-wide, so unlike other Collections API commands it keeps the pre-SOLR-18367 default; an operator can opt in cluster-wide without changing any client by setting the system property `solr.cloud.waitForFinalState.enabled=true` on each Solr node. `async`:: + @@ -724,6 +725,7 @@ If there is more than one node to migrate the replicas to, then the configured P + If `true`, the request will complete only when all affected replicas become active. If `false`, the API will return when the bare minimum replicas are active, such as the affected leader replicas. +This command can affect an arbitrary number of replicas cluster-wide, so unlike other Collections API commands it keeps the pre-SOLR-18367 default; an operator can opt in cluster-wide without changing any client by setting the system property `solr.cloud.waitForFinalState.enabled=true` on each Solr node. `async`:: + @@ -783,7 +785,6 @@ V2 API:: curl -X POST "http://localhost:8983/api/cluster/nodes/localhost:7574_solr/replace" -H 'Content-Type: application/json' -d ' { "targetNodeName": "localhost:8983_solr", - "waitForFinalState": "false", "async": "async" } ' @@ -833,6 +834,7 @@ Keep in mind that this can lead to very high network and disk I/O if the replica + If `true`, the request will complete only when all affected replicas become active. If `false`, the API will return when the bare minimum replicas are active, such as the affected leader replicas. +This command can affect an arbitrary number of replicas cluster-wide, so unlike other Collections API commands it keeps the pre-SOLR-18367 default; an operator can opt in cluster-wide without changing any client by setting the system property `solr.cloud.waitForFinalState.enabled=true` on each Solr node. `async`:: + 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 46452619f7c7..3b45611efd0b 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 @@ -263,11 +263,12 @@ Altering these entries by specifying `property._name_=_value_` is an expert-leve + [%autowidth,frame=none] |=== -|Optional |Default: none +|Optional |Default: `true` |=== + If `true`, the request will complete only when all affected replicas become active. -The default is `false`, which means that the API will return the status of the single action, which may be before the new replica is online and active. +If `false`, the API will return the status of the single action, which may be before the new replica is online and active. +An operator can revert the cluster-wide default back to `false` without changing any client by setting the system property `solr.cloud.waitForFinalState.enabled=false` on each Solr node. `alias`:: + diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/replica-management.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/replica-management.adoc index 73c19a9db478..482549151ede 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/pages/replica-management.adoc +++ b/solr/solr-ref-guide/modules/deployment-guide/pages/replica-management.adoc @@ -239,11 +239,12 @@ Altering these entries by specifying `property._name_=_value_` is an expert-leve + [%autowidth,frame=none] |=== -|Optional |Default: `false` +|Optional |Default: `true` |=== + If `true`, the request will complete only when all affected replicas become active. If `false`, the API will return the status of the single action, which may be before the new replica is online and active. +An operator can revert the cluster-wide default back to `false` without changing any client by setting the system property `solr.cloud.waitForFinalState.enabled=false` on each Solr node. `async`:: + diff --git a/solr/solr-ref-guide/modules/deployment-guide/pages/shard-management.adoc b/solr/solr-ref-guide/modules/deployment-guide/pages/shard-management.adoc index e623424b6aa0..6f504e9e6ccf 100644 --- a/solr/solr-ref-guide/modules/deployment-guide/pages/shard-management.adoc +++ b/solr/solr-ref-guide/modules/deployment-guide/pages/shard-management.adoc @@ -209,11 +209,12 @@ See the section xref:configuration-guide:core-discovery.adoc[] for details on su + [%autowidth,frame=none] |=== -|Optional |Default: `false` +|Optional |Default: `true` |=== + If `true`, the request will complete only when all affected replicas become active. If `false`, the API will return the status of the single action, which may be before the new replica is online and active. +An operator can revert the cluster-wide default back to `false` without changing any client by setting the system property `solr.cloud.waitForFinalState.enabled=false` on each Solr node. `timing`:: + @@ -423,11 +424,12 @@ See the section xref:configuration-guide:core-discovery.adoc[] for details on su + [%autowidth,frame=none] |=== -|Optional |Default: `false` +|Optional |Default: `true` |=== + If `true`, the request will complete only when all affected replicas become active. If `false`, the API will return the status of the single action, which may be before the new replica is online and active. +An operator can revert the cluster-wide default back to `false` without changing any client by setting the system property `solr.cloud.waitForFinalState.enabled=false` on each Solr node. `async`:: + 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 d7501e549ccf..aa2a14198b3c 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 @@ -146,7 +146,6 @@ public abstract static class AsyncCollectionAdminRequest extends CollectionAdminRequest { protected String asyncId = null; - protected boolean waitForFinalState = true; public AsyncCollectionAdminRequest(CollectionAction action) { super(action); @@ -232,9 +231,6 @@ public SolrParams getParams() { if (asyncId != null) { params.set(CommonAdminParams.ASYNC, asyncId); } - if (waitForFinalState) { - params.set(CommonAdminParams.WAIT_FOR_FINAL_STATE, waitForFinalState); - } return params; } } diff --git a/solr/solrj/src/java/org/apache/solr/common/params/CommonAdminParams.java b/solr/solrj/src/java/org/apache/solr/common/params/CommonAdminParams.java index b77516daf08b..3965a51613a0 100644 --- a/solr/solrj/src/java/org/apache/solr/common/params/CommonAdminParams.java +++ b/solr/solrj/src/java/org/apache/solr/common/params/CommonAdminParams.java @@ -25,6 +25,16 @@ public interface CommonAdminParams { @Deprecated(since = "9.10") String WAIT_FOR_FINAL_STATE = "waitForFinalState"; + /** + * Node-level system property controlling the default value of {@link #WAIT_FOR_FINAL_STATE} when + * a request omits it. The per-command literal default it overrides differs by command: CREATE, + * CREATESHARD, SPLITSHARD, ADDREPLICA and MOVEREPLICA default to {@code true}; BALANCE_REPLICAS, + * MIGRATE_REPLICAS and REPLACENODE keep the pre-10.1 {@code false} default, since each can affect + * an arbitrary number of replicas cluster-wide. Setting this property (either value) overrides the + * per-command default uniformly for all 8, mirroring {@code CreateCollectionCmd.PRS_DEFAULT_PROP}. + */ + String WAIT_FOR_FINAL_STATE_DEFAULT_PROP = "solr.cloud.waitForFinalState.enabled"; + /** Allow in-place move of replicas that use shared filesystems. */ String IN_PLACE_MOVE = "inPlaceMove"; diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java index b216bb873383..942b0dbea48a 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/CollectionAdminRequestRequiredParamsTest.java @@ -17,7 +17,6 @@ package org.apache.solr.client.solrj; import static org.apache.solr.common.params.CollectionAdminParams.FLUSH; -import static org.apache.solr.common.params.CommonAdminParams.WAIT_FOR_FINAL_STATE; import static org.apache.solr.common.params.CoreAdminParams.ACTION; import static org.apache.solr.common.params.CoreAdminParams.COLLECTION; import static org.apache.solr.common.params.CoreAdminParams.NAME; @@ -46,30 +45,17 @@ public class CollectionAdminRequestRequiredParamsTest extends SolrTestCase { public void testBalanceShardUnique() { CollectionAdminRequest.BalanceShardUnique request = CollectionAdminRequest.balanceReplicaProperty("foo", "prop"); - assertContainsParams(request.getParams(), ACTION, COLLECTION, "property", WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, COLLECTION, "property"); request.setShardUnique(true); - assertContainsParams( - request.getParams(), ACTION, COLLECTION, "property", "shardUnique", WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, COLLECTION, "property", "shardUnique"); request.setOnlyActiveNodes(false); assertContainsParams( - request.getParams(), - ACTION, - COLLECTION, - "property", - "shardUnique", - "onlyactivenodes", - WAIT_FOR_FINAL_STATE); + request.getParams(), ACTION, COLLECTION, "property", "shardUnique", "onlyactivenodes"); request.setShardUnique(null); - assertContainsParams( - request.getParams(), - ACTION, - COLLECTION, - "property", - "onlyactivenodes", - WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, COLLECTION, "property", "onlyactivenodes"); } public void testClusterProp() { @@ -86,18 +72,17 @@ public void testCollectionProp() { ACTION, CoreAdminParams.NAME, CollectionAdminParams.PROPERTY_NAME, - CollectionAdminParams.PROPERTY_VALUE, - WAIT_FOR_FINAL_STATE); + CollectionAdminParams.PROPERTY_VALUE); } public void testAddRole() { CollectionAdminRequest.AddRole request = CollectionAdminRequest.addRole("node", "role"); - assertContainsParams(request.getParams(), ACTION, "node", "role", WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, "node", "role"); } public void testRemoveRole() { CollectionAdminRequest.RemoveRole request = CollectionAdminRequest.removeRole("node", "role"); - assertContainsParams(request.getParams(), ACTION, "node", "role", WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, "node", "role"); } public void testAddReplica() { @@ -105,31 +90,20 @@ public void testAddReplica() { CollectionAdminRequest.AddReplica request = CollectionAdminRequest.addReplicaToShard("collection", "shard"); assertContainsParams( - request.getParams(), - ACTION, - COLLECTION, - SHARD, - ZkStateReader.REPLICA_TYPE, - WAIT_FOR_FINAL_STATE); + request.getParams(), ACTION, COLLECTION, SHARD, ZkStateReader.REPLICA_TYPE); // with only shard parameter and "server side" implicit type, so no param request = CollectionAdminRequest.addReplicaToShard("collection", "shard", null); - assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD); // with route parameter request = CollectionAdminRequest.addReplicaByRouteKey("collection", "route"); - assertContainsParams( - request.getParams(), ACTION, COLLECTION, ShardParams._ROUTE_, WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, COLLECTION, ShardParams._ROUTE_); // with explicit type parameter request = CollectionAdminRequest.addReplicaToShard("collection", "shard", Replica.Type.NRT); assertContainsParams( - request.getParams(), - ACTION, - COLLECTION, - SHARD, - ZkStateReader.REPLICA_TYPE, - WAIT_FOR_FINAL_STATE); + request.getParams(), ACTION, COLLECTION, SHARD, ZkStateReader.REPLICA_TYPE); } public void testAddReplicaProp() { @@ -137,14 +111,7 @@ public void testAddReplicaProp() { CollectionAdminRequest.addReplicaProperty( "collection", "shard", "replica", "prop", "value"); assertContainsParams( - request.getParams(), - ACTION, - COLLECTION, - SHARD, - REPLICA, - "property", - "property.value", - WAIT_FOR_FINAL_STATE); + request.getParams(), ACTION, COLLECTION, SHARD, REPLICA, "property", "property.value"); } public void testClusterStatus() { @@ -165,33 +132,31 @@ public void testClusterStatus() { public void testCreateShard() { final CollectionAdminRequest.CreateShard request = CollectionAdminRequest.createShard("collection", "shard"); - assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD); } public void testDeleteReplica() { final CollectionAdminRequest.DeleteReplica request = CollectionAdminRequest.deleteReplica("collection", "shard", "replica"); - assertContainsParams( - request.getParams(), ACTION, COLLECTION, SHARD, REPLICA, WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, REPLICA); } public void testDeleteReplicaProp() { final CollectionAdminRequest.DeleteReplicaProp request = CollectionAdminRequest.deleteReplicaProperty("collection", "shard", "replica", "foo"); - assertContainsParams( - request.getParams(), ACTION, COLLECTION, SHARD, REPLICA, "property", WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, REPLICA, "property"); } public void testDeleteShard() { final CollectionAdminRequest.DeleteShard request = CollectionAdminRequest.deleteShard("collection", "shard"); - assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD); } public void testSplitShard() { final CollectionAdminRequest.SplitShard request = CollectionAdminRequest.splitShard("collection").setShardName("shard"); - assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD, WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, COLLECTION, SHARD); } public void testCreateCollection() { @@ -201,8 +166,7 @@ public void testCreateCollection() { ACTION, NAME, ZkStateReader.NUM_SHARDS_PROP, - ZkStateReader.NRT_REPLICAS, - WAIT_FOR_FINAL_STATE); + ZkStateReader.NRT_REPLICAS); // shortest form w/ "explicitly" choosing "implicit" router assertContainsParams( CollectionAdminRequest.createCollectionWithImplicitRouter("foo", null, "bar", 1) @@ -211,31 +175,30 @@ public void testCreateCollection() { NAME, "shards", "router.name", - ZkStateReader.NRT_REPLICAS, - WAIT_FOR_FINAL_STATE); + ZkStateReader.NRT_REPLICAS); } public void testReloadCollection() { final CollectionAdminRequest.Reload request = CollectionAdminRequest.reloadCollection("collection"); - assertContainsParams(request.getParams(), ACTION, NAME, WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, NAME); } public void testDeleteCollection() { final CollectionAdminRequest.Delete request = CollectionAdminRequest.deleteCollection("collection"); - assertContainsParams(request.getParams(), ACTION, NAME, WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, NAME); } public void testCreateAlias() { final CollectionAdminRequest.CreateAlias request = CollectionAdminRequest.createAlias("name", "collections"); - assertContainsParams(request.getParams(), ACTION, NAME, "collections", WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, NAME, "collections"); } public void testDeleteAlias() { final CollectionAdminRequest.DeleteAlias request = CollectionAdminRequest.deleteAlias("name"); - assertContainsParams(request.getParams(), ACTION, NAME, WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, NAME); } public void testListCollections() { @@ -246,19 +209,13 @@ public void testListCollections() { public void testMigrate() { final CollectionAdminRequest.Migrate request = CollectionAdminRequest.migrateData("collection", "target", "splitKey"); - assertContainsParams( - request.getParams(), - ACTION, - COLLECTION, - "target.collection", - "split.key", - WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION, COLLECTION, "target.collection", "split.key"); } public void testOverseerStatus() { final CollectionAdminRequest.OverseerStatus request = new CollectionAdminRequest.OverseerStatus(); - assertContainsParams(request.getParams(), ACTION, WAIT_FOR_FINAL_STATE); + assertContainsParams(request.getParams(), ACTION); } public void testRequestStatus() { From c685149964f53769d57a2c9e17651c5f36c969bd Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Tue, 25 Aug 2026 13:12:59 +0300 Subject: [PATCH 8/9] SOLR-18367: fix CI red -- spotless, forbiddenApis, validateLogCalls - spotlessApply on 5 files with javadoc/line-wrap violations. - Collections.emptyMap()/singletonMap() -> Map.of() in CollectionHandlingUtilsTest (forbiddenApisTest). - Drop an eager e.toString() in a parameterized log call (validateLogCalls flags it as suspicious). --- .../apache/solr/cloud/BalanceReplicasTest.java | 7 +++++-- .../collections/CollectionHandlingUtilsTest.java | 15 +++++++-------- .../WaitForFinalStateEnvFallbackTest.java | 2 +- .../handler/admin/api/MigrateReplicasAPITest.java | 3 +-- ...tForFinalStateRequestBodyOmittedFieldTest.java | 10 +++++----- .../solr/common/params/CommonAdminParams.java | 5 +++-- 6 files changed, 22 insertions(+), 20 deletions(-) diff --git a/solr/core/src/test/org/apache/solr/cloud/BalanceReplicasTest.java b/solr/core/src/test/org/apache/solr/cloud/BalanceReplicasTest.java index 9e6411c78337..94ce8f33e6ff 100644 --- a/solr/core/src/test/org/apache/solr/cloud/BalanceReplicasTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/BalanceReplicasTest.java @@ -139,13 +139,16 @@ public void testSomeNodes() throws Exception { postDataAndGetResponse( "/api/cluster/replicas/balance", - Utils.getReflectWriter(new BalanceReplicasRequestBody(new HashSet<>(l.subList(1, 4)), null))); + Utils.getReflectWriter( + new BalanceReplicasRequestBody(new HashSet<>(l.subList(1, 4)), null))); waitForState( "Timed out waiting for replicas to be balanced across the target nodes", coll, (state) -> - state.getReplicas().stream().map(Replica::getNodeName).collect(Collectors.toSet()) + state.getReplicas().stream() + .map(Replica::getNodeName) + .collect(Collectors.toSet()) .size() == 4); diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionHandlingUtilsTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionHandlingUtilsTest.java index 782458eb2dbe..ee99621b2356 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionHandlingUtilsTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/CollectionHandlingUtilsTest.java @@ -16,7 +16,7 @@ */ package org.apache.solr.cloud.api.collections; -import java.util.Collections; +import java.util.Map; import org.apache.solr.SolrTestCase; import org.apache.solr.common.cloud.ZkNodeProps; import org.junit.After; @@ -34,7 +34,7 @@ public void clearProperty() { @Test public void testMessageUnsetEnvUnsetFallsBackToDefault() { - ZkNodeProps message = new ZkNodeProps(Collections.emptyMap()); + ZkNodeProps message = new ZkNodeProps(Map.of()); assertTrue( CollectionHandlingUtils.getBoolWithEnvFallback(message, MESSAGE_PARAM, ENV_PROP, true)); assertFalse( @@ -44,7 +44,7 @@ public void testMessageUnsetEnvUnsetFallsBackToDefault() { @Test public void testMessageUnsetEnvSetFalseOverridesDefaultTrue() { System.setProperty(ENV_PROP, "false"); - ZkNodeProps message = new ZkNodeProps(Collections.emptyMap()); + ZkNodeProps message = new ZkNodeProps(Map.of()); assertFalse( CollectionHandlingUtils.getBoolWithEnvFallback(message, MESSAGE_PARAM, ENV_PROP, true)); } @@ -52,7 +52,7 @@ public void testMessageUnsetEnvSetFalseOverridesDefaultTrue() { @Test public void testMessageUnsetEnvSetTrueOverridesDefaultFalse() { System.setProperty(ENV_PROP, "true"); - ZkNodeProps message = new ZkNodeProps(Collections.emptyMap()); + ZkNodeProps message = new ZkNodeProps(Map.of()); assertTrue( CollectionHandlingUtils.getBoolWithEnvFallback(message, MESSAGE_PARAM, ENV_PROP, false)); } @@ -60,15 +60,14 @@ public void testMessageUnsetEnvSetTrueOverridesDefaultFalse() { @Test public void testExplicitMessageParamWinsOverEnvFallback() { System.setProperty(ENV_PROP, "true"); - ZkNodeProps messageFalse = new ZkNodeProps(Collections.singletonMap(MESSAGE_PARAM, "false")); + ZkNodeProps messageFalse = new ZkNodeProps(Map.of(MESSAGE_PARAM, "false")); assertFalse( CollectionHandlingUtils.getBoolWithEnvFallback( messageFalse, MESSAGE_PARAM, ENV_PROP, false)); System.setProperty(ENV_PROP, "false"); - ZkNodeProps messageTrue = new ZkNodeProps(Collections.singletonMap(MESSAGE_PARAM, "true")); + ZkNodeProps messageTrue = new ZkNodeProps(Map.of(MESSAGE_PARAM, "true")); assertTrue( - CollectionHandlingUtils.getBoolWithEnvFallback( - messageTrue, MESSAGE_PARAM, ENV_PROP, true)); + CollectionHandlingUtils.getBoolWithEnvFallback(messageTrue, MESSAGE_PARAM, ENV_PROP, true)); } } diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/WaitForFinalStateEnvFallbackTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/WaitForFinalStateEnvFallbackTest.java index 6c31ca4ee267..1076f9137f1f 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/WaitForFinalStateEnvFallbackTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/WaitForFinalStateEnvFallbackTest.java @@ -116,7 +116,7 @@ public void testDefaultTrueActuallyWaitsAndTimesOutWhenRecoveryIsStuck() throws addReplicaWithTimeout(collection, "shard1", 5).process(client); } catch (Exception e) { threw = true; - log.info("ADDREPLICA with default (true) threw as expected: {}", e.toString()); + log.info("ADDREPLICA with default (true) threw as expected: {}", e); } long elapsedMs = (System.nanoTime() - start) / 1_000_000; log.info("ADDREPLICA with default (true) returned/threw after {}ms", elapsedMs); diff --git a/solr/core/src/test/org/apache/solr/handler/admin/api/MigrateReplicasAPITest.java b/solr/core/src/test/org/apache/solr/handler/admin/api/MigrateReplicasAPITest.java index 94959c447941..5c901f1c5de2 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/api/MigrateReplicasAPITest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/api/MigrateReplicasAPITest.java @@ -42,8 +42,7 @@ public void setUp() throws Exception { @Test public void testCreatesValidOverseerMessage() throws Exception { MigrateReplicasRequestBody requestBody = - new MigrateReplicasRequestBody( - Set.of("demoSourceNode"), Set.of("demoTargetNode"), "async"); + new MigrateReplicasRequestBody(Set.of("demoSourceNode"), Set.of("demoTargetNode"), "async"); api.migrateReplicas(requestBody); diff --git a/solr/core/src/test/org/apache/solr/handler/admin/api/WaitForFinalStateRequestBodyOmittedFieldTest.java b/solr/core/src/test/org/apache/solr/handler/admin/api/WaitForFinalStateRequestBodyOmittedFieldTest.java index 5e9b6b13552d..4a9ae8edc46e 100644 --- a/solr/core/src/test/org/apache/solr/handler/admin/api/WaitForFinalStateRequestBodyOmittedFieldTest.java +++ b/solr/core/src/test/org/apache/solr/handler/admin/api/WaitForFinalStateRequestBodyOmittedFieldTest.java @@ -24,11 +24,11 @@ import org.junit.Test; /** - * AC8 (SOLR-18367): a v2 REST request body that omits {@code waitForFinalState} must deserialize - * to {@code null}, not {@code false}, so {@code insertIfNotNull} lets the server's own - * EnvUtils-driven default apply -- the same as SolrJ's field-omitted case. Only applies to the - * bounded commands (CREATE, CREATESHARD, ADDREPLICA); BALANCE_REPLICAS/MIGRATE_REPLICAS/ - * REPLACENODE dropped the field entirely -- their default stays `false` unconditionally. + * AC8 (SOLR-18367): a v2 REST request body that omits {@code waitForFinalState} must deserialize to + * {@code null}, not {@code false}, so {@code insertIfNotNull} lets the server's own EnvUtils-driven + * default apply -- the same as SolrJ's field-omitted case. Only applies to the bounded commands + * (CREATE, CREATESHARD, ADDREPLICA); BALANCE_REPLICAS/MIGRATE_REPLICAS/ REPLACENODE dropped the + * field entirely -- their default stays `false` unconditionally. */ public class WaitForFinalStateRequestBodyOmittedFieldTest extends SolrTestCase { diff --git a/solr/solrj/src/java/org/apache/solr/common/params/CommonAdminParams.java b/solr/solrj/src/java/org/apache/solr/common/params/CommonAdminParams.java index 3965a51613a0..e7d8907ee0c6 100644 --- a/solr/solrj/src/java/org/apache/solr/common/params/CommonAdminParams.java +++ b/solr/solrj/src/java/org/apache/solr/common/params/CommonAdminParams.java @@ -30,8 +30,9 @@ public interface CommonAdminParams { * a request omits it. The per-command literal default it overrides differs by command: CREATE, * CREATESHARD, SPLITSHARD, ADDREPLICA and MOVEREPLICA default to {@code true}; BALANCE_REPLICAS, * MIGRATE_REPLICAS and REPLACENODE keep the pre-10.1 {@code false} default, since each can affect - * an arbitrary number of replicas cluster-wide. Setting this property (either value) overrides the - * per-command default uniformly for all 8, mirroring {@code CreateCollectionCmd.PRS_DEFAULT_PROP}. + * an arbitrary number of replicas cluster-wide. Setting this property (either value) overrides + * the per-command default uniformly for all 8, mirroring {@code + * CreateCollectionCmd.PRS_DEFAULT_PROP}. */ String WAIT_FOR_FINAL_STATE_DEFAULT_PROP = "solr.cloud.waitForFinalState.enabled"; From ffa5017d83884ae53e86a729e60f83da767b3b57 Mon Sep 17 00:00:00 2001 From: Serhiy Bzhezytskyy Date: Tue, 25 Aug 2026 14:23:12 +0300 Subject: [PATCH 9/9] SOLR-18367: keep ADDREPLICA/MOVEREPLICA at false default They wait on a replica catching up on existing data, not a fixed-size empty one like CREATE/CREATESHARD/SPLITSHARD -- bounded replica count isn't bounded time. Matches unmodified main's current behavior; the WAIT_FOR_FINAL_STATE_DEFAULT_PROP escape hatch can still force true. Found via CloudHttp2SolrClientTest.testPerReplicaStateCollection timing out under the true default. Updated WaitForFinalStateEnvFallbackTest to match the new default/override split. --- .../cloud/api/collections/AddReplicaCmd.java | 2 +- .../cloud/api/collections/MoveReplicaCmd.java | 2 +- .../WaitForFinalStateEnvFallbackTest.java | 24 +++++++++---------- .../solr/common/params/CommonAdminParams.java | 10 ++++---- 4 files changed, 20 insertions(+), 18 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java index a308f92d4b04..40db1a35ee5c 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/AddReplicaCmd.java @@ -116,7 +116,7 @@ List addReplica( boolean waitForFinalState = CollectionHandlingUtils.getBoolWithEnvFallback( - message, WAIT_FOR_FINAL_STATE, WAIT_FOR_FINAL_STATE_DEFAULT_PROP, true); + message, WAIT_FOR_FINAL_STATE, WAIT_FOR_FINAL_STATE_DEFAULT_PROP, false); boolean skipCreateReplicaInClusterState = message.getBool(SKIP_CREATE_REPLICA_IN_CLUSTER_STATE, false); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java index 64bae6887436..2d34a457308a 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/MoveReplicaCmd.java @@ -75,7 +75,7 @@ private void moveReplica( String targetNode = message.getStr(CollectionParams.TARGET_NODE); boolean waitForFinalState = CollectionHandlingUtils.getBoolWithEnvFallback( - message, WAIT_FOR_FINAL_STATE, WAIT_FOR_FINAL_STATE_DEFAULT_PROP, true); + message, WAIT_FOR_FINAL_STATE, WAIT_FOR_FINAL_STATE_DEFAULT_PROP, false); boolean inPlaceMove = message.getBool(IN_PLACE_MOVE, true); int timeout = message.getInt(TIMEOUT, 10 * 60); // 10 minutes diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/WaitForFinalStateEnvFallbackTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/WaitForFinalStateEnvFallbackTest.java index 1076f9137f1f..c12f55bb6264 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/WaitForFinalStateEnvFallbackTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/WaitForFinalStateEnvFallbackTest.java @@ -65,28 +65,28 @@ public SolrParams getParams() { } @Test - public void testEnvFallbackFalseSkipsWaitEvenWhenRecoveryIsStuck() throws Exception { + public void testDefaultFalseSkipsWaitEvenWhenRecoveryIsStuck() throws Exception { String collection = "envfallbackfalse"; SolrClient client = cluster.getSolrClient(); CollectionAdminRequest.createCollection(collection, "conf", 1, 1).process(client); cluster.waitForActiveCollection(collection, 1, 1); - System.setProperty(CommonAdminParams.WAIT_FOR_FINAL_STATE_DEFAULT_PROP, "false"); + // no system property set: ADDREPLICA's literal default is false (waiting on a replica's + // recovery is unbounded in time, unlike CREATE/CREATESHARD/SPLITSHARD's brand-new replicas) TestInjection.prepRecoveryOpPauseForever = "true:100"; long start = System.nanoTime(); - // message omits waitForFinalState entirely -- must resolve through the env fallback to false addReplicaWithTimeout(collection, "shard1", 5).process(client); long elapsedMs = (System.nanoTime() - start) / 1_000_000; - log.info("ADDREPLICA with env fallback=false returned after {}ms", elapsedMs); + log.info("ADDREPLICA with default (false) returned after {}ms", elapsedMs); // proves we returned without waiting on the (permanently stuck) recovery: if the // ActiveReplicaWatcher had been registered with the 5s timeout above, a genuine wait would // either finish fast (if it wasn't really stuck) or throw after ~5s -- neither of which we // want; we want no watcher registered at all, so this returns near-instantly. assertTrue( - "expected ADDREPLICA to return promptly since waitForFinalState should resolve to " - + "false via the env fallback, but it took " + "expected ADDREPLICA to return promptly since waitForFinalState defaults to false, " + + "but it took " + elapsedMs + "ms", elapsedMs < 5_000); @@ -101,13 +101,13 @@ public void testEnvFallbackFalseSkipsWaitEvenWhenRecoveryIsStuck() throws Except } @Test - public void testDefaultTrueActuallyWaitsAndTimesOutWhenRecoveryIsStuck() throws Exception { + public void testEnvFallbackTrueActuallyWaitsAndTimesOutWhenRecoveryIsStuck() throws Exception { String collection = "envfallbacktrue"; SolrClient client = cluster.getSolrClient(); CollectionAdminRequest.createCollection(collection, "conf", 1, 1).process(client); cluster.waitForActiveCollection(collection, 1, 1); - // no system property set: default resolves to true (the new SOLR-18367 default) + System.setProperty(CommonAdminParams.WAIT_FOR_FINAL_STATE_DEFAULT_PROP, "true"); TestInjection.prepRecoveryOpPauseForever = "true:100"; long start = System.nanoTime(); @@ -116,15 +116,15 @@ public void testDefaultTrueActuallyWaitsAndTimesOutWhenRecoveryIsStuck() throws addReplicaWithTimeout(collection, "shard1", 5).process(client); } catch (Exception e) { threw = true; - log.info("ADDREPLICA with default (true) threw as expected: {}", e); + log.info("ADDREPLICA with env fallback=true threw as expected: {}", e); } long elapsedMs = (System.nanoTime() - start) / 1_000_000; - log.info("ADDREPLICA with default (true) returned/threw after {}ms", elapsedMs); + log.info("ADDREPLICA with env fallback=true returned/threw after {}ms", elapsedMs); assertTrue( "expected ADDREPLICA to actually wait for final state (and time out, since recovery " - + "is permanently stuck) when waitForFinalState resolves to true by default, but " - + "it returned successfully after only " + + "is permanently stuck) when waitForFinalState resolves to true via the env " + + "fallback, but it returned successfully after only " + elapsedMs + "ms", threw); diff --git a/solr/solrj/src/java/org/apache/solr/common/params/CommonAdminParams.java b/solr/solrj/src/java/org/apache/solr/common/params/CommonAdminParams.java index e7d8907ee0c6..4444df6e8711 100644 --- a/solr/solrj/src/java/org/apache/solr/common/params/CommonAdminParams.java +++ b/solr/solrj/src/java/org/apache/solr/common/params/CommonAdminParams.java @@ -28,10 +28,12 @@ public interface CommonAdminParams { /** * Node-level system property controlling the default value of {@link #WAIT_FOR_FINAL_STATE} when * a request omits it. The per-command literal default it overrides differs by command: CREATE, - * CREATESHARD, SPLITSHARD, ADDREPLICA and MOVEREPLICA default to {@code true}; BALANCE_REPLICAS, - * MIGRATE_REPLICAS and REPLACENODE keep the pre-10.1 {@code false} default, since each can affect - * an arbitrary number of replicas cluster-wide. Setting this property (either value) overrides - * the per-command default uniformly for all 8, mirroring {@code + * CREATESHARD and SPLITSHARD default to {@code true} -- they only ever wait on brand-new, empty + * replicas, so activation is fast and bounded. ADDREPLICA, MOVEREPLICA, BALANCE_REPLICAS, + * MIGRATE_REPLICAS and REPLACENODE keep the pre-10.1 {@code false} default: each can wait on a + * replica catching up on an arbitrary amount of existing data (a full recovery/replication), so + * the wait is unbounded in time even when the replica count is small or fixed. Setting this + * property (either value) overrides the per-command default uniformly for all 8, mirroring {@code * CreateCollectionCmd.PRS_DEFAULT_PROP}. */ String WAIT_FOR_FINAL_STATE_DEFAULT_PROP = "solr.cloud.waitForFinalState.enabled";