dsmiley commented on code in PR #1973:
URL: https://github.com/apache/solr/pull/1973#discussion_r1358902385
##########
solr/core/src/test/org/apache/solr/cloud/AddReplicaTest.java:
##########
@@ -187,4 +188,37 @@ public void test() throws Exception {
assertSame(coll + "\n" + replica, replica.getState(),
Replica.State.ACTIVE);
}
}
+
+ @Test
+ public void testAddReplicaWithUserDefinedProperties() throws Exception {
+ // When creating a collection with user-defined properties
+ CloudSolrClient cloudClient = cluster.getSolrClient();
+ String collectionName = "testAddReplicaWithUserDefinedProperties";
+ CollectionAdminRequest.createCollection(collectionName, "conf1", 1, 1)
+ .withProperty("customProp1", "val1")
+ .withProperty("customProp2", "val2")
+ .process(cloudClient);
+ cluster.waitForActiveCollection(collectionName, 1, 1);
+
+ // When adding a replica to the collection with user-defined properties
+ CollectionAdminRequest.AddReplica addReplica =
+ 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
+ // and inherited from the collection (the former taking precedence over
the latter).
+ Replica replica =
+
cloudClient.getClusterState().getCollection(collectionName).getReplicas().get(1);
+ CoreDescriptor coreDescriptor =
+ cluster
+ .getReplicaJetty(replica)
+ .getCoreContainer()
+ .getCoreDescriptor(replica.getCoreName());
+ assertEquals("val1", coreDescriptor.getCoreProperty("customProp1", ""));
+ assertEquals("val2.1", coreDescriptor.getCoreProperty("customProp2", ""));
Review Comment:
nice
##########
solr/core/src/java/org/apache/solr/cloud/overseer/ClusterStateMutator.java:
##########
@@ -125,6 +125,13 @@ public ZkWriteCommand createCollection(ClusterState
clusterState, ZkNodeProps me
collectionProps.put(ZkStateReader.CONFIGNAME_PROP, configName);
}
+ // add user-defined properties
+ for (String prop : message.keySet()) {
Review Comment:
use EntrySet or better forEach; avoids the get()
##########
solr/core/src/java/org/apache/solr/cloud/overseer/SliceMutator.java:
##########
@@ -94,25 +94,30 @@ public ZkWriteCommand addReplica(ClusterState clusterState,
ZkNodeProps message)
cloudManager
.getClusterStateProvider()
.getClusterProperty(ZkStateReader.URL_SCHEME, "http"));
- Replica replica =
- new Replica(
- coreNodeName,
- Utils.makeMap(
- ZkStateReader.CORE_NAME_PROP,
- message.getStr(ZkStateReader.CORE_NAME_PROP),
- ZkStateReader.STATE_PROP,
- message.getStr(ZkStateReader.STATE_PROP),
- ZkStateReader.NODE_NAME_PROP,
- nodeName,
- ZkStateReader.BASE_URL_PROP,
- baseUrl,
- ZkStateReader.FORCE_SET_STATE_PROP,
- "false",
- ZkStateReader.REPLICA_TYPE,
- message.get(ZkStateReader.REPLICA_TYPE)),
- coll,
- slice);
+ Map<String, Object> replicaProps =
+ Utils.makeMap(
+ ZkStateReader.CORE_NAME_PROP,
+ message.getStr(ZkStateReader.CORE_NAME_PROP),
+ ZkStateReader.STATE_PROP,
+ message.getStr(ZkStateReader.STATE_PROP),
+ ZkStateReader.NODE_NAME_PROP,
+ nodeName,
+ ZkStateReader.BASE_URL_PROP,
+ baseUrl,
+ ZkStateReader.FORCE_SET_STATE_PROP,
+ "false",
+ ZkStateReader.REPLICA_TYPE,
+ message.get(ZkStateReader.REPLICA_TYPE));
+
+ // add user-defined properties
+ for (String prop : message.keySet()) {
Review Comment:
forEach here too
##########
solr/core/src/test/org/apache/solr/cloud/CreateCollectionTest.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * 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;
+
+import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.common.cloud.DocCollection;
+import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.core.CoreDescriptor;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class CreateCollectionTest extends SolrCloudTestCase {
Review Comment:
You are adding a new "CreateCollectionTest" and it kind of amuses me. Lets
rename this to something like CreateCollectionWithPropertiesTest
##########
solr/solr-ref-guide/modules/deployment-guide/pages/collection-management.adoc:
##########
@@ -250,6 +250,7 @@ If `true` the states of individual replicas will be
maintained as individual chi
+
Set core property _name_ to _value_.
See the section xref:configuration-guide:core-discovery.adoc[] for details on
supported properties and values.
Review Comment:
Not evident here is that you can specify whatever custom ones and refer to
them in the configuration. Can you say that please?
--
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
