You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by dw...@apache.org on 2021/03/10 09:47:55 UTC

[lucene] 01/01: backup branch

This is an automated email from the ASF dual-hosted git repository.

dweiss pushed a commit to branch jira/solr-14275
in repository https://gitbox.apache.org/repos/asf/lucene.git

commit f019aee491ecbdc710780577757539ae77996ed6
Author: noble <no...@apache.org>
AuthorDate: Mon Jun 15 21:26:09 2020 +1000

    backup branch
---
 .../cloud/autoscaling/AddReplicaSuggester.java     |   2 -
 .../client/solrj/cloud/autoscaling/Clause.java     |  32 +++--
 .../solrj/cloud/autoscaling/FreeDiskVariable.java  |  17 +++
 .../solrj/cloud/autoscaling/PerClauseData.java     | 131 ++++++++++++++-------
 .../client/solrj/cloud/autoscaling/Policy.java     |   4 +-
 .../solr/client/solrj/cloud/autoscaling/Row.java   |  14 +--
 .../client/solrj/cloud/autoscaling/Suggestion.java |   3 +-
 .../client/solrj/cloud/autoscaling/Variable.java   |  13 ++
 .../client/solrj/cloud/autoscaling/Violation.java  |   7 +-
 .../client/solrj/cloud/autoscaling/TestPolicy.java |  26 ++--
 10 files changed, 164 insertions(+), 85 deletions(-)

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 115dacd..503fbcd 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
@@ -27,7 +27,6 @@ import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.params.CollectionParams;
 import org.apache.solr.common.util.Pair;
