madrob commented on a change in pull request #2479:
URL: https://github.com/apache/lucene-solr/pull/2479#discussion_r605251098
##########
File path: solr/core/src/java/org/apache/solr/cloud/overseer/NodeMutator.java
##########
@@ -24,19 +24,28 @@
import java.util.Map;
import java.util.Map.Entry;
+import org.apache.solr.client.solrj.cloud.SolrCloudManager;
import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.cloud.DocCollection;
+import org.apache.solr.common.cloud.PerReplicaStates;
import org.apache.solr.common.cloud.PerReplicaStatesOps;
import org.apache.solr.common.cloud.Replica;
import org.apache.solr.common.cloud.Slice;
+import org.apache.solr.common.cloud.SolrZkClient;
import org.apache.solr.common.cloud.ZkNodeProps;
import org.apache.solr.common.cloud.ZkStateReader;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static org.apache.solr.cloud.overseer.SliceMutator.getZkClient;
+
public class NodeMutator {
private static final Logger log =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ protected SolrZkClient zkClient;
+ public NodeMutator(SolrCloudManager cloudManager) {
+ zkClient = getZkClient(cloudManager);
Review comment:
please use the qualified classname instead of a static import. easier to
find during review.
##########
File path: solr/core/src/test/org/apache/solr/cloud/NodeMutatorTest.java
##########
@@ -43,7 +43,7 @@
@Test
public void downNodeReportsAllImpactedCollectionsAndNothingElse() throws
IOException {
- NodeMutator nm = new NodeMutator();
+ NodeMutator nm = new NodeMutator(null);
Review comment:
should we add a new constructor instead?
##########
File path:
solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
##########
@@ -1099,7 +1102,7 @@ public void testPerReplicaStateCollection() throws
Exception {
c.forEachReplica((s, replica) -> assertNotNull(replica.getReplicaState()));
PerReplicaStates prs =
PerReplicaStates.fetch(ZkStateReader.getCollectionPath(testCollection),
cluster.getZkClient(), null);
assertEquals(4, prs.states.size());
-
+ JettySolrRunner jsr = cluster.startJettySolrRunner();
Review comment:
unused store? shouldn't the cluster already be running at this point?
##########
File path:
solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
##########
@@ -1120,4 +1123,63 @@ public void testPerReplicaStateCollection() throws
Exception {
assertEquals(4, prs.states.size());
}
+ public void testPRSRestart() throws Exception {
+ String testCollection = "prs_restart_test";
+ MiniSolrCloudCluster cluster1 =
+ configureCluster(1)
+ .addConfig("conf1",
getFile("solrj").toPath().resolve("solr").resolve("configsets").resolve("streaming").resolve("conf"))
+ .withJettyConfig(jetty -> jetty.enableV2(true))
+ .configure();
+ try {
+ CollectionAdminRequest.createCollection(testCollection, "conf1", 1, 1)
+ .setPerReplicaState(Boolean.TRUE)
+ .process(cluster.getSolrClient());
+ cluster1.waitForActiveCollection(testCollection, 1, 1);
+
+ DocCollection c =
cluster1.getSolrClient().getZkStateReader().getCollection(testCollection);
+ c.forEachReplica((s, replica) ->
assertNotNull(replica.getReplicaState()));
+ String collectionPath = ZkStateReader.getCollectionPath(testCollection);
+ PerReplicaStates prs = PerReplicaStates.fetch(collectionPath,
cluster.getZkClient(), null);
+ assertEquals(1, prs.states.size());
+
+ JettySolrRunner jsr = cluster1.startJettySolrRunner();
+ assertEquals(2,cluster1.getJettySolrRunners().size());
+
+ // Now let's do an add replica
+ CollectionAdminRequest
+ .addReplicaToShard(testCollection, "shard1")
+ .process(cluster1.getSolrClient());
+ cluster1.waitForActiveCollection(testCollection, 1, 2);
+ prs = PerReplicaStates.fetch(collectionPath, cluster.getZkClient(),
null);
+ assertEquals(2, prs.states.size());
+ c =
cluster1.getSolrClient().getZkStateReader().getCollection(testCollection);
+ prs.states.forEachEntry((s, state) -> assertEquals(Replica.State.ACTIVE,
state.state));
+
+ String replicaName = null;
+ for (Replica r : c.getSlice("shard1").getReplicas()) {
Review comment:
getReplica(BiPredicate) might be cleaner?
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
