You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by sh...@apache.org on 2020/02/09 02:27:47 UTC
[lucene-solr] 02/02: SOLR-13996: Fixes a bug in the IsLeader
predicate that used the builder's collection instead of the slice's
collection. Added tests for CloudReplicaSource. Added support for making
the first nrt or tlog replica as the leader in ClusterStateMockUtil.
This is an automated email from the ASF dual-hosted git repository.
shalin pushed a commit to branch jira/solr-13996
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit f93ac6e6dd6220f3884c9968310d1732617d0414
Author: Shalin Shekhar Mangar <sh...@apache.org>
AuthorDate: Sun Feb 9 07:57:25 2020 +0530
SOLR-13996: Fixes a bug in the IsLeader predicate that used the builder's collection instead of the slice's collection. Added tests for CloudReplicaSource. Added support for making the first nrt or tlog replica as the leader in ClusterStateMockUtil.
---
.../solr/handler/component/CloudReplicaSource.java | 2 +-
.../apache/solr/cloud/ClusterStateMockUtil.java | 19 +-
.../handler/component/CloudReplicaSourceTest.java | 263 +++++++++++++++++++++
.../org/apache/solr/cloud/MockZkStateReader.java | 7 +
4 files changed, 289 insertions(+), 2 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/handler/component/CloudReplicaSource.java b/solr/core/src/java/org/apache/solr/handler/component/CloudReplicaSource.java
index 272c00d..fb0285c 100644
--- a/solr/core/src/java/org/apache/solr/handler/component/CloudReplicaSource.java
+++ b/solr/core/src/java/org/apache/solr/handler/component/CloudReplicaSource.java
@@ -126,7 +126,7 @@ class CloudReplicaSource implements ReplicaSource {
// if partial results are acceptable
return Collections.emptyList();
} else {
- final Predicate<Replica> isShardLeader = new IsLeaderPredicate(builder.zkStateReader, clusterState, builder.collection, slice.getName());
+ final Predicate<Replica> isShardLeader = new IsLeaderPredicate(builder.zkStateReader, clusterState, slice.getCollection(), slice.getName());
List<Replica> list = slice.getReplicas()
.stream()
.filter(replica -> replica.isActive(clusterState.getLiveNodes()))
diff --git a/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtil.java b/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtil.java
index 90df39a..f41d80a 100644
--- a/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtil.java
+++ b/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtil.java
@@ -158,9 +158,26 @@ public class ClusterStateMockUtil {
Map<String, Object> replicaPropMap = makeReplicaProps(sliceName, node, replicaName, stateCode, m.group(1));
if (collName == null) collName = "collection" + (collectionStates.size() + 1);
if (sliceName == null) collName = "slice" + (slices.size() + 1);
- replica = new Replica(replicaName, replicaPropMap, collName, sliceName);
+ // O(n^2) alert! but this is for mocks and testing so shouldn't be used for very large cluster states
+ boolean leaderFound = false;
+ for (Map.Entry<String, Replica> entry : replicas.entrySet()) {
+ Replica value = entry.getValue();
+ if ("true".equals(value.get(Slice.LEADER))) {
+ leaderFound = true;
+ break;
+ }
+ }
+ if (!leaderFound && !m.group(1).equals("p")) {
+ replicaPropMap.put(Slice.LEADER, "true");
+ }
+ replica = new Replica(replicaName, replicaPropMap, collName, sliceName);
replicas.put(replica.getName(), replica);
+
+ // hack alert: re-create slice with existing data and new replicas map so that it updates its internal leader attribute
+ slice = new Slice(slice.getName(), replicas, null, collName);
+ slices.put(slice.getName(), slice);
+ // we don't need to update doc collection again because we aren't adding a new slice or changing its state
break;
default:
break;
diff --git a/solr/core/src/test/org/apache/solr/handler/component/CloudReplicaSourceTest.java b/solr/core/src/test/org/apache/solr/handler/component/CloudReplicaSourceTest.java
new file mode 100644
index 0000000..186af33
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/handler/component/CloudReplicaSourceTest.java
@@ -0,0 +1,263 @@
+/*
+ * 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.handler.component;
+
+import java.util.List;
+
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.routing.ReplicaListTransformer;
+import org.apache.solr.cloud.ClusterStateMockUtil;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.mockito.Mockito;
+
+/**
+ * Tests for {@link CloudReplicaSource}
+ */
+public class CloudReplicaSourceTest extends SolrTestCaseJ4 {
+
+ @BeforeClass
+ public static void setup() {
+ assumeWorkingMockito();
+ }
+
+ @Test
+ public void testSimple_ShardsParam() {
+ ReplicaListTransformer replicaListTransformer = Mockito.mock(ReplicaListTransformer.class);
+ HttpShardHandlerFactory.WhitelistHostChecker whitelistHostChecker = Mockito.mock(HttpShardHandlerFactory.WhitelistHostChecker.class);
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ params.set("shards", "slice1,slice2");
+ try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2", "baseUrl1_", "baseUrl2_")) {
+ CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder()
+ .collection("collection1")
+ .onlyNrt(false)
+ .zkStateReader(zkStateReader)
+ .replicaListTransformer(replicaListTransformer)
+ .whitelistHostChecker(whitelistHostChecker)
+ .params(params)
+ .build();
+ assertEquals(2, cloudReplicaSource.getSliceCount());
+ assertEquals(2, cloudReplicaSource.getSliceNames().size());
+ assertEquals(1, cloudReplicaSource.getReplicasBySlice(0).size());
+ assertEquals("http://baseUrl1/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(0).get(0));
+ assertEquals(1, cloudReplicaSource.getReplicasBySlice(1).size());
+ assertEquals("http://baseUrl2/slice2_replica2/", cloudReplicaSource.getReplicasBySlice(1).get(0));
+ }
+ }
+
+ @Test
+ public void testShardsParam_DeadNode() {
+ ReplicaListTransformer replicaListTransformer = Mockito.mock(ReplicaListTransformer.class);
+ HttpShardHandlerFactory.WhitelistHostChecker whitelistHostChecker = Mockito.mock(HttpShardHandlerFactory.WhitelistHostChecker.class);
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ params.set("shards", "slice1,slice2");
+ // here node2 is not live so there should be no replicas found for slice2
+ try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2", "baseUrl1_")) {
+ CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder()
+ .collection("collection1")
+ .onlyNrt(false)
+ .zkStateReader(zkStateReader)
+ .replicaListTransformer(replicaListTransformer)
+ .whitelistHostChecker(whitelistHostChecker)
+ .params(params)
+ .build();
+ assertEquals(2, cloudReplicaSource.getSliceCount());
+ assertEquals(2, cloudReplicaSource.getSliceNames().size());
+ assertEquals(1, cloudReplicaSource.getReplicasBySlice(0).size());
+ assertEquals("http://baseUrl1/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(0).get(0));
+ assertEquals(0, cloudReplicaSource.getReplicasBySlice(1).size());
+ }
+ }
+
+ @Test
+ public void testShardsParam_DownReplica() {
+ ReplicaListTransformer replicaListTransformer = Mockito.mock(ReplicaListTransformer.class);
+ HttpShardHandlerFactory.WhitelistHostChecker whitelistHostChecker = Mockito.mock(HttpShardHandlerFactory.WhitelistHostChecker.class);
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ params.set("shards", "slice1,slice2");
+ // here replica3 is in DOWN state so only 1 replica should be returned for slice2
+ try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2r3D", "baseUrl1_", "baseUrl2_", "baseUrl3_")) {
+ CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder()
+ .collection("collection1")
+ .onlyNrt(false)
+ .zkStateReader(zkStateReader)
+ .replicaListTransformer(replicaListTransformer)
+ .whitelistHostChecker(whitelistHostChecker)
+ .params(params)
+ .build();
+ assertEquals(2, cloudReplicaSource.getSliceCount());
+ assertEquals(2, cloudReplicaSource.getSliceNames().size());
+ assertEquals(1, cloudReplicaSource.getReplicasBySlice(0).size());
+ assertEquals("http://baseUrl1/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(0).get(0));
+ assertEquals(1, cloudReplicaSource.getReplicasBySlice(1).size());
+ assertEquals(1, cloudReplicaSource.getReplicasBySlice(1).size());
+ assertEquals("http://baseUrl2/slice2_replica2/", cloudReplicaSource.getReplicasBySlice(1).get(0));
+ }
+ }
+
+ @Test
+ public void testMultipleCollections() {
+ ReplicaListTransformer replicaListTransformer = Mockito.mock(ReplicaListTransformer.class);
+ HttpShardHandlerFactory.WhitelistHostChecker whitelistHostChecker = Mockito.mock(HttpShardHandlerFactory.WhitelistHostChecker.class);
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ params.set("collection", "collection1,collection2");
+ try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2csr*", "baseUrl1_", "baseUrl2_")) {
+ CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder()
+ .collection("collection1")
+ .onlyNrt(false)
+ .zkStateReader(zkStateReader)
+ .replicaListTransformer(replicaListTransformer)
+ .whitelistHostChecker(whitelistHostChecker)
+ .params(params)
+ .build();
+ assertEquals(3, cloudReplicaSource.getSliceCount());
+ List<String> sliceNames = cloudReplicaSource.getSliceNames();
+ assertEquals(3, sliceNames.size());
+ for (int i = 0; i < cloudReplicaSource.getSliceCount(); i++) {
+ String sliceName = sliceNames.get(i);
+ assertEquals(1, cloudReplicaSource.getReplicasBySlice(i).size());
+
+ // need a switch here because unlike the testShards* tests which always returns slices in the order they were specified,
+ // using the collection param can return slice names in any order
+ switch (sliceName) {
+ case "collection1_slice1":
+ assertEquals("http://baseUrl1/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+ break;
+ case "collection1_slice2":
+ assertEquals("http://baseUrl2/slice2_replica2/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+ break;
+ case "collection2_slice1":
+ assertEquals("http://baseUrl1/slice1_replica3/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+ break;
+ }
+ }
+ }
+ }
+
+ @Test
+ public void testSimple_UsingClusterState() {
+ ReplicaListTransformer replicaListTransformer = Mockito.mock(ReplicaListTransformer.class);
+ HttpShardHandlerFactory.WhitelistHostChecker whitelistHostChecker = Mockito.mock(HttpShardHandlerFactory.WhitelistHostChecker.class);
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csr*sr2", "baseUrl1_", "baseUrl2_")) {
+ CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder()
+ .collection("collection1")
+ .onlyNrt(false)
+ .zkStateReader(zkStateReader)
+ .replicaListTransformer(replicaListTransformer)
+ .whitelistHostChecker(whitelistHostChecker)
+ .params(params)
+ .build();
+ assertEquals(2, cloudReplicaSource.getSliceCount());
+ List<String> sliceNames = cloudReplicaSource.getSliceNames();
+ assertEquals(2, sliceNames.size());
+ for (int i = 0; i < cloudReplicaSource.getSliceCount(); i++) {
+ String sliceName = sliceNames.get(i);
+ assertEquals(1, cloudReplicaSource.getReplicasBySlice(i).size());
+
+ // need to switch because without a shards param, the order of slices is not deterministic
+ switch (sliceName) {
+ case "slice1":
+ assertEquals("http://baseUrl1/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+ break;
+ case "slice2":
+ assertEquals("http://baseUrl2/slice2_replica2/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+ break;
+ }
+ }
+ }
+ }
+
+ @Test
+ public void testSimple_OnlyNrt() {
+ ReplicaListTransformer replicaListTransformer = Mockito.mock(ReplicaListTransformer.class);
+ HttpShardHandlerFactory.WhitelistHostChecker whitelistHostChecker = Mockito.mock(HttpShardHandlerFactory.WhitelistHostChecker.class);
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ // the cluster state will have slice2 with two tlog replicas out of which the first one will be the leader
+ try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csrr*st2t2", "baseUrl1_", "baseUrl2_")) {
+ CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder()
+ .collection("collection1")
+ .onlyNrt(true) // enable only nrt mode
+ .zkStateReader(zkStateReader)
+ .replicaListTransformer(replicaListTransformer)
+ .whitelistHostChecker(whitelistHostChecker)
+ .params(params)
+ .build();
+ assertEquals(2, cloudReplicaSource.getSliceCount());
+ List<String> sliceNames = cloudReplicaSource.getSliceNames();
+ assertEquals(2, sliceNames.size());
+ for (int i = 0; i < cloudReplicaSource.getSliceCount(); i++) {
+ String sliceName = sliceNames.get(i);
+ // need to switch because without a shards param, the order of slices is not deterministic
+ switch (sliceName) {
+ case "slice1":
+ assertEquals(2, cloudReplicaSource.getReplicasBySlice(i).size());
+ assertEquals("http://baseUrl1/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+ break;
+ case "slice2":
+ assertEquals(1, cloudReplicaSource.getReplicasBySlice(i).size());
+ assertEquals("http://baseUrl2/slice2_replica3/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+ break;
+ }
+ }
+ }
+ }
+
+ @Test
+ public void testMultipleCollections_OnlyNrt() {
+ ReplicaListTransformer replicaListTransformer = Mockito.mock(ReplicaListTransformer.class);
+ HttpShardHandlerFactory.WhitelistHostChecker whitelistHostChecker = Mockito.mock(HttpShardHandlerFactory.WhitelistHostChecker.class);
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ params.set("collection", "collection1,collection2");
+ // the cluster state will have collection1 with slice2 with two tlog replicas out of which the first one will be the leader
+ // and collection2 with just a single slice and a tlog replica that will be leader
+ try (ZkStateReader zkStateReader = ClusterStateMockUtil.buildClusterState("csrr*st2t2cst", "baseUrl1_", "baseUrl2_")) {
+ CloudReplicaSource cloudReplicaSource = new CloudReplicaSource.Builder()
+ .collection("collection1")
+ .onlyNrt(true) // enable only nrt mode
+ .zkStateReader(zkStateReader)
+ .replicaListTransformer(replicaListTransformer)
+ .whitelistHostChecker(whitelistHostChecker)
+ .params(params)
+ .build();
+ assertEquals(3, cloudReplicaSource.getSliceCount());
+ List<String> sliceNames = cloudReplicaSource.getSliceNames();
+ assertEquals(3, sliceNames.size());
+ for (int i = 0; i < cloudReplicaSource.getSliceCount(); i++) {
+ String sliceName = sliceNames.get(i);
+ // need to switch because without a shards param, the order of slices is not deterministic
+ switch (sliceName) {
+ case "collection1_slice1":
+ assertEquals(2, cloudReplicaSource.getReplicasBySlice(i).size());
+ assertEquals("http://baseUrl1/slice1_replica1/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+ break;
+ case "collection1_slice2":
+ assertEquals(1, cloudReplicaSource.getReplicasBySlice(i).size());
+ assertEquals("http://baseUrl2/slice2_replica3/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+ break;
+ case "collection2_slice1":
+ assertEquals(1, cloudReplicaSource.getReplicasBySlice(i).size());
+ assertEquals("http://baseUrl1/slice1_replica5/", cloudReplicaSource.getReplicasBySlice(i).get(0));
+ break;
+ }
+ }
+ }
+ }
+}
diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/MockZkStateReader.java b/solr/test-framework/src/java/org/apache/solr/cloud/MockZkStateReader.java
index b0ba518..2f73792 100644
--- a/solr/test-framework/src/java/org/apache/solr/cloud/MockZkStateReader.java
+++ b/solr/test-framework/src/java/org/apache/solr/cloud/MockZkStateReader.java
@@ -19,6 +19,7 @@ package org.apache.solr.cloud;
import java.util.Set;
import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.DocCollectionWatcher;
import org.apache.solr.common.cloud.ZkStateReader;
// does not yet mock zkclient at all
@@ -36,4 +37,10 @@ public class MockZkStateReader extends ZkStateReader {
return collections;
}
+ @Override
+ public void registerDocCollectionWatcher(String collection, DocCollectionWatcher stateWatcher) {
+ // the doc collection will never be changed by this mock
+ // so we just call onStateChanged once with the existing DocCollection object an return
+ stateWatcher.onStateChanged(clusterState.getCollectionOrNull(collection));
+ }
}