lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From no...@apache.org
Subject [1/2] lucene-solr:master: Revert "OLR-11986: Allow percentage in freedisk attribute in autoscaling policy rules"
Date Mon, 16 Jul 2018 07:36:10 GMT
Repository: lucene-solr
Updated Branches:
  refs/heads/master 5b9f4f3ec -> 11b22b441


Revert "OLR-11986: Allow percentage in freedisk attribute in autoscaling policy rules"

This reverts commit 5b9f4f3


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/09526a56
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/09526a56
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/09526a56

Branch: refs/heads/master
Commit: 09526a560cbd15e2a7c46e3140f649518074345c
Parents: 5b9f4f3
Author: Noble Paul <noble@apache.org>
Authored: Mon Jul 16 17:33:51 2018 +1000
Committer: Noble Paul <noble@apache.org>
Committed: Mon Jul 16 17:33:51 2018 +1000

----------------------------------------------------------------------
 solr/CHANGES.txt                                |   2 -
 .../solr/cloud/autoscaling/TestPolicyCloud.java |   9 +-
 .../cloud/autoscaling/sim/TestPolicyCloud.java  |   2 +-
 .../client/solrj/cloud/autoscaling/Clause.java  | 101 ++----
 .../client/solrj/cloud/autoscaling/Policy.java  |  16 +-
 .../solrj/cloud/autoscaling/PolicyHelper.java   |   2 +-
 .../client/solrj/cloud/autoscaling/Row.java     |   2 +-
 .../solrj/cloud/autoscaling/Suggestion.java     | 330 +++++--------------
 .../solrj/cloud/autoscaling/Violation.java      |   2 +-
 .../solrj/impl/SolrClientNodeStateProvider.java |  33 +-
 .../java/org/apache/solr/common/MapWriter.java  |   5 -
 .../solrj/cloud/autoscaling/TestPolicy.java     | 117 ++-----
 12 files changed, 153 insertions(+), 468 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/09526a56/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index d8823db..b7df7b6 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -110,8 +110,6 @@ New Features
 * SOLR-12441: New NestedUpdateProcessorFactory (URP) to populate special fields _nest_parent_ and _nest_path_ of nested
   (child) documents.  It will generate a uniqueKey of nested docs if they were blank too. (Moshe Bla, David Smiley)
 