-import org.apache.solr.common.util.Utils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -62,7 +61,6 @@ class AddReplicaSuggester extends Suggester {
         if (!isNodeSuitableForReplicaAddition(row, null)) continue;
         Row tmpRow = row.addReplica(shard.first(), shard.second(), type, strict);
         List<Violation> errs = testChangedMatrix(strict, tmpRow, tmpRow.session);
-        if(row.node.equals("node3")) System.out.println(""+ Utils.toJSONString(errs));
         if (!containsNewErrors(errs)) {
           if ((errs.isEmpty() && isLessDeviant()) ||//there are no violations but this is deviating less
               isLessSerious(errs, leastSeriousViolation)) {//there are errors , but this has less serious violation
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 4bee4b5..37d17ce 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
@@ -90,31 +90,36 @@ public class Clause implements MapWriter, Comparable<Clause> {
   }
 
   // internal use only
-  Clause(Map<String, Object> original, Condition tag, Condition globalTag, boolean isStrict, Put put, boolean nodeSetPresent, DataGrouping dataGrouping) {
+  Clause(Map<String, Object> original, Condition tag, Condition globalTag, boolean isStrict, Put put, boolean nodeSetPresent, DataGrouping dataGrouping, Condition replica, Condition collection, Condition shard) {
     this.hashCode = original.hashCode();
     this.original = original;
-    this.tag = tag;
-    this.globalTag = globalTag;
-    this.globalTag.clause = this;
+    this.tag = setClause(tag);
+    this.globalTag = setClause(globalTag);
     this.type = null;
     this.hasComputedValue = false;
     this.strict = isStrict;
     derivedFrom = null;
     this.put = put;
     this.nodeSetPresent = nodeSetPresent;
-    this.collection = null;
-    this.shard = null;
-    this.replica = null;
+    this.collection = setClause(collection);
+    this.shard = setClause(shard);
+    this.replica = setClause(replica);
     this.dataGrouping = dataGrouping;
   }
 
+  private Condition setClause(Condition condition) {
+    if (condition == null) return null;
+    condition.clause = this;
+    return condition;
+  }
+
   private Clause(Map<String, Object> m) {
     derivedFrom = (Clause) m.remove(Clause.class.getName());
     this.original = Utils.getDeepCopy(m, 10);
     this.hashCode = original.hashCode();
     String type = (String) m.get("type");
     this.type = type == null || ANY.equals(type) ? null : Replica.Type.valueOf(type.toUpperCase(Locale.ROOT));
-    String put = (String) m.getOrDefault("put", m.containsKey(NODESET)? Put.ON_ALL.val: null );
+    String put = (String) m.getOrDefault("put", m.containsKey(NODESET) ? Put.ON_ALL.val : null);
     if (put != null) {
       this.put = Put.get(put);
       if (this.put == null) throwExp(m, "invalid value for put : {0}", put);
@@ -272,6 +277,9 @@ public class Clause implements MapWriter, Comparable<Clause> {
 
   public static Clause create(Map<String, Object> m) {
     Clause clause = new Clause(m);
+    if (clause.tag != null) {
+      clause = clause.tag.varType.transform(clause);
+    }
     return clause.hasComputedValue() ?
         clause :
         clause.getSealedClause(null);
@@ -715,16 +723,16 @@ public class Clause implements MapWriter, Comparable<Clause> {
   public List<Violation> test(Policy.Session session, Row changedRow, double[] deviations) {
     if (isPerCollectiontag()) {
       if(nodeSetPresent) {
-        if(put == Put.ON_EACH) {
-          return testPerNode(session, changedRow, deviations) ;
+        if (put == Put.ON_EACH || getThirdTag().varType.meta.alwaysPutOnEachNode()) {
+          return testPerNode(session, changedRow, deviations);
         } else {
-          return session.perClauseData.computeViolations(session, this);
+          return session.perClauseData.computeViolations(session, this, deviations);
           //return testGroupNodes(session, deviations);
         }
       }
 
       if(dataGrouping != DataGrouping.NODE && tag.varType == Type.NODE){
-        return session.perClauseData.computeViolations(session, this);
+        return session.perClauseData.computeViolations(session, this, deviations);
       }
 
 
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/FreeDiskVariable.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/FreeDiskVariable.java
index 8b6207b..c4b3ac8 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/FreeDiskVariable.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/FreeDiskVariable.java
@@ -53,6 +53,23 @@ public class FreeDiskVariable extends VariableBase {
   }
 
   @Override
+  public Clause transform(Clause clause) {
+    if (clause.replica.computedType == ComputedType.ALL) {
+      return new Clause(clause.original,
+          new Condition(FREEDISK.tagName, clause.tag.val, clause.tag.op.opposite(true), null, null),
+          null,
+          clause.strict,
+          clause.put,
+          clause.nodeSetPresent,
+          clause.dataGrouping,
+          new Condition(Type.REPLICA.tagName, 0, Operand.EQUAL, null, null),
+          clause.collection,
+          clause.shard);
+    }
+    return clause;
+  }
+
+  @Override
   public Object computeValue(Condition condition, Clause.ComputedValueEvaluator evaluator) {
     if (condition.computedType == ComputedType.PERCENT) {
       Row r = evaluator.getNode();
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/PerClauseData.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/PerClauseData.java
index 6f538ac..ae8fc50 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/PerClauseData.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/PerClauseData.java
@@ -32,6 +32,7 @@ import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.util.ReflectMapWriter;
 
 import static java.util.Collections.singletonMap;
+import static org.apache.solr.client.solrj.cloud.autoscaling.Operand.RANGE_EQUAL;
 import static org.apache.solr.client.solrj.cloud.autoscaling.Variable.Type.NODE;
 
 public class PerClauseData implements ReflectMapWriter, Cloneable {
@@ -96,7 +97,6 @@ public class PerClauseData implements ReflectMapWriter, Cloneable {
 
     public final String coll;
     public final Map<String, ShardDetails> shards = new HashMap<>();
-
     Map<Clause,  Map<String,  ReplicaCount>> clauseValues = new HashMap<>();
 
     @Override
@@ -115,6 +115,7 @@ public class PerClauseData implements ReflectMapWriter, Cloneable {
     CollectionDetails copy() {
       CollectionDetails result = new CollectionDetails(coll);
       shards.forEach((k, shardDetails) -> result.shards.put(k, shardDetails.copy()));
+      copyTo(clauseValues, result.clauseValues);
       return result;
     }
 
@@ -146,11 +147,9 @@ public class PerClauseData implements ReflectMapWriter, Cloneable {
 
     ShardDetails copy() {
       ShardDetails result = new ShardDetails(coll, shard);
-      values.forEach((clause, clauseVal) -> {
-        HashMap<String,ReplicaCount> m = new HashMap(clauseVal);
-        for (Map.Entry<String, ReplicaCount> e : m.entrySet()) e.setValue(e.getValue().copy());
-        result.values.put(clause, m);
-      });
+      result.indexSize = indexSize;
+      result.replicas.increment(replicas);
+      copyTo(values, result.values);
       return result;
     }
 
@@ -159,78 +158,126 @@ public class PerClauseData implements ReflectMapWriter, Cloneable {
     }
   }
 
+  static void copyTo(Map<Clause, Map<String, ReplicaCount>> values, Map<Clause, Map<String, ReplicaCount>> sink) {
+    values.forEach((clause, clauseVal) -> {
+      HashMap<String, ReplicaCount> m = new HashMap(clauseVal);
+      for (Map.Entry<String, ReplicaCount> e : m.entrySet()) e.setValue(e.getValue().copy());
+      sink.put(clause, m);
+    });
+  }
+
+
   static class LazyViolation extends Violation {
     private Policy.Session session;
+    private List<ReplicaInfoAndErr> lazyreplicaInfoAndErrs;
 
     LazyViolation(SealedClause clause, String coll, String shard, String node, Object actualVal, Double replicaCountDelta, Object tagKey, Policy.Session session) {
       super(clause, coll, shard, node, actualVal, replicaCountDelta, tagKey);
-      super.replicaInfoAndErrs = null;
       this.session = session;
     }
 
     @Override
     public List<ReplicaInfoAndErr> getViolatingReplicas() {
-      if(replicaInfoAndErrs == null){
+      if (lazyreplicaInfoAndErrs == null) {
         populateReplicas();
       }
-      return replicaInfoAndErrs;
+      return lazyreplicaInfoAndErrs;
     }
 
     private void populateReplicas() {
-      replicaInfoAndErrs = new ArrayList<>();
+      lazyreplicaInfoAndErrs = new ArrayList<>();
       for (Row row : session.matrix) {
-        if(getClause().getThirdTag().isPass(row)) {
-            row.forEachReplica(coll, ri -> {
-              if(shard == null || Objects.equals(shard, ri.getShard()))
-              replicaInfoAndErrs.add(new ReplicaInfoAndErr(ri));
-            });
+        if (node != null && !node.equals(row.node)) continue;
+        if (getClause().getThirdTag().isPass(row)) {
+          row.forEachReplica(coll, ri -> {
+            if (shard == null || Policy.ANY.equals(shard) || Objects.equals(shard, ri.getShard()))
+              lazyreplicaInfoAndErrs.add(new ReplicaInfoAndErr(ri));
+          });
 
         }
       }
     }
   }
-  public static final String SINGLEVALUE = "";
-  private static final Map<String, ReplicaCount> EMPTY = singletonMap(SINGLEVALUE, new ReplicaCount());
-//  private final ReplicaCount EMPTY = new ReplicaCount();
-
-  void getViolations( Map<Clause,  Map<String, ReplicaCount>> vals ,
-                      List<Violation> violations,
-                      Clause.ComputedValueEvaluator evaluator,
-                      Clause clause) {
-    Map<String,ReplicaCount> rc = vals.get(clause);
-    if (rc == null ) rc= EMPTY;
 
+  public static final String SINGLEVALUE = "";
+  private static final ReplicaCount EMPTY_COUNT = new ReplicaCount();
+  private static final Map<String, ReplicaCount> EMPTY = singletonMap(SINGLEVALUE, EMPTY_COUNT);
+
+  void getViolations(Map<Clause, Map<String, ReplicaCount>> vals,
+                     List<Violation> violations,
+                     Clause.ComputedValueEvaluator evaluator,
+                     Clause clause, double[] deviations) {
+    Map<String, ReplicaCount> rc = vals.get(clause);
+    if (rc == null) rc = EMPTY;
     SealedClause sc = clause.getSealedClause(evaluator);
-
-    rc.forEach((name, replicaCount) -> {
-      if (!sc.replica.isPass(replicaCount)) {
-        Violation v = new LazyViolation(
-            sc,
-            evaluator.collName,
-            evaluator.shardName == null? Policy.ANY: evaluator.shardName,
-            NODE.tagName.equals( clause.tag.name)? name:  null,
-            replicaCount,
-            sc.getReplica().replicaCountDelta(replicaCount),
-            name,//TODO
-            evaluator.session);
-        violations.add(v);
+    if (clause.getThirdTag().varType == Variable.Type.NODE && clause.getThirdTag().op == Operand.WILDCARD) {
+      for (Row row : evaluator.session.matrix) {
+        ReplicaCount replicaCount = rc.getOrDefault(row.node, EMPTY_COUNT);
+        addViolation(violations, evaluator, deviations, sc, row.node, row.node, replicaCount);
       }
-    });
+    } else {
+      rc.forEach((name, replicaCount) -> {
+        if (!sc.replica.isPass(replicaCount)) {
+          Violation v = new LazyViolation(
+              sc,
+              evaluator.collName,
+              evaluator.shardName == null ? Policy.ANY : evaluator.shardName,
+              NODE.tagName.equals(clause.tag.name) ? name : null,
+              replicaCount,
+              sc.getReplica().replicaCountDelta(replicaCount),
+              name,
+              evaluator.session);
+          violations.add(v);
+          if (!clause.strict && deviations != null) {
+            clause.getThirdTag().varType.computeDeviation(evaluator.session, deviations, replicaCount, sc);
+          }
+        } else {
+          if (sc.replica.op == RANGE_EQUAL)
+            sc.getThirdTag().varType.computeDeviation(evaluator.session, deviations, replicaCount, sc);
+        }
+      });
+
+    }
+  }
 
+  private void addViolation(List<Violation> violations,
+                            Clause.ComputedValueEvaluator evaluator,
+                            double[] deviations, SealedClause sc,
+                            String node,
+                            String key,
+                            ReplicaCount replicaCount) {
+    if (!sc.replica.isPass(replicaCount)) {
+      Violation v = new LazyViolation(
+          sc,
+          evaluator.collName,
+          evaluator.shardName == null ? Policy.ANY : evaluator.shardName,
+          node,
+          replicaCount,
+          sc.getReplica().replicaCountDelta(replicaCount),
+          key,
+          evaluator.session);
+      violations.add(v);
+      if (!sc.strict && deviations != null) {
+        sc.getThirdTag().varType.computeDeviation(evaluator.session, deviations, replicaCount, sc);
+      }
+    } else {
+      if (sc.replica.op == RANGE_EQUAL)
+        sc.getThirdTag().varType.computeDeviation(evaluator.session, deviations, replicaCount, sc);
+    }
   }
 
-  List<Violation> computeViolations(Policy.Session session, Clause clause) {
+  List<Violation> computeViolations(Policy.Session session, Clause clause, double[] deviations) {
     Clause.ComputedValueEvaluator evaluator = new Clause.ComputedValueEvaluator(session);
     List<Violation> result = new ArrayList<>();
     collections.forEach((coll, cd) -> {
       evaluator.collName = coll;
       evaluator.shardName = null;
       if (clause.dataGrouping == Clause.DataGrouping.COLL) {
-        getViolations(cd.clauseValues, result, evaluator, clause);
+        getViolations(cd.clauseValues, result, evaluator, clause, deviations);
       } else if (clause.dataGrouping == Clause.DataGrouping.SHARD) {
         cd.shards.forEach((shard, sd) -> {
           evaluator.shardName = shard;
-          getViolations(sd.values, result, evaluator, clause);
+          getViolations(sd.values, result, evaluator, clause, deviations);
         });
       }
     });
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 77cbc53..0736a61 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
@@ -572,9 +572,9 @@ public class Policy implements MapWriter {
         if (!vals.isEmpty() && vals.get("withCollection") != null) {
           Map<String, String> withCollMap = (Map<String, String>) vals.get("withCollection");
           if (!withCollMap.isEmpty()) {
-            Clause withCollClause = new Clause((Map<String,Object>)Utils.fromJSONString("{withCollection:'*' , node: '#ANY'}") ,
+            Clause withCollClause = new Clause((Map<String, Object>) Utils.fromJSONString("{withCollection:'*' , node: '#ANY'}"),
                 new Condition(NODE.tagName, "#ANY", Operand.EQUAL, null, null),
-                new Condition(WITH_COLLECTION.tagName,"*" , Operand.EQUAL, null, null), true, null, false, Clause.DataGrouping.NODE
+                new Condition(WITH_COLLECTION.tagName, "*", Operand.EQUAL, null, null), true, null, false, Clause.DataGrouping.NODE, null, null, null
             );
             expandedClauses.add(withCollClause);
           }
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 844fe41..34ed7a5 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
@@ -332,7 +332,7 @@ public class Row implements MapWriter {
       }
     }
 
-    row.modifyPerClauseCount(ri, 1);
+    row.modifyPerClauseCount(ri, 1, this);
 
     return row;
   }
@@ -436,7 +436,7 @@ public class Row implements MapWriter {
     for (Cell cell : row.cells) {
       cell.type.projectRemoveReplica(cell, removed, opCollector);
     }
-    row.modifyPerClauseCount(removed, -1);
+    row.modifyPerClauseCount(removed, -1, this);
     return row;
 
   }
@@ -471,13 +471,13 @@ public class Row implements MapWriter {
     }
   }
 
-  void modifyPerClauseCount(ReplicaInfo ri, int delta) {
+  void modifyPerClauseCount(ReplicaInfo ri, int delta, Row previous) {
     if (session == null || session.perClauseData == null || ri == null) return;
-    session.perClauseData.getShardDetails(ri.getCollection(),ri.getShard()).incrReplicas(ri.getType(), delta);
+    session.perClauseData.getShardDetails(ri.getCollection(), ri.getShard()).incrReplicas(ri.getType(), delta);
     for (Clause clause : session.expandedClauses) {
-      if(!clause.dataGrouping.storePerClauseData()) continue;
-      if(!clause.collection.isPass(ri.getCollection()) ||
-          !clause.shard.isPass(ri.getShard())  ||
+      if (!clause.dataGrouping.storePerClauseData()) continue;
+      if (!clause.collection.isPass(ri.getCollection()) ||
+          !clause.shard.isPass(ri.getShard()) ||
           !clause.isType(ri.getType())) continue;
       if (clause.put == Clause.Put.ON_EACH) continue;
       if (clause.dataGrouping.storePerClauseData()) {
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 ae67e44..09a38d5 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
@@ -67,7 +67,8 @@ public class Suggestion {
     }
 
     public boolean hasTimedOut() {
-      return session.cloudManager.getTimeSource().getTimeNs() >= endTime;
+      return false;//NOCOMMIT
+//      return session.cloudManager.getTimeSource().getTimeNs() >= endTime;
 
     }
 
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Variable.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Variable.java
index dc067fc..41ae96c 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Variable.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Variable.java
@@ -90,6 +90,10 @@ public interface Variable {
 
   Object validate(String name, Object val, boolean isRuleVal);
 
+  default Clause transform(Clause clause) {
+    return clause;
+  }
+
   /**
    * Type details of each variable in policies
    */
@@ -158,6 +162,7 @@ public interface Variable {
         associatedPerReplicaValue = Variable.coreidxsize,
         associatedPerNodeValue = "totaldisk",
         implementation = FreeDiskVariable.class,
+        alwaysPutOnEachNode = true,
         computedValues = ComputedType.PERCENT)
     FREEDISK,
 
@@ -321,10 +326,16 @@ public interface Variable {
       return impl.postValidate(condition);
     }
 
+
     public Object validate(String name, Object val, boolean isRuleVal) {
       return impl.validate(name, val, isRuleVal);
     }
 
+    @Override
+    public Clause transform(Clause clause) {
+      return impl.transform(clause);
+    }
+
     /**
      * Simulate a replica addition to a node in the cluster
      */
@@ -401,6 +412,8 @@ public interface Variable {
 
     Class implementation() default void.class;
 
+    boolean alwaysPutOnEachNode() default false;
+
     ComputedType[] computedValues() default ComputedType.NULL;
   }
 }
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 64efc2c..121d757 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
@@ -21,7 +21,6 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Objects;
 import java.util.function.Function;
@@ -37,7 +36,7 @@ public class Violation implements MapWriter {
   final Object tagKey;
   private final int hash;
   private final Clause clause;
-  protected List<ReplicaInfoAndErr> replicaInfoAndErrs = new ArrayList<>();
+  private List<ReplicaInfoAndErr> replicaInfoAndErrs = new ArrayList<>();
 
   Violation(SealedClause clause, String coll, String shard, String node, Object actualVal, Double replicaCountDelta, Object tagKey) {
     this.clause = clause;
@@ -129,10 +128,6 @@ public class Violation implements MapWriter {
     }
   }
 
-  @Override
-  public String toString() {
-    return Utils.toJSONString(Utils.getDeepCopy(toMap(new LinkedHashMap<>()), 5));
-  }
 
   @Override
   public void writeMap(EntryWriter ew) throws IOException {
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 0d4c391..2afbeca 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
@@ -61,6 +61,7 @@ import org.apache.solr.common.cloud.rule.ImplicitSnitch;
 import org.apache.solr.common.params.CollectionAdminParams;
 import org.apache.solr.common.params.CollectionParams;
 import org.apache.solr.common.params.CollectionParams.CollectionAction;
+import org.apache.solr.common.params.MapSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.JsonTextWriter;
 import org.apache.solr.common.util.NamedList;
@@ -880,9 +881,10 @@ public class TestPolicy extends SolrTestCaseJ4 {
         assertEquals(2.0, val.max.doubleValue(), 0.01);
         assertEquals(1.2d, val.actual.doubleValue(), 0.01d);
         assertEquals(1, violation.replicaCountDelta.doubleValue(), 0.01);
-        assertEquals(3, violation.getViolatingReplicas().size());
+        List<Violation.ReplicaInfoAndErr> violatingReplicas = violation.getViolatingReplicas();
+        assertEquals(3, violatingReplicas.size());
         Set<String> expected = ImmutableSet.of("r1", "r3", "r5");
-        for (Violation.ReplicaInfoAndErr replicaInfoAndErr : violation.getViolatingReplicas()) {
+        for (Violation.ReplicaInfoAndErr replicaInfoAndErr : violatingReplicas) {
           assertTrue(expected.contains(replicaInfoAndErr.replicaInfo.getCore()));
         }
       } else if (violation.node.equals("node5")) {
@@ -892,10 +894,6 @@ public class TestPolicy extends SolrTestCaseJ4 {
         fail();
       }
     }
-//    Violation violation = violations.get(0);
-//    assertEquals("node1", violation.node);
-
-
   }
 
   private static void expectError(String name, Object val, String msg) {
@@ -2243,7 +2241,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
 
     }
     List<Suggester.SuggestionInfo> l = PolicyHelper.getSuggestions(new AutoScalingConfig((Map<String, Object>) Utils.fromJSONString(autoScalingjson)),
-        cloudManagerWithData((Map) loadFromResource("testReplicaCountSuggestions.json")));
+        cloudManagerWithData((Map) loadFromResource("testReplicaCountSuggestions.json")), new MapSolrParams(Map.of("type", "violation")));
     assertFalse(l.isEmpty());
 
     assertEquals(1.0d, l.get(0)._get( "violation/violation/delta",null));
@@ -2303,8 +2301,8 @@ public class TestPolicy extends SolrTestCaseJ4 {
 
   public void testReplicaZonesPercentage() {
     String autoScalingjson = "  { cluster-policy:[" +
-        "    { replica :'33%', shard: '#EACH', sysprop.az : east}," +
-        "    { replica :'67%', shard: '#EACH', sysprop.az : west}" +
+        "    { replica :'33%',  shard: '#EACH', nodeset:{ sysprop.az : east}}," +
+        "    { replica :'67%', shard: '#EACH', nodeset: {sysprop.az : west}}" +
         "    ]," +
         "  cluster-preferences :[{ minimize : cores }]}";
 
@@ -2318,7 +2316,8 @@ public class TestPolicy extends SolrTestCaseJ4 {
 
     int westCount = 0, eastCount = 0;
     for (int i = 0; i < 12; i++) {
-      SolrRequest suggestion = txn.getCurrentSession()
+      Policy.Session currentSession = txn.getCurrentSession();
+      SolrRequest suggestion = currentSession
           .getSuggester(ADDREPLICA)
           .hint(Hint.COLL_SHARD, new Pair<>(COLL_NAME, "shard1"))
           .getSuggestion();
@@ -2434,11 +2433,12 @@ public class TestPolicy extends SolrTestCaseJ4 {
     cfg = new AutoScalingConfig((Map<String, Object>) Utils.fromJSONString(autoScalingjson));
     violations = cfg.getPolicy().createSession(cloudManagerWithData((Map) loadFromResource("testFreeDiskSuggestions.json"))).getViolations();
     assertEquals(1, violations.size());
-    assertEquals(-4, violations.get(0).replicaCountDelta, 0.1);
+    assertEquals(4, violations.get(0).replicaCountDelta, 0.1);
     assertEquals(1, violations.size());
-    assertEquals(0, violations.get(0).getViolatingReplicas().size());
+    assertEquals(4, violations.get(0).getViolatingReplicas().size());
 
-    l = PolicyHelper.getSuggestions(cfg, cloudManagerWithData((Map) loadFromResource("testFreeDiskSuggestions.json")));
+    l = PolicyHelper.getSuggestions(cfg, cloudManagerWithData((Map) loadFromResource("testFreeDiskSuggestions.json")),
+        new MapSolrParams(Collections.singletonMap("type", "violation")));
     assertEquals(3, l.size());
     assertEquals("r4", l.get(0)._get("operation/command/move-replica/replica", null));
     assertEquals("node1", l.get(0)._get("operation/command/move-replica/targetNode", null));