SOLR-11068: MOVEREPLICA and REPLACENODE API parameter names are now 'sourceNode' and 'targetNode'. The old names viz. 'fromNode' for MOVEREPLICA and 'source', 'target' for REPLACENODE have been deprecated

solr/CHANGES.txt

@@ -211,6 +211,10 @@ Upgrading from Solr 6.x
 
 * StandardRequestHandler is deprecated. Simply use SearchHandler instead.
 
+* The parameter names 'fromNode' for MOVEREPLICA and 'source', 'target' for REPLACENODE have been deprecated and
+  replaced with 'sourceNode' and 'targetNode' instead. The old names will continue to work for back-compatibility
+  but they will be removed in 8.0. See SOLR-11068 for more details.
+
 New Features
 ----------------------
 * SOLR-9857, SOLR-9858: Collect aggregated metrics from nodes and shard leaders in overseer. (ab)
@@ -503,6 +507,9 @@ Other Changes
 
 * SOLR-10796: TestPointFields: increase randomized testing of non-trivial values.  (Steve Rowe)
 
+* SOLR-11068: MOVEREPLICA and REPLACENODE API parameter names are now 'sourceNode' and 'targetNode'. The old names
+  viz. 'fromNode' for MOVEREPLICA and 'source', 'target' for REPLACENODE have been deprecated. (shalin)
+
 ==================  6.7.0 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.

solr/core/src/java/org/apache/solr/cloud/MoveReplicaCmd.java

@@ -31,6 +31,7 @@ import org.apache.solr.common.cloud.DocCollection;
 import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.cloud.Slice;
 import org.apache.solr.common.cloud.ZkNodeProps;
+import org.apache.solr.common.params.CollectionParams;
 import org.apache.solr.common.params.CoreAdminParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.Utils;
@@ -81,20 +82,22 @@ public class MoveReplicaCmd implements Cmd{
             "Collection: " + collection + " replica: " + replicaName + " does not exist");
       }
     } else {
-      ocmh.checkRequired(message, SHARD_ID_PROP, "fromNode");
+      String sourceNode = message.getStr(CollectionParams.SOURCE_NODE, message.getStr(CollectionParams.FROM_NODE));
-      String fromNode = message.getStr("fromNode");
+      if (sourceNode == null) {
+        throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "sourceNode is a required param" );
+      }
       String shardId = message.getStr(SHARD_ID_PROP);
       Slice slice = clusterState.getCollection(collection).getSlice(shardId);
       List<Replica> sliceReplicas = new ArrayList<>(slice.getReplicas());
       Collections.shuffle(sliceReplicas, RANDOM);
       for (Replica r : slice.getReplicas()) {
-        if (r.getNodeName().equals(fromNode)) {
+        if (r.getNodeName().equals(sourceNode)) {
           replica = r;
         }
       }
       if (replica == null) {
         throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
-            "Collection: " + collection + " node: " + fromNode + " do not have any replica belong to shard: " + shardId);
+            "Collection: " + collection + " node: " + sourceNode + " do not have any replica belong to shard: " + shardId);
       }
     }
 

solr/core/src/java/org/apache/solr/cloud/ReplaceNodeCmd.java

@@ -37,6 +37,7 @@ import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.cloud.Slice;
 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.CoreAdminParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.zookeeper.KeeperException;
@@ -59,9 +60,11 @@ public class ReplaceNodeCmd implements OverseerCollectionMessageHandler.Cmd {
   @Override
   public void call(ClusterState state, ZkNodeProps message, NamedList results) throws Exception {
     ZkStateReader zkStateReader = ocmh.zkStateReader;
-    ocmh.checkRequired(message, "source", "target");
+    String source = message.getStr(CollectionParams.SOURCE_NODE, message.getStr("source"));
-    String source = message.getStr("source");
+    String target = message.getStr(CollectionParams.TARGET_NODE, message.getStr("target"));
-    String target = message.getStr("target");
+    if (source == null || target == null) {
+      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "sourceNode and targetNode are required params" );
+    }
     String async = message.getStr("async");
     int timeout = message.getInt("timeout", 10 * 60); // 10 minutes
     boolean parallel = message.getBool("parallel", false);

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

@@ -882,14 +882,26 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
       rsp.add(SolrSnapshotManager.SNAPSHOTS_INFO, snapshots);
       return null;
     }),
