From fe86ab982d14b02d5fc9842259f9d0ae1a949757 Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Fri, 12 Jan 2018 23:48:30 +1100 Subject: [PATCH] SOLR-11063: Suggesters should accept required freedisk as a hint --- solr/CHANGES.txt | 2 + .../autoscaling/AddReplicaSuggester.java | 3 +- .../autoscaling/MoveReplicaSuggester.java | 3 +- .../solrj/cloud/autoscaling/Policy.java | 10 ++--- .../solrj/cloud/autoscaling/Suggester.java | 36 ++++++++++++++-- .../solrj/cloud/autoscaling/Suggestion.java | 1 + .../solrj/cloud/autoscaling/TestPolicy.java | 42 +++++++++++++++++++ 7 files changed, 84 insertions(+), 13 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 4ba50b95256..799637df404 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -82,6 +82,8 @@ New Features * SOLR-11062: new tag "diskType" in autoscaling policy (noble) +* SOLR-11063: Suggesters should accept required freedisk as a hint (noble) + Bug Fixes ---------------------- 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 3b997bfa402..3f96f3e5b10 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 @@ -49,8 +49,7 @@ class AddReplicaSuggester extends Suggester { Integer targetNodeIndex = null; for (int i = getMatrix().size() - 1; i >= 0; i--) { Row row = getMatrix().get(i); - if (!row.isLive) continue; - if (!isAllowed(row.node, Hint.TARGET_NODE)) continue; + if (!isNodeSuitable(row)) continue; Row tmpRow = row.addReplica(shard.first(), shard.second(), type); List errs = testChangedMatrix(strict, getModifiedMatrix(getMatrix(), tmpRow, i)); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/MoveReplicaSuggester.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/MoveReplicaSuggester.java index b68b0b5a553..25e75adad9b 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/MoveReplicaSuggester.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/MoveReplicaSuggester.java @@ -57,8 +57,7 @@ public class MoveReplicaSuggester extends Suggester { for (int j = getMatrix().size() - 1; j >= stopAt; j--) { if (j == i) continue; Row targetRow = getMatrix().get(j); - if(!targetRow.isLive) continue; - if (!isAllowed(targetRow.node, Hint.TARGET_NODE)) continue; + if (!isNodeSuitable(targetRow)) continue; targetRow = targetRow.addReplica(coll, shard, replicaInfo.getType()); List errs = testChangedMatrix(strict, getModifiedMatrix(getModifiedMatrix(getMatrix(), srcTmpRow, i), targetRow, j)); if (!containsNewErrors(errs) && isLessSerious(errs, leastSeriousViolation) && 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 2c4bf43ae62..15c63b8a788 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 @@ -39,6 +39,7 @@ import org.apache.solr.client.solrj.impl.ClusterStateProvider; import org.apache.solr.common.IteratorWriter; import org.apache.solr.common.MapWriter; import org.apache.solr.common.cloud.ClusterState; +import org.apache.solr.common.cloud.rule.ImplicitSnitch; import org.apache.solr.common.params.CollectionParams.CollectionAction; import org.apache.solr.common.util.StrUtils; import org.apache.solr.common.util.Utils; @@ -94,12 +95,9 @@ public class Policy implements MapWriter { } this.clusterPreferences = Collections.unmodifiableList(initialClusterPreferences); final SortedSet paramsOfInterest = new TreeSet<>(); - for (Preference preference : clusterPreferences) { - if (paramsOfInterest.contains(preference.name.name())) { - throw new RuntimeException(preference.name + " is repeated"); - } - paramsOfInterest.add(preference.name.toString()); - } + paramsOfInterest.add(ImplicitSnitch.DISK);//always get freedisk anyway. + paramsOfInterest.add(ImplicitSnitch.CORES);//always get cores anyway. + clusterPreferences.forEach(preference -> paramsOfInterest.add(preference.name.toString())); List newParams = new ArrayList<>(paramsOfInterest); clusterPolicy = ((List>) jsonMap.getOrDefault(CLUSTER_POLICY, emptyList())).stream() .map(Clause::new) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggester.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggester.java index 15d124dd93c..57007681ad6 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggester.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggester.java @@ -29,14 +29,18 @@ import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.function.Consumer; +import java.util.function.Predicate; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.impl.ClusterStateProvider; import org.apache.solr.common.MapWriter; import org.apache.solr.common.cloud.Replica; +import org.apache.solr.common.cloud.rule.ImplicitSnitch; import org.apache.solr.common.util.Pair; import org.apache.solr.common.util.Utils; +import static org.apache.solr.client.solrj.cloud.autoscaling.Suggestion.ConditionType.FREEDISK; + /* A suggester is capable of suggesting a collection operation * given a particular session. Before it suggests a new operation, * it ensures that , @@ -76,6 +80,13 @@ public abstract class Suggester implements MapWriter { return this; } + protected boolean isNodeSuitable(Row row) { + if (!row.isLive) return false; + if (!isAllowed(row.node, Hint.TARGET_NODE)) return false; + if (!isAllowed(row.getVal(ImplicitSnitch.DISK), Hint.MINFREEDISK)) return false; + return true; + } + abstract SolrRequest init(); @@ -227,11 +238,12 @@ public abstract class Suggester implements MapWriter { protected boolean isAllowed(Object v, Hint hint) { Object hintVal = hints.get(hint); + if (hintVal == null) return true; if (hint.multiValued) { Set set = (Set) hintVal; return set == null || set.contains(v); } else { - return hintVal == null || Objects.equals(v, hintVal); + return hintVal == null || hint.valueValidator.test(new Pair<>(hintVal, v)); } } @@ -258,10 +270,20 @@ public abstract class Suggester implements MapWriter { if (!(o instanceof Replica.Type)) { throw new RuntimeException("REPLICATYPE hint must use a ReplicaType"); } + }), + + MINFREEDISK(false, o -> { + if (!(o instanceof Number)) throw new RuntimeException("MINFREEDISK hint must be a number"); + }, hintValVsActual -> { + Double hintFreediskInGb = (Double) FREEDISK.validate(null, hintValVsActual.first(), false); + Double actualFreediskInGb = (Double) FREEDISK.validate(null, hintValVsActual.second(), false); + if(actualFreediskInGb == null) return false; + return actualFreediskInGb > hintFreediskInGb; }); public final boolean multiValued; public final Consumer validator; + public final Predicate> valueValidator; Hint(boolean multiValued) { this(multiValued, v -> { @@ -273,12 +295,20 @@ public abstract class Suggester implements MapWriter { } Hint(boolean multiValued, Consumer c) { - this.multiValued = multiValued; - this.validator = c; + this(multiValued, c, equalsPredicate); } + Hint(boolean multiValued, Consumer c, Predicate> testval) { + this.multiValued = multiValued; + this.validator = c; + this.valueValidator = testval; + } + + } + static Predicate> equalsPredicate = valPair -> Objects.equals(valPair.first(), valPair.second()); + @Override public String toString() { return jsonStr(); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggestion.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggestion.java index 51b5046ef93..b29fb382424 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggestion.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggestion.java @@ -279,6 +279,7 @@ public class Suggestion { } public Object validate(String name, Object val, boolean isRuleVal) { + if (name == null) name = this.tagName; if (type == Double.class) { Double num = Clause.parseDouble(name, val); if (isRuleVal) { 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 e89f10e73f9..3a5caf94624 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 @@ -1620,4 +1620,46 @@ public class TestPolicy extends SolrTestCaseJ4 { } } + public void testDiskSpaceHint() { + + String dataproviderdata = "{" + + " liveNodes:[" + + " '127.0.0.1:51078_solr'," + + " '127.0.0.1:51147_solr']," + + " replicaInfo:{" + + " '127.0.0.1:51147_solr':{}," + + " '127.0.0.1:51078_solr':{testNodeAdded:{shard1:[" + + " { core_node3 : { type : NRT}}," + + " { core_node4 : { type : NRT}}]}}}," + + " nodeValues:{" + + " '127.0.0.1:51147_solr':{" + + " node:'127.0.0.1:51147_solr'," + + " cores:0," + + " freedisk : 100}," + + " '127.0.0.1:51078_solr':{" + + " node:'127.0.0.1:51078_solr'," + + " cores:2," + + " freedisk:200}}}"; + + String autoScalingjson = "cluster-preferences:[" + + " {minimize : cores}]" + + " cluster-policy:[{cores:'<10',node:'#ANY'}," + + " {replica:'<2', shard:'#EACH',node:'#ANY'}," + + " { nodeRole:overseer,replica:0}]}"; + Policy policy = new Policy((Map) Utils.fromJSONString(autoScalingjson)); + Policy.Session session = policy.createSession(cloudManagerWithData(dataproviderdata)); + Suggester suggester = session.getSuggester(CollectionParams.CollectionAction.ADDREPLICA) + .hint(Hint.COLL_SHARD, new Pair<>("coll1", "shard1")) + .hint(Hint.MINFREEDISK, 150); + CollectionAdminRequest.AddReplica op = (CollectionAdminRequest.AddReplica) suggester.getSuggestion(); + + assertEquals("127.0.0.1:51078_solr" , op.getNode()); + + suggester = session.getSuggester(CollectionParams.CollectionAction.ADDREPLICA) + .hint(Hint.COLL_SHARD, new Pair<>("coll1", "shard1")); + op = (CollectionAdminRequest.AddReplica) suggester.getSuggestion(); + + assertEquals("127.0.0.1:51147_solr" , op.getNode()); + } + }