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