-    REPLACENODE_OP(REPLACENODE, (req, rsp, h) -> req.getParams().required().getAll(req.getParams().getAll(null, "parallel"), "source", "target")),
+    REPLACENODE_OP(REPLACENODE, (req, rsp, h) -> {
+      SolrParams params = req.getParams();
+      String sourceNode = params.get(CollectionParams.SOURCE_NODE, params.get("source"));
+      if (sourceNode == null) {
+        throw new SolrException(ErrorCode.BAD_REQUEST, CollectionParams.SOURCE_NODE + " is a require parameter");
+      }
+      String targetNode = params.get(CollectionParams.TARGET_NODE, params.get("target"));
+      if (targetNode == null) {
+        throw new SolrException(ErrorCode.BAD_REQUEST, CollectionParams.TARGET_NODE + " is a require parameter");
+      }
+      return params.getAll(null, "source", "target", CollectionParams.SOURCE_NODE, CollectionParams.TARGET_NODE);
+    }),
     MOVEREPLICA_OP(MOVEREPLICA, (req, rsp, h) -> {
       Map<String, Object> map = req.getParams().required().getAll(null,
           COLLECTION_PROP);
 
       return req.getParams().getAll(map,
-          "fromNode",
+          CollectionParams.FROM_NODE,
-          "targetNode",
+          CollectionParams.SOURCE_NODE,
+          CollectionParams.TARGET_NODE,
           "replica",
           "shard");
     }),

solr/core/src/test/org/apache/solr/cloud/MoveReplicaTest.java

@@ -35,6 +35,9 @@ import org.apache.solr.client.solrj.response.RequestStatusState;
 import org.apache.solr.common.cloud.DocCollection;
 import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.cloud.Slice;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.SolrParams;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.slf4j.Logger;
@@ -86,7 +89,7 @@ public class MoveReplicaTest extends SolrCloudTestCase {
       }
     }
 
-    CollectionAdminRequest.MoveReplica moveReplica = new CollectionAdminRequest.MoveReplica(coll, replica.getName(), targetNode);
+    CollectionAdminRequest.MoveReplica moveReplica = createMoveReplicaRequest(coll, replica, targetNode);
     moveReplica.processAsync("000", cloudClient);
     CollectionAdminRequest.RequestStatus requestStatus = CollectionAdminRequest.requestStatus("000");
     // wait for async request success
@@ -141,7 +144,7 @@ public class MoveReplicaTest extends SolrCloudTestCase {
     }
     assertTrue("replica never fully recovered", recovered);
 
-    moveReplica = new CollectionAdminRequest.MoveReplica(coll, shardId, targetNode, replica.getNodeName());
+    moveReplica = createMoveReplicaRequest(coll, replica, targetNode, shardId);
     moveReplica.process(cloudClient);
     checkNumOfCores(cloudClient, replica.getNodeName(), 1);
     // wait for recovery
@@ -181,6 +184,49 @@ public class MoveReplicaTest extends SolrCloudTestCase {
     assertTrue("replica never fully recovered", recovered);
   }
 
