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:54 UTC

[lucene] branch jira/solr-14275 created (now f019aee)

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

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


      at f019aee  backup branch

This branch includes the following new commits:

     new f019aee  backup branch

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[lucene] 01/01: backup branch

Posted by dw...@apache.org.
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));