-* SOLR-11986: Allow percentage in freedisk attribute in autoscaling policy rules (noble)
-
 Bug Fixes
 ----------------------
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/09526a56/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java
index a25a67a..19324e7 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java
@@ -119,15 +119,8 @@ public class TestPolicyCloud extends SolrCloudTestCase {
 
     Policy.Session session = config.getPolicy().createSession(cloudManager);
 
-    for (Row row : session.getSortedNodes()) {
-      Object val = row.getVal(Suggestion.ConditionType.TOTALDISK.tagName, null);
-      log.info("node: {} , totaldisk : {}, freedisk : {}", row.node, val, row.getVal("freedisk",null));
-      assertTrue(val != null);
-
-    }
-
     count .set(0);
-    for (Row row : session.getSortedNodes()) {
+    for (Row row : session.getSorted()) {
       row.collectionVsShardVsReplicas.forEach((c, shardVsReplicas) -> shardVsReplicas.forEach((s, replicaInfos) -> {
         for (ReplicaInfo replicaInfo : replicaInfos) {
           if (replicaInfo.getVariables().containsKey(Suggestion.ConditionType.CORE_IDX.tagName)) count.incrementAndGet();

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/09526a56/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestPolicyCloud.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestPolicyCloud.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestPolicyCloud.java
index fad637d..b6368d6 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestPolicyCloud.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestPolicyCloud.java
@@ -92,7 +92,7 @@ public class TestPolicyCloud extends SimSolrCloudTestCase {
     Policy.Session session = config.getPolicy().createSession(cluster);
 
     AtomicInteger count = new AtomicInteger(0);
-    for (Row row : session.getSortedNodes()) {
+    for (Row row : session.getSorted()) {
       row.collectionVsShardVsReplicas.forEach((c, shardVsReplicas) -> shardVsReplicas.forEach((s, replicaInfos) -> {
         for (ReplicaInfo replicaInfo : replicaInfos) {
           if (replicaInfo.getVariables().containsKey(Suggestion.ConditionType.CORE_IDX.tagName)) count.incrementAndGet();

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/09526a56/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java
index 06dc7a0..60ebf9b 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java
@@ -30,7 +30,6 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.function.Function;
 
-import org.apache.solr.client.solrj.cloud.autoscaling.Suggestion.ConditionType;
 import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.util.StrUtils;
@@ -117,10 +116,6 @@ public class Clause implements MapWriter, Comparable<Clause> {
     }
   }
 
-  public static Clause create(String json) {
-    return create((Map<String, Object>) Utils.fromJSONString(json));
-  }
-
   public static Clause create(Map<String, Object> m) {
     Clause clause = new Clause(m);
     return clause.hasComputedValue() ?
@@ -155,7 +150,7 @@ public class Clause implements MapWriter, Comparable<Clause> {
 
   private Condition evaluateValue(Condition condition, Function<Condition, Object> computedValueEvaluator) {
     if (condition == null) return null;
-    if (condition.computedType == null) return condition;
+    if (condition.computationType == null) return condition;
     Object val = computedValueEvaluator.apply(condition);
     val = condition.op.readRuleValue(new Condition(condition.name, val, condition.op, null, null));
     return new Condition(condition.name, val, condition.op, null, this);
@@ -187,9 +182,9 @@ public class Clause implements MapWriter, Comparable<Clause> {
   }
 
   private boolean hasComputedValue() {
-    if (replica != null && replica.computedType != null) return true;
-    if (tag != null && tag.computedType != null) return true;
-    if (globalTag != null && globalTag.computedType != null) return true;
+    if (replica != null && replica.computationType != null) return true;
+    if (tag != null && tag.computationType != null) return true;
+    if (globalTag != null && globalTag.computationType != null) return true;
     return false;
 
   }
@@ -238,12 +233,9 @@ public class Clause implements MapWriter, Comparable<Clause> {
 
   Condition parse(String s, Map m) {
     Object expectedVal = null;
-    ComputedType computedType = null;
+    ComputationType computationType = null;
     Object val = m.get(s);
-    ConditionType varType = Suggestion.getTagType(s);
-    if (varType.isHidden) {
-      throw new IllegalArgumentException(StrUtils.formatString("''{0}'' is not allowed in a policy rule :  ''{1}''  ", varType.tagName, Utils.toJSONString(m)));
-    }
+    Suggestion.ConditionType varType = Suggestion.getTagType(s);
     try {
       String conditionName = s.trim();
       Operand operand = null;
@@ -252,39 +244,32 @@ public class Clause implements MapWriter, Comparable<Clause> {
         expectedVal = Policy.ANY;
       } else if (val instanceof String) {
         String strVal = ((String) val).trim();
-        val = strVal;
         if (Policy.ANY.equals(strVal) || Policy.EACH.equals(strVal)) operand = WILDCARD;
         else if (strVal.startsWith(NOT_EQUAL.operand)) operand = NOT_EQUAL;
         else if (strVal.startsWith(GREATER_THAN.operand)) operand = GREATER_THAN;
         else if (strVal.startsWith(LESS_THAN.operand)) operand = LESS_THAN;
         else operand = Operand.EQUAL;
         strVal = strVal.substring(Operand.EQUAL == operand || WILDCARD == operand ? 0 : 1);
-        for (ComputedType t : ComputedType.values()) {
+        for (ComputationType t : ComputationType.values()) {
           String changedVal = t.match(strVal);
           if (changedVal != null) {
-            computedType = t;
+            computationType = t;
             strVal = changedVal;
-            if (varType == null || !varType.supportedComputedTypes.contains(computedType)) {
-              throw new IllegalArgumentException(StrUtils.formatString("''{0}'' is not allowed for variable :  ''{1}'' , in clause : ''{2}'' ",
+            if (varType == null || !varType.supportComputed(computationType, this)) {
+              throw new IllegalArgumentException(StrUtils.formatString("''{0}'' is not allowed for variable :  ''{1}'' , in condition : ''{2}'' ",
                   t, conditionName, Utils.toJSONString(m)));
             }
           }
         }
-        if (computedType == null && ((String) val).charAt(0) == '#' && !varType.wildCards.contains(val)) {
-          throw new IllegalArgumentException(StrUtils.formatString("''{0}'' is not an allowed value for ''{1}'' , in clause : ''{2}'' . Supported value is : {3}",
-              val, conditionName, Utils.toJSONString(m), varType.wildCards));
-
-
-        }
-        operand = varType == null ? operand : varType.getOperand(operand, strVal, computedType);
-        expectedVal = validate(s, new Condition(s, strVal, operand, computedType, null), true);
+        operand = varType == null ? operand : varType.getOperand(operand, strVal, computationType);
+        expectedVal = validate(s, new Condition(s, strVal, operand, computationType, null), true);
 
       } else if (val instanceof Number) {
         operand = Operand.EQUAL;
         operand = varType.getOperand(operand, val, null);
         expectedVal = validate(s, new Condition(s, val, operand, null, null), true);
       }
-      return new Condition(conditionName, expectedVal, operand, computedType, this);
+      return new Condition(conditionName, expectedVal, operand, computationType, this);
 
     } catch (IllegalArgumentException iae) {
       throw iae;
@@ -305,17 +290,16 @@ public class Clause implements MapWriter, Comparable<Clause> {
           computedValueEvaluator.shardName = shardVsCount.getKey();
           if (!shard.isPass(computedValueEvaluator.shardName)) continue;
           for (Map.Entry<String, ReplicaCount> counts : shardVsCount.getValue().entrySet()) {
-            if(tag.varType.isPerNodeValue) computedValueEvaluator.node = counts.getKey();
             SealedClause sealedClause = getSealedClause(computedValueEvaluator);
             ReplicaCount replicas = counts.getValue();
             if (!sealedClause.replica.isPass(replicas)) {
               Violation violation = new Violation(sealedClause,
                   computedValueEvaluator.collName,
                   computedValueEvaluator.shardName,
-                  tag.varType.isPerNodeValue ? computedValueEvaluator.node : null,
+                  tag.name.equals("node") ? counts.getKey() : null,
                   counts.getValue(),
                   sealedClause.getReplica().delta(replicas),
-                  tag.varType.isPerNodeValue? null:  counts.getKey());
+                  counts.getKey());
               tag.varType.addViolatingReplicas(ctx.reset(counts.getKey(), replicas, violation));
             }
           }
@@ -325,7 +309,7 @@ public class Clause implements MapWriter, Comparable<Clause> {
       for (Row r : session.matrix) {
         SealedClause sealedClause = getSealedClause(computedValueEvaluator);
         if (!sealedClause.getGlobalTag().isPass(r)) {
-          ConditionType.CORES.addViolatingReplicas(ctx.reset(null, null,
+          Suggestion.ConditionType.CORES.addViolatingReplicas(ctx.reset(null, null,
               new Violation(sealedClause, null, null, r.node, r.getVal(sealedClause.globalTag.name), sealedClause.globalTag.delta(r.getVal(globalTag.name)), null)));
         }
       }
@@ -338,7 +322,6 @@ public class Clause implements MapWriter, Comparable<Clause> {
                                                                                    ComputedValueEvaluator computedValueEvaluator) {
     Map<String, Map<String, Map<String, ReplicaCount>>> collVsShardVsTagVsCount = new HashMap<>();
     for (Row row : allRows) {
-      computedValueEvaluator.node = row.node;
       for (Map.Entry<String, Map<String, List<ReplicaInfo>>> colls : row.collectionVsShardVsReplicas.entrySet()) {
         String collectionName = colls.getKey();
         if (!collection.isPass(collectionName)) continue;
@@ -352,19 +335,10 @@ public class Clause implements MapWriter, Comparable<Clause> {
           computedValueEvaluator.collName = collectionName;
           computedValueEvaluator.shardName = shardName;
           SealedClause sealedClause = getSealedClause(computedValueEvaluator);
-          Condition t = sealedClause.getTag();
-          if(t.varType.isPerNodeValue){
-            boolean pass = t.getOperand().match(t.val, tagVal) == TestStatus.PASS;
-            tagVsCount.computeIfAbsent(row.node, s -> new ReplicaCount());
-            if(pass) {
-              tagVsCount.get(row.node).increment(shards.getValue());
-            }
-          } else {
-            boolean pass = sealedClause.getTag().isPass(tagVal);
-            tagVsCount.computeIfAbsent(pass ? String.valueOf(tagVal) : "", s -> new ReplicaCount());
-            if (pass) {
-              tagVsCount.get(String.valueOf(tagVal)).increment(shards.getValue());
-            }
+          boolean pass = sealedClause.getTag().isPass(tagVal);
+          tagVsCount.computeIfAbsent(pass ? String.valueOf(tagVal) : "", s -> new ReplicaCount());
+          if (pass) {
+            tagVsCount.get(String.valueOf(tagVal)).increment(shards.getValue());
           }
         }
       }
@@ -372,8 +346,7 @@ public class Clause implements MapWriter, Comparable<Clause> {
     return collVsShardVsTagVsCount;
   }
 
-  enum ComputedType {
-    NULL(),
+  enum ComputationType {
     EQUAL() {
       @Override
       public String wrap(String value) {
@@ -415,12 +388,6 @@ public class Clause implements MapWriter, Comparable<Clause> {
       }
 
       @Override
-      public Object compute(Object val, Condition c) {
-        if (val == null || Clause.parseDouble(c.name, val) == 0) return 0d;
-        return Clause.parseDouble(c.name, val) * Clause.parseDouble(c.name, c.val).doubleValue() / 100;
-      }
-
-      @Override
       public String toString() {
         return "%";
       }
@@ -435,34 +402,29 @@ public class Clause implements MapWriter, Comparable<Clause> {
     public String wrap(String value) {
       return value;
     }
-
-    public Object compute(Object val, Condition c) {
-      return val;
-    }
-
   }
 
   public static class Condition implements MapWriter {
     final String name;
     final Object val;
-    final ConditionType varType;
-    final ComputedType computedType;
+    final Suggestion.ConditionType varType;
+    final ComputationType computationType;
     final Operand op;
     private Clause clause;
 
-    Condition(String name, Object val, Operand op, ComputedType computedType, Clause parent) {
+    Condition(String name, Object val, Operand op, ComputationType computationType, Clause parent) {
       this.name = name;
       this.val = val;
       this.op = op;
       varType = Suggestion.getTagType(name);
-      this.computedType = computedType;
+      this.computationType = computationType;
       this.clause = parent;
     }
 
     @Override
     public void writeMap(EntryWriter ew) throws IOException {
       String value = op.wrap(val);
-      if (computedType != null) value = computedType.wrap(value);
+      if (computationType != null) value = computationType.wrap(value);
       ew.put(name, value);
     }
 
@@ -476,12 +438,12 @@ public class Clause implements MapWriter, Comparable<Clause> {
     }
 
     boolean isPass(Object inputVal) {
-      if (computedType != null) {
+      if (computationType != null) {
         throw new IllegalStateException("This is supposed to be called only from a Condition with no computed value or a SealedCondition");
 
       }
       if (inputVal instanceof ReplicaCount) inputVal = ((ReplicaCount) inputVal).getVal(getClause().type);
-      if (varType == ConditionType.LAZY) { // we don't know the type
+      if (varType == Suggestion.ConditionType.LAZY) { // we don't know the type
         return op.match(parseString(val), parseString(inputVal)) == PASS;
       } else {
         return op.match(val, validate(name, inputVal, false)) == PASS;
@@ -552,15 +514,14 @@ public class Clause implements MapWriter, Comparable<Clause> {
     final Policy.Session session;
     String collName = null;
     String shardName = null;
-    String node = null;
 
     public ComputedValueEvaluator(Policy.Session session) {
       this.session = session;
     }
 
     @Override
-    public Object apply(Condition computedCondition) {
-      return computedCondition.varType.computeValue(session, computedCondition, collName, shardName, node);
+    public Object apply(Condition computedValue) {
+      return computedValue.varType.computeValue(session, computedValue, collName, shardName);
     }
 
   }
@@ -573,7 +534,7 @@ public class Clause implements MapWriter, Comparable<Clause> {
    */
   public static Object  validate(String name, Object val, boolean isRuleVal) {
     if (val == null) return null;
-    ConditionType info = Suggestion.getTagType(name);
+    Suggestion.ConditionType info = Suggestion.getTagType(name);
     if (info == null) throw new RuntimeException("Unknown type :" + name);
     return info.validate(name, val, isRuleVal);
   }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/09526a56/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 9addc67..f7b8c03 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
@@ -38,7 +38,6 @@ import java.util.stream.Collectors;
 
 import org.apache.solr.client.solrj.cloud.NodeStateProvider;
 import org.apache.solr.client.solrj.cloud.SolrCloudManager;
-import org.apache.solr.client.solrj.cloud.autoscaling.Suggestion.ConditionType;
 import org.apache.solr.client.solrj.impl.ClusterStateProvider;
 import org.apache.solr.common.IteratorWriter;
 import org.apache.solr.common.MapWriter;
@@ -88,7 +87,7 @@ public class Policy implements MapWriter {
   final Map<String, List<Clause>> policies;
   final List<Clause> clusterPolicy;
   final List<Preference> clusterPreferences;
-  final List<Pair<String, ConditionType>> params;
+  final List<Pair<String, Suggestion.ConditionType>> params;
   final List<String> perReplicaAttributes;
 
   public Policy() {
@@ -120,15 +119,6 @@ public class Policy implements MapWriter {
         })
         .collect(collectingAndThen(toList(), Collections::unmodifiableList));
 
-    for (String newParam : new ArrayList<>(newParams)) {
-      ConditionType t = Suggestion.getTagType(newParam);
-      if(t != null && !t.associatedPerNodeValues.isEmpty()){
-        for (String s : t.associatedPerNodeValues) {
-          if(!newParams.contains(s)) newParams.add(s);
-        }
-      }
-    }
-
     this.policies = Collections.unmodifiableMap(
         policiesFromMap((Map<String, List<Map<String, Object>>>) jsonMap.getOrDefault(POLICIES, emptyMap()), newParams));
     this.params = Collections.unmodifiableList(newParams.stream()
@@ -139,7 +129,7 @@ public class Policy implements MapWriter {
 
   private List<String> readPerReplicaAttrs() {
     return this.params.stream()
-        .map(s -> s.second().perReplicaValue)
+        .map(s -> Suggestion.tagVsPerReplicaVal.get(s.first()))
         .filter(Objects::nonNull)
         .collect(Collectors.toList());
   }
@@ -581,7 +571,7 @@ public class Policy implements MapWriter {
       return Utils.toJSONString(toMap(new LinkedHashMap<>()));
     }
 
-    public List<Row> getSortedNodes() {
+    public List<Row> getSorted() {
       return Collections.unmodifiableList(matrix);
     }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/09526a56/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 a965bf8..7785fda 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
@@ -182,7 +182,7 @@ public class PolicyHelper {
 
   public static MapWriter getDiagnostics(Policy policy, SolrCloudManager cloudManager) {
     Policy.Session session = policy.createSession(cloudManager);
-    List<Row> sorted = session.getSortedNodes();
+    List<Row> sorted = session.getSorted();
     List<Violation> violations = session.getViolations();
 
     List<Preference> clusterPreferences = policy.getClusterPreferences();

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/09526a56/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Row.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Row.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Row.java
index 1adfdbf..f1799c4 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Row.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Row.java
@@ -96,7 +96,7 @@ public class Row implements MapWriter {
     return null;
   }
 
-  public Object getVal(String name, Object def) {
+  Object getVal(String name, Object def) {
     for (Cell cell : cells)
       if (cell.name.equals(name)) {
         return cell.val == null ? def : cell.val;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/09526a56/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggestion.java
----------------------------------------------------------------------
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 97b26fc..f5e25cc 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
@@ -17,12 +17,9 @@
 
 package org.apache.solr.client.solrj.cloud.autoscaling;
 
-import java.lang.annotation.ElementType;
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
-import java.lang.annotation.Target;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -32,16 +29,15 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Function;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.V2RequestSupport;
-import org.apache.solr.client.solrj.cloud.autoscaling.Clause.ComputedType;
 import org.apache.solr.client.solrj.cloud.autoscaling.Violation.ReplicaInfoAndErr;
 import org.apache.solr.common.cloud.rule.ImplicitSnitch;
 import org.apache.solr.common.util.Pair;
 import org.apache.solr.common.util.StrUtils;
 
-import static java.util.Collections.emptySet;
 import static java.util.Collections.unmodifiableSet;
 import static org.apache.solr.client.solrj.cloud.autoscaling.Policy.ANY;
 import static org.apache.solr.common.params.CollectionParams.CollectionAction.MOVEREPLICA;
@@ -50,41 +46,10 @@ public class Suggestion {
   public static final String coreidxsize = "INDEX.sizeInGB";
 
   static final Map<String, ConditionType> validatetypes = new HashMap<>();
-  private static final String NULL = "";
-
-  @Target(ElementType.FIELD)
-  @Retention(RetentionPolicy.RUNTIME)
-  @interface Meta {
-    String name();
-
-    Class type();
-
-    String[] associatedPerNodeValue() default NULL;
-
-    String associatedPerReplicaValue() default NULL;
-
-    String[] enumVals() default NULL;
-
-    String[] wildCards() default NULL;
-
-    boolean isNodeSpecificVal() default false;
-
-    boolean isHidden() default false;
-
-    boolean isAdditive() default true;
-
-    double min() default -1d;
-
-    double max() default -1d;
-
-    String metricsKey() default NULL;
-
-    ComputedType[] computedValues() default ComputedType.NULL;
-  }
 
   public static ConditionType getTagType(String name) {
     ConditionType info = validatetypes.get(name);
-    if (info == null && name.startsWith(ImplicitSnitch.SYSPROP)) info = ConditionType.STRING;
+    if (info == null && name.startsWith(ImplicitSnitch.SYSPROP)) info = ConditionType.LAZY;
     if (info == null && name.startsWith(Clause.METRICS_PREFIX)) info = ConditionType.LAZY;
     return info;
   }
@@ -92,7 +57,7 @@ public class Suggestion {
   private static Object getOperandAdjustedValue(Object val, Object original) {
     if (original instanceof Clause.Condition) {
       Clause.Condition condition = (Clause.Condition) original;
-      if (condition.computedType == null && isIntegerEquivalent(val)) {
+      if (condition.computationType == null && isIntegerEquivalent(val)) {
         if (condition.op == Operand.LESS_THAN) {
           //replica : '<3'
           val = val instanceof Long ?
@@ -154,32 +119,26 @@ public class Suggestion {
   }
 
 
+  public static final Map<String, String> tagVsPerReplicaVal = Stream.of(ConditionType.values())
+      .filter(tag -> tag.perReplicaValue != null)
+      .collect(Collectors.toMap(tag -> tag.tagName, tag -> tag.perReplicaValue));
+
   /**
    * Type details of each variable in policies
    */
   public enum ConditionType {
 
-    @Meta(name = "collection",
-        type = String.class)
-    COLL(),
-    @Meta(
-        name = "shard",
-        type = String.class,
-        wildCards = {Policy.EACH, Policy.ANY})
-    SHARD(),
-
-    @Meta(name = "replica",
-        type = Double.class,
-        min = 0, max = -1,
-        computedValues = {ComputedType.EQUAL, ComputedType.PERCENT})
-    REPLICA() {
+    COLL("collection", String.class, null, null, null),
+    SHARD("shard", String.class, null, null, null),
+    REPLICA("replica", Double.class, null, 0L, null) {
       @Override
       public Object validate(String name, Object val, boolean isRuleVal) {
         return getOperandAdjustedValue(super.validate(name, val, isRuleVal), val);
       }
 
       @Override
-      public Operand getOperand(Operand expected, Object strVal, ComputedType computedType) {
+      public Operand getOperand(Operand expected, Object strVal, Clause.ComputationType computationType) {
+//        if (computationType == Clause.ComputationType.EQUAL) return expected;
         if (strVal instanceof String) {
           String s = ((String) strVal).trim();
           int hyphenIdx = s.indexOf('-');
@@ -193,18 +152,24 @@ public class Suggestion {
 
         }
 
-        if (expected == Operand.EQUAL && (computedType != null || !isIntegerEquivalent(strVal))) {
+        if (expected == Operand.EQUAL && (computationType != null || !isIntegerEquivalent(strVal))) {
           return Operand.RANGE_EQUAL;
         }
-        if (expected == Operand.NOT_EQUAL && (computedType != null || !isIntegerEquivalent(strVal)))
+        if (expected == Operand.NOT_EQUAL && (computationType != null || !isIntegerEquivalent(strVal)))
           return Operand.RANGE_NOT_EQUAL;
 
         return expected;
       }
 
       @Override
+      public boolean supportComputed(Clause.ComputationType computedType, Clause clause) {
+        if (computedType == Clause.ComputationType.PERCENT || computedType == Clause.ComputationType.EQUAL) return true;
+        return false;
+      }
+
+      @Override
       public String postValidate(Clause.Condition condition) {
-        if (condition.computedType == ComputedType.EQUAL) {
+        if (condition.computationType == Clause.ComputationType.EQUAL) {
           if (condition.getClause().tag != null &&
               condition.getClause().tag.varType == NODE &&
               condition.getClause().tag.op == Operand.WILDCARD) {
@@ -217,53 +182,27 @@ public class Suggestion {
       }
 
       @Override
-      public Object computeValue(Policy.Session session, Clause.Condition cv, String collection, String shard, String node) {
-        if (cv.computedType == ComputedType.EQUAL) {
+      public Object computeValue(Policy.Session session, Clause.Condition cv, String collection, String shard) {
+        if (cv.computationType == Clause.ComputationType.EQUAL) {
           int relevantReplicasCount = getRelevantReplicasCount(session, cv, collection, shard);
           if (relevantReplicasCount == 0) return 0;
           return (double) session.matrix.size() / (double) relevantReplicasCount;
-        } else if (cv.computedType == ComputedType.PERCENT) {
-          return ComputedType.PERCENT.compute(getRelevantReplicasCount(session, cv, collection, shard), cv);
+        } else if (cv.computationType == Clause.ComputationType.PERCENT) {
+          int relevantReplicasCount = getRelevantReplicasCount(session, cv, collection, shard);
+          if (relevantReplicasCount == 0) return 0;
+          return (double) relevantReplicasCount * Clause.parseDouble(cv.name, cv.val).doubleValue() / 100;
         } else {
-          throw new IllegalArgumentException("Unsupported type " + cv.computedType);
+          throw new IllegalArgumentException("Unsupported type " + cv.computationType);
 
         }
       }
     },
-    @Meta(name = ImplicitSnitch.PORT,
-        type = Long.class,
-        min = 1,
-        max = 65535)
-    PORT(),
-    @Meta(name = "ip_1",
-        type = Long.class,
-        min = 0,
-        max = 255)
-    IP_1(),
-    @Meta(name = "ip_2",
-        type = Long.class,
-        min = 0,
-        max = 255)
-    IP_2(),
-    @Meta(name = "ip_3",
-        type = Long.class,
-        min = 0,
-        max = 255)
-    IP_3(),
-    @Meta(name = "ip_4",
-        type = Long.class,
-        min = 0,
-        max = 255)
-    IP_4(),
-
-    @Meta(name = ImplicitSnitch.DISK,
-        type = Double.class,
-        min = 0,
-        isNodeSpecificVal = true,
-        associatedPerReplicaValue = coreidxsize,
-        associatedPerNodeValue = "totaldisk",
-        computedValues = ComputedType.PERCENT)
-    FREEDISK() {
+    PORT(ImplicitSnitch.PORT, Long.class, null, 1L, 65535L),
+    IP_1("ip_1", Long.class, null, 0L, 255L),
+    IP_2("ip_2", Long.class, null, 0L, 255L),
+    IP_3("ip_3", Long.class, null, 0L, 255L),
+    IP_4("ip_4", Long.class, null, 0L, 255L),
+    FREEDISK(ImplicitSnitch.DISK, Double.class, null, 0d, Double.MAX_VALUE, coreidxsize, Boolean.TRUE,null) {
       @Override
       public Object convertVal(Object val) {
         Number value = (Number) super.validate(ImplicitSnitch.DISK, val, false);
@@ -274,18 +213,6 @@ public class Suggestion {
       }
 
       @Override
-      public Object computeValue(Policy.Session session, Clause.Condition condition, String collection, String shard, String node) {
-        if (condition.computedType == ComputedType.PERCENT) {
-          Row r = session.getNode(node);
-          if (r == null) return 0d;
-          return ComputedType.PERCENT.compute(r.getVal(TOTALDISK.tagName), condition);
-        }
-        throw new IllegalArgumentException("Unsupported type " + condition.computedType);
-      }
-
-
-
-      @Override
       public int compareViolation(Violation v1, Violation v2) {
         //TODO use tolerance compare
         return Double.compare(
@@ -361,38 +288,14 @@ public class Suggestion {
         cell.val = currFreeDisk + idxSize;
       }
     },
-
-    @Meta(name = "totaldisk",
-        type = Double.class,
-        isHidden = true)
-    TOTALDISK() {
-      @Override
-      public Object convertVal(Object val) {
-        return FREEDISK.convertVal(val);
-      }
-    },
-
-    @Meta(name = coreidxsize,
-        type = Double.class,
-        isNodeSpecificVal = true,
-        isHidden = true,
-        min = 0,
-        metricsKey = "INDEX.sizeInBytes")
-    CORE_IDX() {
+    CORE_IDX(coreidxsize, Double.class, null, 0d, Double.MAX_VALUE,null, false,"INDEX.sizeInBytes" ) {
       @Override
       public Object convertVal(Object val) {
         return FREEDISK.convertVal(val);
       }
     },
-    @Meta(name = ImplicitSnitch.NODEROLE,
-        type = String.class,
-        enumVals = "overseer")
-    NODE_ROLE(),
-
-    @Meta(name = ImplicitSnitch.CORES,
-        type = Long.class,
-        min = 0)
-    CORES() {
+    NODE_ROLE(ImplicitSnitch.NODEROLE, String.class, Collections.singleton("overseer"), null, null),
+    CORES(ImplicitSnitch.CORES, Long.class, null, 0L, Long.MAX_VALUE) {
       @Override
       public Object validate(String name, Object val, boolean isRuleVal) {
         return getOperandAdjustedValue(super.validate(name, val, isRuleVal), val);
@@ -432,33 +335,12 @@ public class Suggestion {
         cell.val = cell.val == null ? 0 : ((Number) cell.val).longValue() - 1;
       }
     },
+    SYSLOADAVG(ImplicitSnitch.SYSLOADAVG, Double.class, null, 0d, 100d),
+    HEAPUSAGE(ImplicitSnitch.HEAPUSAGE, Double.class, null, 0d, null),
+    NUMBER("NUMBER", Long.class, null, 0L, Long.MAX_VALUE),
 
-    @Meta(name = ImplicitSnitch.SYSLOADAVG,
-        type = Double.class,
-        min = 0,
-        max = 100,
-        isNodeSpecificVal = true)
-    SYSLOADAVG(),
-
-    @Meta(name = ImplicitSnitch.HEAPUSAGE,
-        type = Double.class,
-        min = 0,
-        isNodeSpecificVal = true)
-    HEAPUSAGE(),
-    @Meta(name = "NUMBER",
-        type = Long.class,
-        min = 0)
-    NUMBER(),
-    @Meta(name = "STRING",
-        type = String.class,
-        wildCards = Policy.EACH)
-    STRING(),
-
-    @Meta(name = "node",
-        type = String.class,
-        isNodeSpecificVal = true,
-        wildCards = {Policy.ANY, Policy.EACH})
-    NODE() {
+    STRING("STRING", String.class, null, null, null),
+    NODE("node", String.class, null, null, null) {
       @Override
       public void getSuggestions(SuggestionCtx ctx) {
         if (ctx.violation == null || ctx.violation.replicaCountDelta == 0) return;
@@ -474,17 +356,14 @@ public class Suggestion {
 
       }
 
-      /*@Override
+      @Override
       public void addViolatingReplicas(ViolationCtx ctx) {
         for (Row r : ctx.allRows) {
           if(r.node.equals(ctx.tagKey)) collectViolatingReplicas(ctx,r);
         }
-      }*/
+      }
     },
-
-    @Meta(name = "LAZY",
-        type = void.class)
-    LAZY() {
+    LAZY("LAZY", null, null, null, null) {
       @Override
       public Object validate(String name, Object val, boolean isRuleVal) {
         return Clause.parseString(val);
@@ -495,73 +374,38 @@ public class Suggestion {
         perNodeSuggestions(ctx);
       }
     },
-
-    @Meta(name = ImplicitSnitch.DISKTYPE,
-        type = String.class,
-        enumVals = {"ssd", "rotational"})
-    DISKTYPE() {
+    DISKTYPE(ImplicitSnitch.DISKTYPE, String.class,
+        unmodifiableSet(new HashSet(Arrays.asList("ssd", "rotational"))), null, null) {
       @Override
       public void getSuggestions(SuggestionCtx ctx) {
         perNodeSuggestions(ctx);
       }
     };
 
+    final Class type;
+    final Set<String> vals;
+    final Number min;
+    final Number max;
+    final Boolean additive;
     public final String tagName;
-    public final Class type;
-    public Meta meta;
-
-    public final Set<String> vals;
-    public final Number min;
-    public final Number max;
-    public final Boolean additive;
-    public final boolean isHidden;
-    public final Set<String> wildCards;
     public final String perReplicaValue;
-    public final Set<String> associatedPerNodeValues;
     public final String metricsAttribute;
-    public final boolean isPerNodeValue;
-    public final Set<ComputedType> supportedComputedTypes;
-
-
-    ConditionType() {
-      try {
-        meta = ConditionType.class.getField(name()).getAnnotation(Meta.class);
-        if (meta == null) {
-          throw new RuntimeException("Invalid type, should have a @Meta annotation " + name());
-        }
-      } catch (NoSuchFieldException e) {
-        //cannot happen
-      }
-      this.tagName = meta.name();
-      this.type = meta.type();
-
-      this.vals = readSet(meta.enumVals());
-      this.max = readNum(meta.max());
-      this.min = readNum(meta.min());
-      this.perReplicaValue = readStr(meta.associatedPerReplicaValue());
-      this.associatedPerNodeValues = readSet(meta.associatedPerNodeValue());
-      this.additive = meta.isAdditive();
-      this.metricsAttribute = readStr(meta.metricsKey());
-      this.isPerNodeValue = meta.isNodeSpecificVal();
-      this.supportedComputedTypes = meta.computedValues()[0] == ComputedType.NULL ?
-          emptySet() :
-          unmodifiableSet(new HashSet(Arrays.asList(meta.computedValues())));
-      this.isHidden = meta.isHidden();
-      this.wildCards = readSet(meta.wildCards());
-    }
 
-    private String readStr(String s) {
-      return NULL.equals(s) ? null : s;
-    }
+    ConditionType(String tagName, Class type, Set<String> vals, Number min, Number max) {
+      this(tagName, type, vals, min, max, null, Boolean.TRUE, null);
 
-    private Number readNum(double v) {
-      return v == -1 ? null :
-          (Number) validate(null, v, true);
     }
 
-    Set<String> readSet(String[] vals) {
-      if (NULL.equals(vals[0])) return emptySet();
-      return unmodifiableSet(new HashSet<>(Arrays.asList(vals)));
+    ConditionType(String tagName, Class type, Set<String> vals, Number min, Number max, String perReplicaValue,
+                  Boolean additive, String metricsAttribute) {
+      this.tagName = tagName;
+      this.type = type;
+      this.vals = vals;
+      this.min = min;
+      this.max = max;
+      this.perReplicaValue = perReplicaValue;
+      this.additive = additive;
+      this.metricsAttribute = metricsAttribute;
     }
 
     public void getSuggestions(SuggestionCtx ctx) {
@@ -570,12 +414,11 @@ public class Suggestion {
 
     public void addViolatingReplicas(ViolationCtx ctx) {
       for (Row row : ctx.allRows) {
-        if (ctx.clause.tag.varType.isPerNodeValue && !row.node.equals(ctx.tagKey)) continue;
         collectViolatingReplicas(ctx, row);
       }
     }
 
-    public Operand getOperand(Operand expected, Object val, ComputedType computedType) {
+    public Operand getOperand(Operand expected, Object val, Clause.ComputationType computationType) {
       return expected;
     }
 
@@ -618,7 +461,7 @@ public class Suggestion {
         }
         return num;
       } else if (type == String.class) {
-        if (isRuleVal && !vals.isEmpty() && !vals.contains(val))
+        if (isRuleVal && vals != null && !vals.contains(val))
           throw new RuntimeException(name + ": " + val + " must be one of " + StrUtils.join(vals, ','));
         return val;
       } else {
@@ -642,32 +485,24 @@ public class Suggestion {
       return Math.abs(v1.replicaCountDelta) < Math.abs(v2.replicaCountDelta) ? -1 : 1;
     }
 
-    public Object computeValue(Policy.Session session, Clause.Condition condition, String collection, String shard, String node) {
+    public boolean supportComputed(Clause.ComputationType computedType, Clause clause) {
+      return false;
+    }
+
+    public Object computeValue(Policy.Session session, Clause.Condition condition, String collection, String shard) {
       return condition.val;
     }
   }
 
   private static void collectViolatingReplicas(ViolationCtx ctx, Row row) {
-    if (ctx.clause.tag.varType.isPerNodeValue) {
-      row.forEachReplica(replica -> {
-        if (ctx.clause.collection.isPass(replica.getCollection()) && ctx.clause.getShard().isPass(replica.getShard())) {
-          ctx.currentViolation.addReplica(new ReplicaInfoAndErr(replica)
-              .withDelta(ctx.clause.tag.delta(row.getVal(ctx.clause.tag.name))));
-        }
-      });
-    } else {
-      row.forEachReplica(replica -> {
-        if (ctx.clause.replica.isPass(0) && !ctx.clause.tag.isPass(row)) return;
-        if (!ctx.clause.replica.isPass(0) && ctx.clause.tag.isPass(row)) return;
-        if (!ctx.currentViolation.matchShard(replica.getShard())) return;
-        if (!ctx.clause.collection.isPass(ctx.currentViolation.coll) || !ctx.clause.shard.isPass(ctx.currentViolation.shard))
-          return;
-        ctx.currentViolation.addReplica(new ReplicaInfoAndErr(replica).withDelta(ctx.clause.tag.delta(row.getVal(ctx.clause.tag.name))));
-      });
-
-    }
-
-
+    row.forEachReplica(replica -> {
+      if (ctx.clause.replica.isPass(0) && !ctx.clause.tag.isPass(row)) return;
+      if (!ctx.clause.replica.isPass(0) && ctx.clause.tag.isPass(row)) return;
+      if (!ctx.currentViolation.matchShard(replica.getShard())) return;
+      if (!ctx.clause.collection.isPass(ctx.currentViolation.coll) || !ctx.clause.shard.isPass(ctx.currentViolation.shard))
+        return;
+      ctx.currentViolation.addReplica(new ReplicaInfoAndErr(replica).withDelta(ctx.clause.tag.delta(row.getVal(ctx.clause.tag.name))));
+    });
   }
 
   private static int getRelevantReplicasCount(Policy.Session session, Clause.Condition cv, String collection, String shard) {
@@ -722,12 +557,7 @@ public class Suggestion {
     }
   }
 
-  /*public static final Map<String, String> tagVsPerReplicaVal = Stream.of(ConditionType.values())
-      .filter(tag -> tag.perReplicaValue != null)
-      .collect(Collectors.toMap(tag -> tag.tagName, tag -> tag.perReplicaValue));*/
   static {
     for (Suggestion.ConditionType t : Suggestion.ConditionType.values()) Suggestion.validatetypes.put(t.tagName, t);
   }
-
-
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/09526a56/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Violation.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Violation.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Violation.java
index d4ab0e6..87b0abe 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Violation.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Violation.java
@@ -30,7 +30,7 @@ import org.apache.solr.common.util.Utils;
 public class Violation implements MapWriter {
   final String shard, coll, node;
   final Object actualVal;
-  final Double replicaCountDelta;//how many extra replicas
+  final Double replicaCountDelta;//how far is the actual value from the expected value
   final Object tagKey;
   private final int hash;
   private final Clause clause;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/09526a56/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java
index 92183b8..903a75c 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientNodeStateProvider.java
@@ -58,8 +58,6 @@ import org.slf4j.LoggerFactory;
 
 import static java.util.Collections.emptyMap;
 import static org.apache.solr.client.solrj.cloud.autoscaling.Clause.METRICS_PREFIX;
-import static org.apache.solr.client.solrj.cloud.autoscaling.Suggestion.ConditionType.FREEDISK;
-import static org.apache.solr.client.solrj.cloud.autoscaling.Suggestion.ConditionType.TOTALDISK;
 
 /**
  *
@@ -217,15 +215,18 @@ public class SolrClientNodeStateProvider implements NodeStateProvider, MapWriter
         }
       }
       if (requestedTags.contains(ImplicitSnitch.DISKTYPE)) {
-        metricsKeyVsTag.put("solr.node:CONTAINER.fs.coreRoot.spins", (Function<Object, Pair<String, Object>>) o -> {
-          if("true".equals(String.valueOf(o))){
-            return new Pair<>(ImplicitSnitch.DISKTYPE, "rotational");
-          }
-          if("false".equals(String.valueOf(o))){
-            return new Pair<>(ImplicitSnitch.DISKTYPE, "ssd");
-          }
-          return new Pair<>(ImplicitSnitch.DISKTYPE,null);
+        metricsKeyVsTag.put("solr.node:CONTAINER.fs.coreRoot.spins", new Function<Object, Pair<String,Object>>() {
+          @Override
+          public Pair<String, Object> apply(Object o) {
+            if("true".equals(String.valueOf(o))){
+              return new Pair<>(ImplicitSnitch.DISKTYPE, "rotational");
+            }
+            if("false".equals(String.valueOf(o))){
+              return new Pair<>(ImplicitSnitch.DISKTYPE, "ssd");
+            }
+            return new Pair<>(ImplicitSnitch.DISKTYPE,null);
 
+          }
         });
       }
       if (!metricsKeyVsTag.isEmpty()) {
@@ -238,10 +239,6 @@ public class SolrClientNodeStateProvider implements NodeStateProvider, MapWriter
         groups.add("solr.node");
         prefixes.add("CONTAINER.fs.usableSpace");
       }
-      if (requestedTags.contains(TOTALDISK.tagName)) {
-        groups.add("solr.node");
-        prefixes.add("CONTAINER.fs.totalSpace");
-      }
       if (requestedTags.contains(CORES)) {
         groups.add("solr.core");
         prefixes.add("CORE.coreName");
@@ -263,13 +260,9 @@ public class SolrClientNodeStateProvider implements NodeStateProvider, MapWriter
       try {
         SimpleSolrResponse rsp = snitchContext.invoke(solrNode, CommonParams.METRICS_PATH, params);
         Map m = rsp.nl.asMap(4);
-        if (requestedTags.contains(FREEDISK.tagName)) {
+        if (requestedTags.contains(DISK)) {
           Object n = Utils.getObjectByPath(m, true, "metrics/solr.node/CONTAINER.fs.usableSpace");
-          if (n != null) ctx.getTags().put(FREEDISK.tagName, FREEDISK.convertVal(n));
-        }
-        if (requestedTags.contains(TOTALDISK.tagName)) {
-          Object n = Utils.getObjectByPath(m, true, "metrics/solr.node/CONTAINER.fs.totalSpace");
-          if (n != null) ctx.getTags().put(TOTALDISK.tagName, TOTALDISK.convertVal(n));
+          if (n != null) ctx.getTags().put(DISK, Suggestion.getTagType(DISK).convertVal(n));
         }
         if (requestedTags.contains(CORES)) {
           int count = 0;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/09526a56/solr/solrj/src/java/org/apache/solr/common/MapWriter.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/common/MapWriter.java b/solr/solrj/src/java/org/apache/solr/common/MapWriter.java
index fc57e3d..1fba397 100644
--- a/solr/solrj/src/java/org/apache/solr/common/MapWriter.java
+++ b/solr/solrj/src/java/org/apache/solr/common/MapWriter.java
@@ -107,11 +107,6 @@ public interface MapWriter extends MapSerializable {
       return this;
     }
 
-    default EntryWriter putStringIfNotNull(String k, Object v) throws IOException {
-      if(v != null) put(k,String.valueOf(v));
-      return this;
-    }
-
 
     default EntryWriter put(String k, int v) throws IOException {
       put(k, (Integer) v);

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/09526a56/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 2d74db8..112d136 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
@@ -212,30 +212,30 @@ public class TestPolicy extends SolrTestCaseJ4 {
     assertEquals(Operand.RANGE_EQUAL, REPLICA.getOperand(Operand.EQUAL, "2.01", null));
     assertEquals(Operand.RANGE_NOT_EQUAL, REPLICA.getOperand(Operand.NOT_EQUAL, "2.01", null));
 
-    Clause clause = Clause.create("{replica: '1.23', node:'#ANY'}");
+    Clause clause = Clause.create((Map<String, Object>) Utils.fromJSONString("{replica: '1.23', node:'#ANY'}"));
     assertTrue(clause.getReplica().isPass(2));
     assertTrue(clause.getReplica().isPass(1));
     assertFalse(clause.getReplica().isPass(0));
     assertFalse(clause.getReplica().isPass(3));
 
-    clause = Clause.create("{replica: '<1.23', node:'#ANY'}");
+    clause = Clause.create((Map<String, Object>) Utils.fromJSONString("{replica: '<1.23', node:'#ANY'}"));
     assertTrue(clause.getReplica().isPass(1));
     assertTrue(clause.getReplica().isPass(0));
     assertFalse(clause.getReplica().isPass(2));
 
-    clause = Clause.create("{replica: '!1.23', node:'#ANY'}");
+    clause = Clause.create((Map<String, Object>) Utils.fromJSONString("{replica: '!1.23', node:'#ANY'}"));
     assertFalse(clause.getReplica().isPass(2));
     assertFalse(clause.getReplica().isPass(1));
     assertTrue(clause.getReplica().isPass(0));
     assertTrue(clause.getReplica().isPass(3));
 
-    clause = Clause.create("{replica: 1.23, node:'#ANY'}");
+    clause = Clause.create((Map<String, Object>) Utils.fromJSONString("{replica: 1.23, node:'#ANY'}"));
     assertTrue(clause.getReplica().isPass(2));
     assertTrue(clause.getReplica().isPass(1));
     assertFalse(clause.getReplica().isPass(0));
     assertFalse(clause.getReplica().isPass(3));
 
-    clause = Clause.create("{replica: '33%', node:'#ANY'}");
+    clause =  Clause.create((Map<String, Object>) Utils.fromJSONString("{replica: '33%', node:'#ANY'}"));
     assertEquals(Operand.RANGE_EQUAL, clause.getReplica().op);
     clause = clause.getSealedClause(condition -> {
       if (condition.name.equals("replica")) {
@@ -245,7 +245,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
     });
     assertTrue( clause.getReplica().isPass(2));
 
-    clause = Clause.create("{replica: '3 - 5', node:'#ANY'}");
+    clause =  Clause.create((Map<String, Object>) Utils.fromJSONString("{replica: '3 - 5', node:'#ANY'}"));
     assertEquals(Operand.RANGE_EQUAL,  clause.getReplica().getOperand());
     RangeVal range = (RangeVal) clause.getReplica().getValue();
     assertEquals(3.0 , range.min);
@@ -261,27 +261,18 @@ public class TestPolicy extends SolrTestCaseJ4 {
     assertEquals(new Double(0.0), clause.replica.delta(4));
 
     expectThrows(IllegalArgumentException.class,
-        () -> Clause.create("{replica: '-33%', node:'#ANY'}"));
+        () -> Clause.create((Map<String, Object>) Utils.fromJSONString("{replica: '-33%', node:'#ANY'}")));
     expectThrows(IllegalArgumentException.class,
-        () -> Clause.create("{replica: 'x%', node:'#ANY'}"));
+        () -> Clause.create((Map<String, Object>) Utils.fromJSONString("{replica: 'x%', node:'#ANY'}")));
     expectThrows(IllegalArgumentException.class,
-        () -> Clause.create("{replica: '20%-33%', node:'#ANY'}"));
+        () -> Clause.create((Map<String, Object>) Utils.fromJSONString("{replica: '20%-33%', node:'#ANY'}")));
 
-    clause = Clause.create("{replica: '#EQUAL', shard:'#EACH', node:'#ANY'}");
+    clause = Clause.create((Map<String, Object>) Utils.fromJSONString("{replica: '#EQUAL', shard:'#EACH', node:'#ANY'}"));
     assertEquals(Operand.RANGE_EQUAL, clause.replica.op);
-    clause = Clause.create("{replica: '#EQUAL', node:'#ANY'}");
+    clause = Clause.create((Map<String, Object>) Utils.fromJSONString("{replica: '#EQUAL', node:'#ANY'}"));
     assertEquals(Operand.RANGE_EQUAL, clause.replica.op);
     expectThrows(IllegalArgumentException.class,
-        () -> Clause.create("{replica: '#EQUAL', node:'node_1'}"));
-    clause = Clause.create("{replica : 0, freedisk:'<20%'}");
-    assertEquals(clause.tag.computedType, Clause.ComputedType.PERCENT);
-    assertEquals(clause.tag.op, Operand.LESS_THAN);
-    expectThrows(IllegalArgumentException.class,
-        () -> Clause.create("{replica : 0, INDEX.sizeInGB:'>300'}"));
-    expectThrows(IllegalArgumentException.class,
-        () -> Clause.create("{replica:'<3', shard: '#ANV', node:'#ANY'}"));
-    expectThrows(IllegalArgumentException.class,
-        () -> Clause.create("{replica:'<3', shard: '#EACH', node:'#E4CH'}"));
+        () -> Clause.create((Map<String, Object>) Utils.fromJSONString("{replica: '#EQUAL', node:'node_1'}")));
   }
 
 
@@ -437,36 +428,36 @@ public class TestPolicy extends SolrTestCaseJ4 {
   }
 
   public void testOperands() {
-    Clause c = Clause.create("{replica:'<2', node:'#ANY'}");
+    Clause c = Clause.create((Map<String, Object>) Utils.fromJSONString("{replica:'<2', node:'#ANY'}"));
     assertFalse(c.replica.isPass(3));
     assertFalse(c.replica.isPass(2));
     assertTrue(c.replica.isPass(1));
     assertEquals("{\"replica\":\"<2.0\"}", c.replica.toString());
 
-    c = Clause.create("{replica:'>2', node:'#ANY'}");
+    c = Clause.create((Map<String, Object>) Utils.fromJSONString("{replica:'>2', node:'#ANY'}"));
     assertTrue(c.replica.isPass(3));
     assertFalse(c.replica.isPass(2));
     assertFalse(c.replica.isPass(1));
     assertEquals("{\"replica\":\">2.0\"}", c.replica.toString());
 
 
-    c = Clause.create("{replica:0, nodeRole:'!overseer'}");
+    c = Clause.create((Map<String, Object>) Utils.fromJSONString("{replica:0, nodeRole:'!overseer'}"));
     assertTrue(c.tag.isPass("OVERSEER"));
     assertFalse(c.tag.isPass("overseer"));
 
-    c = Clause.create("{replica:0, sysLoadAvg:'<12.7'}");
+    c = Clause.create((Map<String, Object>) Utils.fromJSONString("{replica:0, sysLoadAvg:'<12.7'}"));
     assertTrue(c.tag.isPass("12.6"));
     assertTrue(c.tag.isPass(12.6d));
     assertFalse(c.tag.isPass("12.9"));
     assertFalse(c.tag.isPass(12.9d));
 
-    c = Clause.create("{replica:0, sysLoadAvg:'>12.7'}");
+    c = Clause.create((Map<String, Object>) Utils.fromJSONString("{replica:0, sysLoadAvg:'>12.7'}"));
     assertTrue(c.tag.isPass("12.8"));
     assertTrue(c.tag.isPass(12.8d));
     assertFalse(c.tag.isPass("12.6"));
     assertFalse(c.tag.isPass(12.6d));
 
-    c = Clause.create("{replica:0, 'metrics:x:y:z':'>12.7'}");
+    c = Clause.create((Map<String, Object>) Utils.fromJSONString("{replica:0, 'metrics:x:y:z':'>12.7'}"));
     assertTrue(c.tag.val instanceof String);
     assertTrue(c.tag.isPass("12.8"));
     assertTrue(c.tag.isPass(12.8d));
@@ -474,7 +465,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
     assertFalse(c.tag.isPass(12.6d));
 
     try {
-      c = Clause.create("{replica:0, 'ip_1':'<30%'}");
+      c = Clause.create((Map<String, Object>) Utils.fromJSONString("{replica:0, 'ip_1':'<30%'}"));
       fail("Expected exception");
     } catch (Exception e) {
       assertTrue(e.getMessage().contains("'%' is not allowed for variable :  'ip_1'"));
@@ -1108,7 +1099,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
     Policy.Session session;
     session = policy.createSession(getSolrCloudManager(nodeValues, clusterState));
 
-    List<Row> l = session.getSortedNodes();
+    List<Row> l = session.getSorted();
     assertEquals("node1", l.get(0).node);
     assertEquals("node3", l.get(1).node);
     assertEquals("node4", l.get(2).node);
@@ -1396,7 +1387,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
         "{replica:0, 'nodeRole':'overseer','strict':false}," +
         "{'replica':'<1','node':'node3'}," +
         "{'replica':'<2','node':'#ANY','shard':'#EACH'}," +
-        "{'replica':'<3','shard':'#EACH','sysprop.rack':'#EACH'}" +
+        "{'replica':'<3','shard':'#EACH','sysprop.rack':'#ANY'}" +
         "]" +
         "}" +
         "}";
@@ -1880,10 +1871,6 @@ public class TestPolicy extends SolrTestCaseJ4 {
     List<Violation> violations = cfg.getPolicy().createSession(cloudManagerWithData(dataproviderdata)).getViolations();
     assertEquals(1, violations.size());
     assertEquals(4, violations.get(0).getViolatingReplicas().size());
-    for (Violation.ReplicaInfoAndErr r : violations.get(0).getViolatingReplicas()) {
-      assertEquals(500d, r.delta, 0.1);
-
-    }
     List<Suggester.SuggestionInfo> l = PolicyHelper.getSuggestions(cfg, cloudManagerWithData(dataproviderdata));
     assertEquals(3, l.size());
     Map m = l.get(0).toMap(new LinkedHashMap<>());
@@ -2886,66 +2873,4 @@ public void testUtilizeNodeFailure2() throws Exception {
   }
 
 
-  public void testFreediskPercentage(){
-    String dataproviderdata = "{" +
-        "  'liveNodes': [" +
-        "    'node1:8983'," +
-        "    'node2:8984'," +
-        "    'node3:8985'" +
-        "  ]," +
-        "  'replicaInfo': {" +
-        "    'node1:8983': {" +
-        "      'c1': {" +
-        "        's1': [" +
-        "          {'r1': {'type': 'NRT'}}," +
-        "          {'r2': {'type': 'NRT'}}" +
-        "        ]," +
-        "        's2': [" +
-        "          {'r1': {'type': 'NRT'}}," +
-        "          {'r2': {'type': 'NRT'}}" +
-        "        ]" +
-        "      }" +
-        "    }" +
-        "  }," +
-        "  'nodeValues': {" +
-        "    'node1:8983': {" +
-        "      'cores': 4," +
-        "      'freedisk': 230," +
-        "      'totaldisk': 800," +
-        "      'port': 8983" +
-        "    }," +
-        "    'node2:8984': {" +
-        "      'cores': 0," +
-        "      'freedisk': 1000," +
-        "      'totaldisk': 1200," +
-        "      'port': 8984" +
-        "    }," +
-        "    'node3:8985': {" +
-        "      'cores': 0," +
-        "      'freedisk': 1500," +
-        "      'totaldisk': 1700," +
-        "      'port': 8985" +
-        "    }" +
-        "  }" +
-        "}";
-    String autoScalingjson = "{" +
-        "  'cluster-preferences': [" +
-        "    { 'maximize': 'freedisk', 'precision': 50}," +
-        "    { 'minimize': 'cores', 'precision': 3}" +
-        "  ]," +
-        "  'cluster-policy': [" +
-        "    { 'replica': 0,  freedisk : '<30%'}" +
-        "  ]" +
-        "}";
-    AutoScalingConfig cfg = new AutoScalingConfig((Map<String, Object>) Utils.fromJSONString(autoScalingjson));
-    List<Violation> violations = cfg.getPolicy().createSession(cloudManagerWithData((Map) Utils.fromJSONString(dataproviderdata))).getViolations();
-    assertEquals(1, violations.size());
-    assertEquals(4, violations.get(0).getViolatingReplicas().size());
-    for (Violation.ReplicaInfoAndErr r : violations.get(0).getViolatingReplicas()) {
-      assertEquals(10.0d, r.delta.doubleValue(), 0.1);
-    }
-
-  }
-
-
 }


Mime
View raw message