+  private CollectionAdminRequest.MoveReplica createMoveReplicaRequest(String coll, Replica replica, String targetNode, String shardId) {
+    if (random().nextBoolean()) {
+      return new CollectionAdminRequest.MoveReplica(coll, shardId, targetNode, replica.getNodeName());
+    } else  {
+      // for backcompat testing of SOLR-11068
+      // todo remove in solr 8.0
+      return new BackCompatMoveReplicaRequest(coll, shardId, targetNode, replica.getNodeName());
+    }
+  }
+
+  private CollectionAdminRequest.MoveReplica createMoveReplicaRequest(String coll, Replica replica, String targetNode) {
+    if (random().nextBoolean()) {
+      return new CollectionAdminRequest.MoveReplica(coll, replica.getName(), targetNode);
+    } else  {
+      // for backcompat testing of SOLR-11068
+      // todo remove in solr 8.0
+      return new BackCompatMoveReplicaRequest(coll, replica.getName(), targetNode);
+    }
+  }
+
+  /**
+   * Added for backcompat testing
+   * todo remove in solr 8.0
+   */
+  static class BackCompatMoveReplicaRequest extends CollectionAdminRequest.MoveReplica {
+    public BackCompatMoveReplicaRequest(String collection, String replica, String targetNode) {
+      super(collection, replica, targetNode);
+    }
+
+    public BackCompatMoveReplicaRequest(String collection, String shard, String sourceNode, String targetNode) {
+      super(collection, shard, sourceNode, targetNode);
+    }
+
+    @Override
+    public SolrParams getParams() {
+      ModifiableSolrParams params = (ModifiableSolrParams) super.getParams();
+      if (randomlyMoveReplica) {
+        params.set(CollectionParams.FROM_NODE, sourceNode);
+      }
+      return params;
+    }
+  }
+
   private Replica getRandomReplica(String coll, CloudSolrClient cloudClient) {
     List<Replica> replicas = cloudClient.getZkStateReader().getClusterState().getCollection(coll).getReplicas();
     Collections.shuffle(replicas, random());

solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java

@@ -33,6 +33,9 @@ import org.apache.solr.client.solrj.response.RequestStatusState;
 import org.apache.solr.common.cloud.DocCollection;
 import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.cloud.Slice;
+import org.apache.solr.common.params.CollectionParams;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.StrUtils;
 import org.junit.BeforeClass;
 import org.junit.Test;
@@ -80,7 +83,7 @@ public class ReplaceNodeTest extends SolrCloudTestCase {
     create.setCreateNodeSet(StrUtils.join(l, ',')).setMaxShardsPerNode(3);
     cloudClient.request(create);
     log.info("excluded_node : {}  ", emptyNode);
-    new CollectionAdminRequest.ReplaceNode(node2bdecommissioned, emptyNode).processAsync("000", cloudClient);
+    createReplaceNodeRequest(node2bdecommissioned, emptyNode, null).processAsync("000", cloudClient);
     CollectionAdminRequest.RequestStatus requestStatus = CollectionAdminRequest.requestStatus("000");
     boolean success = false;
     for (int i = 0; i < 300; i++) {
@@ -99,7 +102,7 @@ public class ReplaceNodeTest extends SolrCloudTestCase {
     }
 
     //let's do it back
-    new CollectionAdminRequest.ReplaceNode(emptyNode, node2bdecommissioned).setParallel(Boolean.TRUE).processAsync("001", cloudClient);
+    createReplaceNodeRequest(emptyNode, node2bdecommissioned, Boolean.TRUE).processAsync("001", cloudClient);
     requestStatus = CollectionAdminRequest.requestStatus("001");
 
     for (int i = 0; i < 200; i++) {
@@ -125,4 +128,23 @@ public class ReplaceNodeTest extends SolrCloudTestCase {
       assertEquals(create.getNumPullReplicas().intValue(), s.getReplicas(EnumSet.of(Replica.Type.PULL)).size());
     }
   }
+
+  private CollectionAdminRequest.AsyncCollectionAdminRequest createReplaceNodeRequest(String sourceNode, String targetNode, Boolean parallel) {
+    if (random().nextBoolean()) {
+      return new CollectionAdminRequest.ReplaceNode(sourceNode, targetNode).setParallel(parallel);
+    } else  {
+      // test back compat with old param names
+      // todo remove in solr 8.0
+      return new CollectionAdminRequest.AsyncCollectionAdminRequest(CollectionParams.CollectionAction.REPLACENODE)  {
+        @Override
+        public SolrParams getParams() {
+          ModifiableSolrParams params = (ModifiableSolrParams) super.getParams();
+          params.set("source", sourceNode);
+          params.set("target", targetNode);
+          if (parallel != null) params.set("parallel", parallel.toString());
+          return params;
+        }
+      };
+    }
+  }
 }

solr/solr-ref-guide/src/collections-api.adoc

@@ -1832,14 +1832,14 @@ This command recreates replicas in one node (the source) to another node (the ta
 
 For source replicas that are also shard leaders the operation will wait for the number of seconds set with the `timeout` parameter to make sure there's an active replica that can become a leader (either an existing replica becoming a leader or the new replica completing recovery and becoming a leader).
 
-`/admin/collections?action=REPLACENODE&source=_source-node_&target=_target-node_`
+`/admin/collections?action=REPLACENODE&sourceNode=_source-node_&targetNode=_target-node_`
 
 === REPLACENODE Parameters
 
-`source`::
+`sourceNode`::
 The source node from which the replicas need to be copied from. This parameter is required.
 
-`target`::
+`targetNode`::
 The target node where replicas will be copied. This parameter is required.
 
 `parallel`::
@@ -1860,7 +1860,7 @@ This operation does not hold necessary locks on the replicas that belong to on t
 
 This command moves a replica from one node to a new node. In case of shared filesystems the `dataDir` will be reused.
 
-`/admin/collections?action=MOVEREPLICA&collection=collection&shard=shard&replica=replica&node=nodeName&toNode=nodeName`
+`/admin/collections?action=MOVEREPLICA&collection=collection&shard=shard&replica=replica&sourceNode=nodeName&targetNode=nodeName`
 
 === MOVEREPLICA Parameters
 
@@ -1873,10 +1873,10 @@ The name of the shard that the replica belongs to. This parameter is required.
 `replica`::
 The name of the replica. This parameter is required.
 
-`node`::
+`sourceNode`::
 The name of the node that contains the replica. This parameter is required.
 
-`toNode`::
+`targetNode`::
 The name of the destination node. This parameter is required.
 
 `async`::

solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/MoveReplicaSuggester.java

@@ -39,7 +39,7 @@ public class MoveReplicaSuggester extends Suggester {
     //iterate through elements and identify the least loaded
     List<Clause.Violation> leastSeriousViolation = null;
     Integer targetNodeIndex = null;
-    Integer fromNodeIndex = null;
+    Integer sourceNodeIndex = null;
     ReplicaInfo fromReplicaInfo = null;
     for (Pair<ReplicaInfo, Row> fromReplica : getValidReplicas(true, true, -1)) {
       Row fromRow = fromReplica.second();
@@ -64,13 +64,13 @@ public class MoveReplicaSuggester extends Suggester {
         if (!containsNewErrors(errs) && isLessSerious(errs, leastSeriousViolation)) {
           leastSeriousViolation = errs;
           targetNodeIndex = j;
-          fromNodeIndex = i;
+          sourceNodeIndex = i;
           fromReplicaInfo = replicaInfo;
         }
       }
     }
-    if (targetNodeIndex != null && fromNodeIndex != null) {
+    if (targetNodeIndex != null && sourceNodeIndex != null) {
-      getMatrix().set(fromNodeIndex, getMatrix().get(fromNodeIndex).removeReplica(fromReplicaInfo.collection, fromReplicaInfo.shard).first());
+      getMatrix().set(sourceNodeIndex, getMatrix().get(sourceNodeIndex).removeReplica(fromReplicaInfo.collection, fromReplicaInfo.shard).first());
       getMatrix().set(targetNodeIndex, getMatrix().get(targetNodeIndex).addReplica(fromReplicaInfo.collection, fromReplicaInfo.shard));
       return new CollectionAdminRequest.MoveReplica(
           fromReplicaInfo.collection,

solr/solrj/src/java/org/apache/solr/client/solrj/request/CollectionAdminRequest.java

@@ -37,6 +37,7 @@ import org.apache.solr.common.cloud.ImplicitDocRouter;
 import org.apache.solr.common.cloud.Replica;
 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.CollectionParams.CollectionAction;
 import org.apache.solr.common.params.CommonAdminParams;
 import org.apache.solr.common.params.CoreAdminParams;
@@ -547,7 +548,7 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
   }
 
   public static class ReplaceNode extends AsyncCollectionAdminRequest {
-    String source, target;
+    String sourceNode, targetNode;
     Boolean parallel;
 
     /**
@@ -556,8 +557,8 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
      */
     public ReplaceNode(String source, String target) {
       super(CollectionAction.REPLACENODE);
-      this.source = checkNotNull("source",source);
+      this.sourceNode = checkNotNull(CollectionParams.SOURCE_NODE, source);
-      this.target = checkNotNull("target",target);
+      this.targetNode = checkNotNull(CollectionParams.TARGET_NODE, target);
     }
 
     public ReplaceNode setParallel(Boolean flag) {
@@ -568,8 +569,8 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
     @Override
     public SolrParams getParams() {
       ModifiableSolrParams params = (ModifiableSolrParams) super.getParams();
-      params.set("source", source);
+      params.set(CollectionParams.SOURCE_NODE, sourceNode);
-      params.set("target", target);
+      params.set(CollectionParams.TARGET_NODE, targetNode);
       if (parallel != null) params.set("parallel", parallel.toString());
       return params;
     }
@@ -577,9 +578,9 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
   }
 
   public static class MoveReplica extends AsyncCollectionAdminRequest {
-    String collection, replica, targetNode;
+    protected String collection, replica, targetNode;
-    String shard, fromNode;
+    protected String shard, sourceNode;
-    boolean randomlyMoveReplica;
+    protected boolean randomlyMoveReplica;
 
     public MoveReplica(String collection, String replica, String targetNode) {
       super(CollectionAction.MOVEREPLICA);
@@ -589,12 +590,12 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
       this.randomlyMoveReplica = false;
     }
 
-    public MoveReplica(String collection, String shard, String fromNode, String targetNode) {
+    public MoveReplica(String collection, String shard, String sourceNode, String targetNode) {
       super(CollectionAction.MOVEREPLICA);
       this.collection = checkNotNull("collection",collection);
       this.shard = checkNotNull("shard",shard);
-      this.fromNode = checkNotNull("fromNode",fromNode);
+      this.sourceNode = checkNotNull(CollectionParams.SOURCE_NODE, sourceNode);
-      this.targetNode = checkNotNull("targetNode",targetNode);
+      this.targetNode = checkNotNull(CollectionParams.TARGET_NODE, targetNode);
       this.randomlyMoveReplica = true;
     }
 
@@ -602,10 +603,10 @@ public abstract class CollectionAdminRequest<T extends CollectionAdminResponse>
     public SolrParams getParams() {
       ModifiableSolrParams params = (ModifiableSolrParams) super.getParams();
       params.set("collection", collection);
-      params.set("targetNode", targetNode);
+      params.set(CollectionParams.TARGET_NODE, targetNode);
       if (randomlyMoveReplica) {
         params.set("shard", shard);
-        params.set("fromNode", fromNode);
+        params.set(CollectionParams.SOURCE_NODE, sourceNode);
       } else {
         params.set("replica", replica);
       }

solr/solrj/src/java/org/apache/solr/common/params/CollectionParams.java

@@ -31,6 +31,15 @@ public interface CollectionParams {
   String ACTION = "action";
   String NAME = "name";
 
+  /**
+   * @deprecated use {@link #SOURCE_NODE} instead
+   */
+  @Deprecated
+  String FROM_NODE = "fromNode";
+
+  String SOURCE_NODE = "sourceNode";
+  String TARGET_NODE = "targetNode";
+
 
   enum LockLevel {
     CLUSTER(0),