SOLR-18367: waitForFinalState now defaults to true, remove the deprecated opt-out

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

solr/api/src/java/org/apache/solr/client/api/model/BalanceReplicasRequestBody.java

@@ -24,9 +24,8 @@ public class BalanceReplicasRequestBody {
 
   public BalanceReplicasRequestBody() {}
 
-  public BalanceReplicasRequestBody(Set<String> nodes, Boolean waitForFinalState, String async) {
+  public BalanceReplicasRequestBody(Set<String> nodes, String async) {
     this.nodes = nodes;
-    this.waitForFinalState = waitForFinalState;
     this.async = async;
   }
 
@@ -36,15 +35,6 @@ public BalanceReplicasRequestBody(Set<String> nodes, Boolean waitForFinalState,
   @JsonProperty(value = "nodes")
   public Set<String> 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;

solr/api/src/java/org/apache/solr/client/api/model/MigrateReplicasRequestBody.java

@@ -25,10 +25,9 @@ public class MigrateReplicasRequestBody {
   public MigrateReplicasRequestBody() {}
 
   public MigrateReplicasRequestBody(
-      Set<String> sourceNodes, Set<String> targetNodes, Boolean waitForFinalState, String async) {
+      Set<String> sourceNodes, Set<String> targetNodes, String async) {
     this.sourceNodes = sourceNodes;
     this.targetNodes = targetNodes;
-    this.waitForFinalState = waitForFinalState;
     this.async = async;
   }
 
@@ -42,15 +41,6 @@ public MigrateReplicasRequestBody(
   @JsonProperty
   public Set<String> 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;

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;

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<ZkNodeProps> addReplica(
           "Collection: " + collectionName + " shard: " + shard + " does not exist");
     }
 
-    boolean waitForFinalState = message.getBool(WAIT_FOR_FINAL_STATE, false);
+    boolean waitForFinalState =
+        CollectionHandlingUtils.getBoolWithEnvFallback(
+            message, WAIT_FOR_FINAL_STATE, WAIT_FOR_FINAL_STATE_DEFAULT_PROP, false);
     boolean skipCreateReplicaInClusterState =
         message.getBool(SKIP_CREATE_REPLICA_IN_CLUSTER_STATE, false);
 

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, false);
+    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);
 

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<Replica.Type> 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<envProp>=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,

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, false);
+    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);

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, false);
+    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)

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)) {

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<String> sourceNodes = getNodesFromParam(message, CollectionParams.SOURCE_NODES);
     Set<String> targetNodes = getNodesFromParam(message, CollectionParams.TARGET_NODES);
-    boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, false);
+    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");

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, false);
+    boolean waitForFinalState =
+        CollectionHandlingUtils.getBoolWithEnvFallback(
+            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
 
@@ -373,7 +376,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<Object> addResult = new NamedList<>();
     SolrCloseableLatch countDownLatch = new SolrCloseableLatch(1, ccc.getCloseableToLatchOn());

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, false);
+    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");

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<Object> nl = new NamedList<>();
       final ZkNodeProps addedReplica =
           new AddReplicaCmd(ccc)

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;

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<Object> results)
       throws Exception {
-    boolean waitForFinalState = message.getBool(CommonAdminParams.WAIT_FOR_FINAL_STATE, false);
+    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());

solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java

@@ -1164,7 +1164,6 @@ public Map<String, Object> 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 =

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<String, Object> remoteMessage = new HashMap<>();
     if (requestBody != null) {
       insertIfNotNull(remoteMessage, NODES, requestBody.nodes);
-      insertIfNotNull(remoteMessage, WAIT_FOR_FINAL_STATE, requestBody.waitForFinalState);
     }
 
     return new ZkNodeProps(remoteMessage);

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,