You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ab...@apache.org on 2020/02/07 16:56:50 UTC
[lucene-solr] branch master updated: SOLR-14245: Validate Replica /
ReplicaInfo on creation.
This is an automated email from the ASF dual-hosted git repository.
ab pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/master by this push:
new 9a19093 SOLR-14245: Validate Replica / ReplicaInfo on creation.
9a19093 is described below
commit 9a190935869a5fba8c4935f85988fe712066c465
Author: Andrzej Bialecki <ab...@apache.org>
AuthorDate: Fri Feb 7 17:56:28 2020 +0100
SOLR-14245: Validate Replica / ReplicaInfo on creation.
---
solr/CHANGES.txt | 2 +
.../cloud/api/collections/CreateCollectionCmd.java | 1 +
.../apache/solr/cloud/ClusterStateMockUtil.java | 1 +
.../org/apache/solr/cloud/ClusterStateTest.java | 2 +
.../test/org/apache/solr/cloud/SliceStateTest.java | 2 +
.../solrj/cloud/autoscaling/ReplicaInfo.java | 21 +++++--
.../java/org/apache/solr/common/cloud/Replica.java | 17 ++++--
.../client/solrj/cloud/autoscaling/TestPolicy.java | 69 ++++++++++++++++++----
.../solrj/routing/ReplicaListTransformerTest.java | 3 +
.../ShufflingReplicaListTransformerTest.java | 9 ++-
10 files changed, 108 insertions(+), 19 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 8861bce..e660350 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -220,6 +220,8 @@ Improvements
* SOLR-10567: Add Support for DateRangeField in JSON Facet range (Stephen Weiss, Munendra S N)
+* SOLR-14245: Validate Replica / ReplicaInfo on creation. (ab)
+
Optimizations
---------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
index fe87182..2e81d51 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
@@ -243,6 +243,7 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd
ZkStateReader.CORE_NAME_PROP, coreName,
ZkStateReader.STATE_PROP, Replica.State.DOWN.toString(),
ZkStateReader.BASE_URL_PROP, baseUrl,
+ ZkStateReader.NODE_NAME_PROP, nodeName,
ZkStateReader.REPLICA_TYPE, replicaPosition.type.name(),
CommonAdminParams.WAIT_FOR_FINAL_STATE, Boolean.toString(waitForFinalState));
ocmh.overseer.offerStateUpdate(Utils.toJSON(props));
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 10cfdfb..63ee8ec 100644
--- a/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtil.java
+++ b/solr/core/src/test/org/apache/solr/cloud/ClusterStateMockUtil.java
@@ -171,6 +171,7 @@ public class ClusterStateMockUtil {
replicaPropMap.put(ZkStateReader.NODE_NAME_PROP, nodeName);
replicaPropMap.put(ZkStateReader.BASE_URL_PROP, "http://baseUrl" + node);
replicaPropMap.put(ZkStateReader.STATE_PROP, state.toString());
+ replicaPropMap.put(ZkStateReader.CORE_NAME_PROP, "core_" + replicaName);
if(collName == null) collName = "collection" + (collectionStates.size() + 1);
if(sliceName == null) collName = "slice" + (slices.size() + 1);
replica = new Replica(replicaName, replicaPropMap, collName, sliceName);
diff --git a/solr/core/src/test/org/apache/solr/cloud/ClusterStateTest.java b/solr/core/src/test/org/apache/solr/cloud/ClusterStateTest.java
index 9af6bbf..5606c5b 100644
--- a/solr/core/src/test/org/apache/solr/cloud/ClusterStateTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/ClusterStateTest.java
@@ -42,6 +42,8 @@ public class ClusterStateTest extends SolrTestCaseJ4 {
Map<String,Slice> slices = new HashMap<>();
Map<String,Replica> sliceToProps = new HashMap<>();
Map<String,Object> props = new HashMap<>();
+ props.put("node_name", "node1:10000_solr");
+ props.put("core", "core1");
props.put("prop1", "value");
props.put("prop2", "value2");
diff --git a/solr/core/src/test/org/apache/solr/cloud/SliceStateTest.java b/solr/core/src/test/org/apache/solr/cloud/SliceStateTest.java
index cd6bb89..cab5420 100644
--- a/solr/core/src/test/org/apache/solr/cloud/SliceStateTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/SliceStateTest.java
@@ -41,6 +41,8 @@ public class SliceStateTest extends SolrTestCaseJ4 {
Map<String, Slice> slices = new HashMap<>();
Map<String, Replica> sliceToProps = new HashMap<>();
Map<String, Object> props = new HashMap<>();
+ props.put("node_name", "127.0.0.1:10000_solr");
+ props.put("core", "core1");
Replica replica = new Replica("node1", props, "collection1", "shard1");
sliceToProps.put("node1", replica);
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaInfo.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaInfo.java
index f9a83de..09b8bfa 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaInfo.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaInfo.java
@@ -23,6 +23,7 @@ import java.util.HashMap;
import java.util.HashSet;
import java.util.Locale;
import java.util.Map;
+import java.util.Objects;
import java.util.function.BiPredicate;
import org.apache.solr.common.MapWriter;
@@ -37,9 +38,9 @@ import static org.apache.solr.common.cloud.ZkStateReader.LEADER_PROP;
public class ReplicaInfo implements MapWriter {
private final String name;
- private String core, collection, shard;
- private Replica.Type type;
- private String node;
+ private final String core, collection, shard;
+ private final Replica.Type type;
+ private final String node;
public final boolean isLeader;
private final Map<String, Object> variables = new HashMap<>();
@@ -49,13 +50,14 @@ public class ReplicaInfo implements MapWriter {
this.collection = coll;
this.shard = shard;
this.type = r.getType();
+ this.node = r.getNodeName();
boolean maybeLeader = r.getBool(LEADER_PROP, false);
if (vals != null) {
this.variables.putAll(vals);
maybeLeader = "true".equals(String.valueOf(vals.getOrDefault(LEADER_PROP, maybeLeader)));
}
this.isLeader = maybeLeader;
- this.node = r.getNodeName();
+ validate();
}
public ReplicaInfo(String name, String core, String coll, String shard, Replica.Type type, String node, Map<String, Object> vals) {
@@ -70,6 +72,7 @@ public class ReplicaInfo implements MapWriter {
this.type = type;
this.core = core;
this.node = node;
+ validate();
}
public ReplicaInfo(Map<String, Object> map) {
@@ -85,6 +88,16 @@ public class ReplicaInfo implements MapWriter {
type = Replica.Type.valueOf((String) details.getOrDefault("type", "NRT"));
details.remove("type");
this.variables.putAll(details);
+ validate();
+ }
+
+ private final void validate() {
+ Objects.requireNonNull(this.name, "'name' must not be null");
+ Objects.requireNonNull(this.core, "'core' must not be null");
+ Objects.requireNonNull(this.collection, "'collection' must not be null");
+ Objects.requireNonNull(this.shard, "'shard' must not be null");
+ Objects.requireNonNull(this.type, "'type' must not be null");
+ Objects.requireNonNull(this.node, "'node' must not be null");
}
public Object clone() {
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/Replica.java b/solr/solrj/src/java/org/apache/solr/common/cloud/Replica.java
index 5ff10c2..1409e6c 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/Replica.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/Replica.java
@@ -18,6 +18,7 @@ package org.apache.solr.common.cloud;
import java.util.Locale;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import org.apache.solr.common.util.Utils;
@@ -108,6 +109,7 @@ public class Replica extends ZkNodeProps {
private final String name;
private final String nodeName;
+ private final String core;
private final State state;
private final Type type;
public final String slice, collection;
@@ -118,13 +120,20 @@ public class Replica extends ZkNodeProps {
this.slice = slice;
this.name = name;
this.nodeName = (String) propMap.get(ZkStateReader.NODE_NAME_PROP);
+ this.core = (String) propMap.get(ZkStateReader.CORE_NAME_PROP);
+ type = Type.get((String) propMap.get(ZkStateReader.REPLICA_TYPE));
+ Objects.requireNonNull(this.collection, "'collection' must not be null");
+ Objects.requireNonNull(this.slice, "'slice' must not be null");
+ Objects.requireNonNull(this.name, "'name' must not be null");
+ Objects.requireNonNull(this.nodeName, "'node_name' must not be null");
+ Objects.requireNonNull(this.core, "'core' must not be null");
+ Objects.requireNonNull(this.type, "'type' must not be null");
if (propMap.get(ZkStateReader.STATE_PROP) != null) {
this.state = State.getState((String) propMap.get(ZkStateReader.STATE_PROP));
} else {
this.state = State.ACTIVE; //Default to ACTIVE
propMap.put(ZkStateReader.STATE_PROP, state.toString());
}
- type = Type.get((String) propMap.get(ZkStateReader.REPLICA_TYPE));
}
public String getCollection(){
@@ -151,7 +160,7 @@ public class Replica extends ZkNodeProps {
}
public String getCoreUrl() {
- return ZkCoreNodeProps.getCoreUrl(getStr(ZkStateReader.BASE_URL_PROP), getStr(ZkStateReader.CORE_NAME_PROP));
+ return ZkCoreNodeProps.getCoreUrl(getStr(ZkStateReader.BASE_URL_PROP), core);
}
public String getBaseUrl(){
return getStr(ZkStateReader.BASE_URL_PROP);
@@ -159,7 +168,7 @@ public class Replica extends ZkNodeProps {
/** SolrCore name. */
public String getCoreName() {
- return getStr(ZkStateReader.CORE_NAME_PROP);
+ return core;
}
/** The name of the node this replica resides on */
@@ -183,7 +192,7 @@ public class Replica extends ZkNodeProps {
public String getProperty(String propertyName) {
final String propertyKey;
if (!propertyName.startsWith(ZkStateReader.PROPERTY_PROP_PREFIX)) {
- propertyKey = ZkStateReader.PROPERTY_PROP_PREFIX+propertyName;
+ propertyKey = ZkStateReader.PROPERTY_PROP_PREFIX + propertyName;
} else {
propertyKey = propertyName;
}
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java b/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java
index e6917ec..b785142 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java
@@ -1469,8 +1469,8 @@ public class TestPolicy extends SolrTestCaseJ4 {
});
Row row = session.getNode("nodex");
- Row r1 = row.addReplica("c1", "s1", null);
- Row r2 = r1.addReplica("c1", "s1", null);
+ Row r1 = row.addReplica("c1", "s1", Replica.Type.NRT);
+ Row r2 = r1.addReplica("c1", "s1", Replica.Type.NRT);
assertEquals(1, r1.collectionVsShardVsReplicas.get("c1").get("s1").size());
assertEquals(2, r2.collectionVsShardVsReplicas.get("c1").get("s1").size());
assertTrue(r2.collectionVsShardVsReplicas.get("c1").get("s1").get(0) instanceof ReplicaInfo);
@@ -2594,13 +2594,13 @@ public class TestPolicy extends SolrTestCaseJ4 {
if (node.equals("node1")) {
Map m = Utils.makeMap("newColl",
Utils.makeMap("shard1", Collections.singletonList(new ReplicaInfo("r1", "shard1",
- new Replica("r1", Utils.makeMap(ZkStateReader.NODE_NAME_PROP, "node1"), "newColl", "shard1"),
+ new Replica("r1", Utils.makeMap(ZkStateReader.NODE_NAME_PROP, "node1", ZkStateReader.CORE_NAME_PROP, "core1"), "newColl", "shard1"),
Utils.makeMap(FREEDISK.perReplicaValue, 200)))));
return m;
} else if (node.equals("node2")) {
Map m = Utils.makeMap("newColl",
Utils.makeMap("shard2", Collections.singletonList(new ReplicaInfo("r1", "shard2",
- new Replica("r1", Utils.makeMap(ZkStateReader.NODE_NAME_PROP, "node2"),"newColl", "shard2"),
+ new Replica("r1", Utils.makeMap(ZkStateReader.NODE_NAME_PROP, "node2", ZkStateReader.CORE_NAME_PROP, "core2"),"newColl", "shard2"),
Utils.makeMap(FREEDISK.perReplicaValue, 200)))));
return m;
}
@@ -2623,9 +2623,9 @@ public class TestPolicy extends SolrTestCaseJ4 {
@Override
public Replica getLeader(String sliceName) {
if (sliceName.equals("shard1"))
- return new Replica("r1", Utils.makeMap(ZkStateReader.NODE_NAME_PROP, "node1"), name, "shard1");
+ return new Replica("r1", Utils.makeMap(ZkStateReader.NODE_NAME_PROP, "node1", ZkStateReader.CORE_NAME_PROP, "core1"), name, "shard1");
if (sliceName.equals("shard2"))
- return new Replica("r2", Utils.makeMap(ZkStateReader.NODE_NAME_PROP, "node2"),name, "shard2");
+ return new Replica("r2", Utils.makeMap(ZkStateReader.NODE_NAME_PROP, "node2", ZkStateReader.CORE_NAME_PROP, "core2"),name, "shard2");
return null;
}
};
@@ -2642,7 +2642,12 @@ public class TestPolicy extends SolrTestCaseJ4 {
public void testMoveReplicaLeaderlast() {
List<Pair<ReplicaInfo, Row>> validReplicas = new ArrayList<>();
- Replica replica = new Replica("r1", Utils.makeMap("leader", "true"), "c1", "s1");
+ Map<String, Object> propMap = Utils.makeMap(
+ "leader", "true",
+ ZkStateReader.NODE_NAME_PROP, "node1",
+ ZkStateReader.REPLICA_TYPE, Replica.Type.NRT.toString(),
+ ZkStateReader.CORE_NAME_PROP, "core1");
+ Replica replica = new Replica("r1", propMap, "c1", "s1");
ReplicaInfo replicaInfo = new ReplicaInfo(replica.collection, replica.slice ,replica, new HashMap<>());
validReplicas.add(new Pair<>(replicaInfo, null));
@@ -2650,11 +2655,12 @@ public class TestPolicy extends SolrTestCaseJ4 {
validReplicas.add(new Pair<>(replicaInfo, null));
- replica = new Replica("r2", Utils.makeMap("leader", false),"c1","s1");
+ propMap.put("leader", false);
+ replica = new Replica("r2", propMap,"c1","s1");
replicaInfo = new ReplicaInfo(replica.collection, replica.slice, replica, new HashMap<>());
validReplicas.add(new Pair<>(replicaInfo, null));
- replica = new Replica("r3", Utils.makeMap("leader", false),"c1","s1");
+ replica = new Replica("r3", propMap,"c1","s1");
replicaInfo = new ReplicaInfo(replica.collection,replica.slice, replica, new HashMap<>());
validReplicas.add(new Pair<>(replicaInfo, null));
@@ -2832,8 +2838,51 @@ public class TestPolicy extends SolrTestCaseJ4 {
}
+ public static void fixRequiredProps(Map<String, Object> testData) {
+ Map<String, Object> clusterState = (Map<String, Object>) testData.get("clusterstate");
+ clusterState.forEach((collection, val) -> {
+ Map<String, Object> docColl = (Map<String, Object>) val;
+ Map<String, Object> shards = (Map<String, Object>) docColl.get("shards");
+ shards.forEach((shardName, val2) -> {
+ Map<String, Object> shard = (Map<String, Object>) val2;
+ Map<String, Object> replicas = (Map<String, Object>) shard.get("replicas");
+ replicas.forEach((coreNode, val3) -> {
+ Map<String, Object> replica = (Map<String, Object>) val3;
+ if (!replica.containsKey("node_name")) {
+ replica.put("node_name", "node1");
+ }
+ if (!replica.containsKey("core")) {
+ replica.put("core", "core_" + coreNode);
+ }
+ });
+ });
+ });
+ Map<String, Object> replicaInfo = (Map<String, Object>) testData.get("replicaInfo");
+ replicaInfo.forEach((node, val) -> {
+ Map m1 = (Map) val;
+ m1.forEach((coll, val2) -> {
+ Map m2 = (Map) val2;
+ m2.forEach((shard, val3) -> {
+ List l3 = (List) val3;
+ l3.forEach(o -> {
+ Map replica = (Map) o;
+ String coreNode = replica.keySet().iterator().next().toString();
+ replica = (Map) replica.get(coreNode);
+ if (!replica.containsKey("node_name")) {
+ replica.put("node_name", "node1");
+ }
+ if (!replica.containsKey("core")) {
+ replica.put("core", "core_" + coreNode);
+ }
+ });
+ });
+ });
+ });
+ }
+
public void testAutoscalingPreferencesUsedWithNoPolicy() throws IOException, InterruptedException {
- Map m = (Map) loadFromResource("testAutoscalingPreferencesUsedWithNoPolicy.json");
+ Map<String, Object> m = (Map<String, Object>) loadFromResource("testAutoscalingPreferencesUsedWithNoPolicy.json");
+ fixRequiredProps(m);
Map clusterState = (Map) m.remove("clusterstate");
Map replicaInfo = (Map) m.get("replicaInfo");
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ReplicaListTransformerTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ReplicaListTransformerTest.java
index 905f82e..2b784e5 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ReplicaListTransformerTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ReplicaListTransformerTest.java
@@ -126,6 +126,9 @@ public class ReplicaListTransformerTest extends SolrTestCase {
final String url = urls.get(ii);
final Map<String,Object> propMap = new HashMap<String,Object>();
propMap.put("base_url", url);
+ propMap.put("core", "core1");
+ propMap.put("node_name", "node1");
+ propMap.put("type", "NRT");
// a skeleton replica, good enough for this test's purposes
final Replica replica = new Replica(name, propMap,"c1","s1");
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ShufflingReplicaListTransformerTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ShufflingReplicaListTransformerTest.java
index 33d1baa..4350511 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ShufflingReplicaListTransformerTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/routing/ShufflingReplicaListTransformerTest.java
@@ -21,6 +21,7 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
+import java.util.Map;
import java.util.Set;
import org.apache.solr.SolrTestCase;
@@ -34,8 +35,14 @@ public class ShufflingReplicaListTransformerTest extends SolrTestCase {
@Test
public void testTransformReplicas() throws Exception {
final List<Replica> replicas = new ArrayList<>();
+ int counter = 0;
for (final String url : createRandomUrls()) {
- replicas.add(new Replica(url, new HashMap<String,Object>(),"c1","s1"));
+ Map<String, Object> propMap = new HashMap<>();
+ propMap.put("core", "core" + counter);
+ propMap.put("type", "NRT");
+ propMap.put("node_name", "node" + counter);
+ counter++;
+ replicas.add(new Replica(url, propMap, "c1", "s1"));
}
implTestTransform(replicas);
}