Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id 9E829200CFD for ; Wed, 6 Sep 2017 15:46:58 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id 9D3F7160BDE; Wed, 6 Sep 2017 13:46:58 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 6F75E1609ED for ; Wed, 6 Sep 2017 15:46:57 +0200 (CEST) Received: (qmail 62259 invoked by uid 500); 6 Sep 2017 13:46:48 -0000 Mailing-List: contact commits-help@lucene.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@lucene.apache.org Delivered-To: mailing list commits@lucene.apache.org Received: (qmail 59898 invoked by uid 99); 6 Sep 2017 13:46:46 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 06 Sep 2017 13:46:46 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id A3070F56DB; Wed, 6 Sep 2017 13:46:45 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: ab@apache.org To: commits@lucene.apache.org Date: Wed, 06 Sep 2017 13:47:31 -0000 Message-Id: In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [48/50] lucene-solr:jira/solr-11285: SOLR-11307: The SHARD hint is deprecated in favor of a COLL_SHARD hint archived-at: Wed, 06 Sep 2017 13:46:58 -0000 SOLR-11307: The SHARD hint is deprecated in favor of a COLL_SHARD hint Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/054f445a Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/054f445a Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/054f445a Branch: refs/heads/jira/solr-11285 Commit: 054f445a27ce89fd710160ebf4084e7c4cf3acd3 Parents: b2cf38c Author: Noble Paul Authored: Mon Sep 4 22:55:55 2017 +0930 Committer: Noble Paul Committed: Mon Sep 4 22:55:55 2017 +0930 ---------------------------------------------------------------------- .../cloud/autoscaling/AddReplicaSuggester.java | 15 ++-- .../client/solrj/cloud/autoscaling/Policy.java | 73 +++++++++++++++----- .../solrj/cloud/autoscaling/PolicyHelper.java | 4 +- .../solrj/cloud/autoscaling/TestPolicy.java | 69 ++++++++---------- 4 files changed, 94 insertions(+), 67 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/054f445a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AddReplicaSuggester.java ---------------------------------------------------------------------- diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AddReplicaSuggester.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AddReplicaSuggester.java index c813e17..f96c66d 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AddReplicaSuggester.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AddReplicaSuggester.java @@ -17,6 +17,7 @@ package org.apache.solr.client.solrj.cloud.autoscaling; +import java.util.Collections; import java.util.List; import java.util.Set; @@ -24,6 +25,7 @@ import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.cloud.autoscaling.Policy.Suggester; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.common.cloud.Replica; +import org.apache.solr.common.util.Pair; class AddReplicaSuggester extends Suggester { @@ -34,12 +36,11 @@ class AddReplicaSuggester extends Suggester { } SolrRequest tryEachNode(boolean strict) { - Set collections = (Set) hints.get(Hint.COLL); - String shard = (String) hints.get(Hint.SHARD); - if (collections == null || shard == null) { + Set> shards = (Set>) hints.getOrDefault(Hint.COLL_SHARD, Collections.emptySet()); + if (shards.isEmpty()) { throw new RuntimeException("add-replica requires 'collection' and 'shard'"); } - for (String coll : collections) { + for (Pair shard : shards) { Replica.Type type = Replica.Type.get((String) hints.get(Hint.REPLICATYPE)); //iterate through elements and identify the least loaded List leastSeriousViolation = null; @@ -48,7 +49,7 @@ class AddReplicaSuggester extends Suggester { Row row = getMatrix().get(i); if (!row.isLive) continue; if (!isAllowed(row.node, Hint.TARGET_NODE)) continue; - Row tmpRow = row.addReplica(coll, shard, type); + Row tmpRow = row.addReplica(shard.first(), shard.second(), type); List errs = testChangedMatrix(strict, getModifiedMatrix(getMatrix(), tmpRow, i)); if (!containsNewErrors(errs)) { @@ -60,9 +61,9 @@ class AddReplicaSuggester extends Suggester { } if (targetNodeIndex != null) {// there are no rule violations - getMatrix().set(targetNodeIndex, getMatrix().get(targetNodeIndex).addReplica(coll, shard, type)); + getMatrix().set(targetNodeIndex, getMatrix().get(targetNodeIndex).addReplica(shard.first(), shard.second(), type)); return CollectionAdminRequest - .addReplicaToShard(coll, shard) + .addReplicaToShard(shard.first(), shard.second()) .setType(type) .setNode(getMatrix().get(targetNodeIndex).node); } http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/054f445a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Policy.java ---------------------------------------------------------------------- diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Policy.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Policy.java index 9c90e34..761f1b8 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Policy.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Policy.java @@ -32,6 +32,7 @@ import java.util.Objects; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; +import java.util.function.Consumer; import java.util.function.Supplier; import java.util.stream.Collectors; @@ -39,6 +40,7 @@ import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.cloud.autoscaling.Clause.Violation; import org.apache.solr.common.IteratorWriter; import org.apache.solr.common.MapWriter; +import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.params.CollectionParams.CollectionAction; import org.apache.solr.common.util.Pair; import org.apache.solr.common.util.StrUtils; @@ -360,7 +362,8 @@ public class Policy implements MapWriter { } public Suggester hint(Hint hint, Object value) { - if (hint == Hint.TARGET_NODE || hint == Hint.SRC_NODE || hint == Hint.COLL) { + hint.validator.accept(value); + if (hint.multiValued) { Collection values = value instanceof Collection ? (Collection)value : Collections.singletonList(value); ((Set) hints.computeIfAbsent(hint, h -> new HashSet<>())).addAll(values); } else { @@ -374,22 +377,20 @@ public class Policy implements MapWriter { public SolrRequest getOperation() { if (!isInitialized) { - Set collections = (Set) hints.get(Hint.COLL); - String shard = (String) hints.get(Hint.SHARD); - if (collections != null) { - for (String coll : collections) { + Set collections = (Set) hints.getOrDefault(Hint.COLL, Collections.emptySet()); + Set> s = (Set>) hints.getOrDefault(Hint.COLL_SHARD, Collections.emptySet()); + if (!collections.isEmpty() || !s.isEmpty()) { + HashSet> shards = new HashSet<>(s); + collections.stream().forEach(c -> shards.add(new Pair<>(c, null))); + for (Pair shard : shards) { // if this is not a known collection from the existing clusterstate, // then add it - if (session.matrix.stream().noneMatch(row -> row.collectionVsShardVsReplicas.containsKey(coll))) { - session.addClausesForCollection(session.dataProvider, coll); + if (session.matrix.stream().noneMatch(row -> row.collectionVsShardVsReplicas.containsKey(shard.first()))) { + session.addClausesForCollection(session.dataProvider, shard.first()); } for (Row row : session.matrix) { - if (!row.collectionVsShardVsReplicas.containsKey(coll)) - row.collectionVsShardVsReplicas.put(coll, new HashMap<>()); - if (shard != null) { - Map> shardInfo = row.collectionVsShardVsReplicas.get(coll); - if (!shardInfo.containsKey(shard)) shardInfo.put(shard, new ArrayList<>()); - } + Map> shardInfo = row.collectionVsShardVsReplicas.computeIfAbsent(shard.first(), it -> new HashMap<>()); + if (shard.second() != null) shardInfo.computeIfAbsent(shard.second(), it -> new ArrayList<>()); } } Collections.sort(session.expandedClauses); @@ -462,7 +463,7 @@ public class Policy implements MapWriter { for (Map.Entry>> e : r.collectionVsShardVsReplicas.entrySet()) { if (!isAllowed(e.getKey(), Hint.COLL)) continue; for (Map.Entry> shard : e.getValue().entrySet()) { - if (!isAllowed(e.getKey(), Hint.SHARD)) continue;//todo fix + if (!isAllowed(new Pair<>(e.getKey(), shard.getKey()), Hint.COLL_SHARD)) continue;//todo fix if(shard.getValue() == null || shard.getValue().isEmpty()) continue; replicaList.add(new Pair<>(shard.getValue().get(0), r)); } @@ -490,7 +491,7 @@ public class Policy implements MapWriter { protected boolean isAllowed(Object v, Hint hint) { Object hintVal = hints.get(hint); - if (hint == Hint.TARGET_NODE || hint == Hint.SRC_NODE || hint == Hint.COLL) { + if (hint.multiValued) { Set set = (Set) hintVal; return set == null || set.contains(v); } else { @@ -499,7 +500,47 @@ public class Policy implements MapWriter { } public enum Hint { - COLL, SHARD, SRC_NODE, TARGET_NODE, REPLICATYPE + COLL(true), + // collection shard pair + // this should be a Pair , (collection,shard) + COLL_SHARD(true, v -> { + Collection c = v instanceof Collection ? (Collection) v : Collections.singleton(v); + for (Object o : c) { + if (!(o instanceof Pair)) { + throw new RuntimeException("SHARD hint must use a Pair"); + } + Pair p = (Pair) o; + if (p.first() == null || p.second() == null) { + throw new RuntimeException("Both collection and shard must not be null"); + } + } + + }), + SRC_NODE(true), + TARGET_NODE(true), + REPLICATYPE(false, o -> { + if (!(o instanceof Replica.Type)) { + throw new RuntimeException("REPLICATYPE hint must use a ReplicaType"); + } + }); + + public final boolean multiValued; + public final Consumer validator; + + Hint(boolean multiValued) { + this(multiValued, v -> { + Collection c = v instanceof Collection ? (Collection) v : Collections.singleton(v); + for (Object o : c) { + if (!(o instanceof String)) throw new RuntimeException("hint must be of type String"); + } + }); + } + + Hint(boolean multiValued, Consumer c) { + this.multiValued = multiValued; + this.validator = c; + } + } http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/054f445a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/PolicyHelper.java ---------------------------------------------------------------------- diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/PolicyHelper.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/PolicyHelper.java index a8c99ec..bc2a5d7 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/PolicyHelper.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/PolicyHelper.java @@ -32,6 +32,7 @@ import org.apache.solr.client.solrj.cloud.autoscaling.Policy.Suggester.Hint; import org.apache.solr.common.SolrException; import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.cloud.ReplicaPosition; +import org.apache.solr.common.util.Pair; import org.apache.solr.common.util.Utils; import static org.apache.solr.common.params.CollectionParams.CollectionAction.ADDREPLICA; @@ -89,9 +90,8 @@ public class PolicyHelper { for (Map.Entry e : typeVsCount.entrySet()) { for (int i = 0; i < e.getValue(); i++) { Policy.Suggester suggester = session.getSuggester(ADDREPLICA) - .hint(Hint.COLL, collName) .hint(Hint.REPLICATYPE, e.getKey()) - .hint(Hint.SHARD, shardName); + .hint(Hint.COLL_SHARD, new Pair<>(collName, shardName)); if (nodesList != null) { for (String nodeName : nodesList) { suggester = suggester.hint(Hint.TARGET_NODE, nodeName); http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/054f445a/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java ---------------------------------------------------------------------- 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 a11eab4..96e2257 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 @@ -27,7 +27,6 @@ import java.util.HashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import com.google.common.collect.ImmutableList; import org.apache.solr.SolrTestCaseJ4; @@ -39,6 +38,7 @@ import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.cloud.ReplicaPosition; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.Pair; import org.apache.solr.common.util.Utils; import org.apache.solr.common.util.ValidatingJsonMap; import org.junit.Test; @@ -469,9 +469,8 @@ public class TestPolicy extends SolrTestCaseJ4 { Policy policy = new Policy(policies); Policy.Suggester suggester = policy.createSession(getClusterDataProvider(nodeValues, clusterState)) .getSuggester(ADDREPLICA) - .hint(Hint.COLL, "newColl") - .hint(Hint.REPLICATYPE, Replica.Type.PULL) - .hint(Hint.SHARD, "shard1"); + .hint(Hint.COLL_SHARD, new Pair("newColl", "shard1")) + .hint(Hint.REPLICATYPE, Replica.Type.PULL); SolrRequest op = suggester.getOperation(); assertNotNull(op); assertEquals(Replica.Type.PULL.name(), op.getParams().get("type")); @@ -479,9 +478,8 @@ public class TestPolicy extends SolrTestCaseJ4 { suggester = suggester.getSession() .getSuggester(ADDREPLICA) - .hint(Hint.COLL, "newColl") - .hint(Hint.REPLICATYPE, Replica.Type.PULL) - .hint(Hint.SHARD, "shard2"); + .hint(Hint.COLL_SHARD, new Pair<>("newColl", "shard2")) + .hint(Hint.REPLICATYPE, Replica.Type.PULL); op = suggester.getOperation(); assertNotNull(op); assertEquals(Replica.Type.PULL.name(), op.getParams().get("type")); @@ -489,9 +487,8 @@ public class TestPolicy extends SolrTestCaseJ4 { suggester = suggester.getSession() .getSuggester(ADDREPLICA) - .hint(Hint.COLL, "newColl") - .hint(Hint.REPLICATYPE, Replica.Type.TLOG) - .hint(Hint.SHARD, "shard1"); + .hint(Hint.COLL_SHARD, new Pair("newColl", "shard1")) + .hint(Hint.REPLICATYPE, Replica.Type.TLOG); op = suggester.getOperation(); assertNotNull(op); assertEquals(Replica.Type.TLOG.name(), op.getParams().get("type")); @@ -499,9 +496,8 @@ public class TestPolicy extends SolrTestCaseJ4 { suggester = suggester.getSession() .getSuggester(ADDREPLICA) - .hint(Hint.COLL, "newColl") - .hint(Hint.REPLICATYPE, Replica.Type.TLOG) - .hint(Hint.SHARD, "shard2"); + .hint(Hint.COLL_SHARD, new Pair("newColl", "shard2")) + .hint(Hint.REPLICATYPE, Replica.Type.TLOG); op = suggester.getOperation(); assertNotNull(op); assertEquals(Replica.Type.TLOG.name(), op.getParams().get("type")); @@ -509,9 +505,8 @@ public class TestPolicy extends SolrTestCaseJ4 { suggester = suggester.getSession() .getSuggester(ADDREPLICA) - .hint(Hint.COLL, "newColl") - .hint(Hint.REPLICATYPE, Replica.Type.TLOG) - .hint(Hint.SHARD, "shard2"); + .hint(Hint.COLL_SHARD, new Pair<>("newColl", "shard2")) + .hint(Hint.REPLICATYPE, Replica.Type.TLOG); op = suggester.getOperation(); assertNull("No node should qualify for this" ,op); @@ -670,10 +665,9 @@ public class TestPolicy extends SolrTestCaseJ4 { Policy policy = new Policy(policies); Policy.Suggester suggester = policy.createSession(getClusterDataProvider(nodeValues, clusterState)) .getSuggester(ADDREPLICA) - .hint(Hint.COLL, "newColl") - .hint(Hint.COLL, "newColl2") .hint(Hint.REPLICATYPE, Replica.Type.PULL) - .hint(Hint.SHARD, "shard1"); + .hint(Hint.COLL_SHARD, new Pair<>("newColl", "shard1")) + .hint(Hint.COLL_SHARD, new Pair<>("newColl2", "shard1")); SolrRequest op; int countOp = 0; int countNewCollOp = 0; @@ -681,10 +675,9 @@ public class TestPolicy extends SolrTestCaseJ4 { while ((op = suggester.getOperation()) != null) { countOp++; suggester = suggester.getSession().getSuggester(ADDREPLICA) - .hint(Hint.COLL, "newColl") - .hint(Hint.COLL, "newColl2") .hint(Hint.REPLICATYPE, Replica.Type.PULL) - .hint(Hint.SHARD, "shard1"); + .hint(Hint.COLL_SHARD, new Pair<>("newColl", "shard1")) + .hint(Hint.COLL_SHARD, new Pair<>("newColl2", "shard1")); assertEquals(Replica.Type.PULL.name(), op.getParams().get("type")); String collection = op.getParams().get("collection"); assertTrue("Collection for replica is not as expected " + collection, collection.equals("newColl") || collection.equals("newColl2")); @@ -701,18 +694,16 @@ public class TestPolicy extends SolrTestCaseJ4 { countNewColl2Op = 0; suggester = suggester.getSession() .getSuggester(ADDREPLICA) - .hint(Hint.COLL, "newColl") - .hint(Hint.COLL, "newColl2") - .hint(Hint.REPLICATYPE, Replica.Type.TLOG) - .hint(Hint.SHARD, "shard2"); + .hint(Hint.COLL_SHARD, new Pair<>("newColl", "shard2")) + .hint(Hint.COLL_SHARD, new Pair<>("newColl2", "shard2")) + .hint(Hint.REPLICATYPE, Replica.Type.TLOG); while ((op = suggester.getOperation()) != null) { countOp++; suggester = suggester.getSession() .getSuggester(ADDREPLICA) - .hint(Hint.COLL, "newColl") - .hint(Hint.COLL, "newColl2") - .hint(Hint.REPLICATYPE, Replica.Type.TLOG) - .hint(Hint.SHARD, "shard2"); + .hint(Hint.COLL_SHARD, new Pair<>("newColl", "shard2")) + .hint(Hint.COLL_SHARD, new Pair<>("newColl2", "shard2")) + .hint(Hint.REPLICATYPE, Replica.Type.TLOG); assertEquals(Replica.Type.TLOG.name(), op.getParams().get("type")); String collection = op.getParams().get("collection"); assertTrue("Collection for replica is not as expected " + collection, collection.equals("newColl") || collection.equals("newColl2")); @@ -816,8 +807,7 @@ public class TestPolicy extends SolrTestCaseJ4 { assertTrue(violations.stream().anyMatch(violation -> (violation.getClause().replica.getOperand() == Operand.LESS_THAN && "node".equals(violation.getClause().tag.getName())))); Policy.Suggester suggester = session.getSuggester(ADDREPLICA) - .hint(Hint.COLL, "gettingstarted") - .hint(Hint.SHARD, "r1"); + .hint(Hint.COLL_SHARD, new Pair<>("gettingstarted","r1")); SolrParams operation = suggester.getOperation().getParams(); assertEquals("node2", operation.get("node")); @@ -934,8 +924,7 @@ public class TestPolicy extends SolrTestCaseJ4 { for (int i = 0; i < 3; i++) { Policy.Suggester suggester = session.getSuggester(ADDREPLICA); SolrRequest op = suggester - .hint(Hint.COLL, "newColl") - .hint(Hint.SHARD, "shard1") + .hint(Hint.COLL_SHARD, new Pair<>("newColl","shard1")) .getOperation(); assertNotNull(op); assertEquals("node3", op.getParams().get("node")); @@ -987,16 +976,14 @@ public class TestPolicy extends SolrTestCaseJ4 { Policy.Session session = policy.createSession(cdp); Policy.Suggester suggester = session.getSuggester(ADDREPLICA); SolrRequest op = suggester - .hint(Hint.COLL, "newColl") - .hint(Hint.SHARD, "shard1") + .hint(Hint.COLL_SHARD, new Pair<>("newColl", "shard1")) .getOperation(); assertNotNull(op); assertEquals("node3", op.getParams().get("node")); suggester = suggester .getSession() .getSuggester(ADDREPLICA) - .hint(Hint.COLL, "newColl") - .hint(Hint.SHARD, "shard1"); + .hint(Hint.COLL_SHARD, new Pair<>("newColl", "shard1")); op = suggester.getOperation(); assertNotNull(op); assertEquals("node3", op.getParams().get("node")); @@ -1004,8 +991,7 @@ public class TestPolicy extends SolrTestCaseJ4 { suggester = suggester .getSession() .getSuggester(ADDREPLICA) - .hint(Hint.COLL, "newColl") - .hint(Hint.SHARD, "shard1"); + .hint(Hint.COLL_SHARD, new Pair<>("newColl", "shard1")); op = suggester.getOperation(); assertNotNull(op); assertEquals("node2", op.getParams().get("node")); @@ -1124,8 +1110,7 @@ public class TestPolicy extends SolrTestCaseJ4 { CollectionAdminRequest.AddReplica op = (CollectionAdminRequest.AddReplica) session .getSuggester(ADDREPLICA) - .hint(Hint.COLL, "newColl") - .hint(Hint.SHARD, "s1").getOperation(); + .hint(Hint.COLL_SHARD, new Pair<>("newColl", "s1")).getOperation(); assertNotNull(op); assertEquals("node2", op.getNode()); }