pvcnt commented on code in PR #2039:
URL: https://github.com/apache/solr/pull/2039#discussion_r1409408694


##########
solr/core/src/java/org/apache/solr/cloud/api/collections/CreateShardCmd.java:
##########
@@ -97,33 +101,30 @@ public void call(ClusterState clusterState, ZkNodeProps 
message, NamedList<Objec
         CollectionHandlingUtils.waitForNewShard(collectionName, sliceName, 
ccc.getZkStateReader());
 
     String async = message.getStr(ASYNC);
-    ZkNodeProps addReplicasProps =
-        new ZkNodeProps(
+    Map<String, Object> addReplicasProps =
+        Utils.makeMap(
             COLLECTION_PROP,
-            collectionName,
+            (Object) collectionName,
             SHARD_ID_PROP,
             sliceName,
-            ZkStateReader.NRT_REPLICAS,
-            String.valueOf(numReplicas.get(Replica.Type.NRT)),
-            ZkStateReader.TLOG_REPLICAS,
-            String.valueOf(numReplicas.get(Replica.Type.TLOG)),
-            ZkStateReader.PULL_REPLICAS,
-            String.valueOf(numReplicas.get(Replica.Type.PULL)),
             CollectionHandlingUtils.CREATE_NODE_SET,
             message.getStr(CollectionHandlingUtils.CREATE_NODE_SET),
             CommonAdminParams.WAIT_FOR_FINAL_STATE,
             Boolean.toString(waitForFinalState));
+    for (Replica.Type replicaType : numReplicas.keySet()) {
+      addReplicasProps.put(replicaType.numReplicasProperty, 
numReplicas.get(replicaType));
+    }
 
-    Map<String, Object> propertyParams = new HashMap<>();
-    CollectionHandlingUtils.addPropertyParams(message, propertyParams);
-    addReplicasProps = addReplicasProps.plus(propertyParams);
-    if (async != null) addReplicasProps.getProperties().put(ASYNC, async);
+    CollectionHandlingUtils.addPropertyParams(message, addReplicasProps);
+    if (async != null) {
+      addReplicasProps.put(ASYNC, async);
+    }
     final NamedList<Object> addResult = new NamedList<>();
     try {
       new AddReplicaCmd(ccc)
           .addReplica(
               clusterState,
-              addReplicasProps,
+              new ZkNodeProps(addReplicasProps),

Review Comment:
   Not sure to understand your suggestion. Would you like that addReplicaProps 
is a ZkNodeProps that is modified with new params, instead of mutating a map a 
building a ZkNodeProps at the end?
   
   I did not do so because ZkNodeProps is marked as immutable in the docs and 
implements mutating methods which copy the map (e.g., `ZkNodeProps#plus`), even 
though in practice it is mutable by altering the map returned from 
`ZkNodeProps#getProperties`).
   
   The previous code showed this mixed behaviour:
   ```java
   // Copy
   addReplicasProps = addReplicasProps.plus(propertyParams);
   // Mutate in-place
   if (async != null) addReplicasProps.getProperties().put(ASYNC, async);
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org

Reply via email to