You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by no...@apache.org on 2017/05/22 06:11:02 UTC

lucene-solr:feature/autoscaling: test all rows in one go instead of one row at a time

Repository: lucene-solr
Updated Branches:
  refs/heads/feature/autoscaling c15a17379 -> 07e630b7c


test all rows in one go instead of one row at a time


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

Branch: refs/heads/feature/autoscaling
Commit: 07e630b7c4fc437816ea2c9cc612acf6492541a8
Parents: c15a173
Author: Noble Paul <no...@apache.org>
Authored: Mon May 22 15:40:49 2017 +0930
Committer: Noble Paul <no...@apache.org>
Committed: Mon May 22 15:40:49 2017 +0930

----------------------------------------------------------------------
 .../cloud/autoscaling/AddReplicaSuggester.java  |  16 ++-
 .../org/apache/solr/cloud/autoscaling/Cell.java |   7 ++
 .../apache/solr/cloud/autoscaling/Clause.java   | 112 ++++++++++++++-----
 .../cloud/autoscaling/MoveReplicaSuggester.java |  46 +++-----
 .../apache/solr/cloud/autoscaling/Operand.java  |   2 +-
 .../apache/solr/cloud/autoscaling/Policy.java   |  43 +++++--
 .../solr/cloud/autoscaling/PolicyHelper.java    |  15 ++-
 .../org/apache/solr/cloud/autoscaling/Row.java  |  13 ++-
 .../solr/cloud/autoscaling/TestPolicy.java      |  53 +++++----
 9 files changed, 201 insertions(+), 106 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/07e630b7/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/AddReplicaSuggester.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/AddReplicaSuggester.java b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/AddReplicaSuggester.java
index fc0cecc..d0e510d 100644
--- a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/AddReplicaSuggester.java
+++ b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/AddReplicaSuggester.java
@@ -17,6 +17,8 @@
 
 package org.apache.solr.cloud.autoscaling;
 
+import java.util.List;
+
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.cloud.autoscaling.Policy.Suggester;
@@ -35,15 +37,16 @@ class AddReplicaSuggester extends Suggester {
     if (coll == null || shard == null)
       throw new RuntimeException("add-replica requires 'collection' and 'shard'");
     //iterate through elements and identify the least loaded
+
     for (int i = getMatrix().size() - 1; i >= 0; i--) {
       Row row = getMatrix().get(i);
       if (!isAllowed(row.node, Hint.TARGET_NODE)) continue;
-      row = row.addReplica(coll, shard);
-      row.violations.clear();
-      for (Clause clause : session.expandedClauses) {
-        if (strict || clause.strict) clause.test(row);
-      }
-      if (row.violations.isEmpty()) {// there are no rule violations
+      Row tmpRow = row.addReplica(coll, shard);
+      tmpRow.violations.clear();
+
+      List<Clause.Violation> errs = testChangedRow(strict, getModifiedMatrix(getMatrix(), tmpRow, i));
+
+      if (!containsNewErrors(errs)) {// there are no rule violations
         getMatrix().set(i, getMatrix().get(i).addReplica(coll, shard));
         return CollectionAdminRequest
             .addReplicaToShard(coll, shard)
@@ -53,4 +56,5 @@ class AddReplicaSuggester extends Suggester {
     return null;
   }
 
+
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/07e630b7/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Cell.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Cell.java b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Cell.java
index c14490f..540872c 100644
--- a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Cell.java
+++ b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Cell.java
@@ -18,8 +18,10 @@
 package org.apache.solr.cloud.autoscaling;
 
 import java.io.IOException;
+import java.util.HashMap;
 
 import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.util.Utils;
 
 class Cell implements MapWriter {
   final int index;
@@ -44,6 +46,11 @@ class Cell implements MapWriter {
     ew.put(name, val);
   }
 
+  @Override
+  public String toString() {
+    return Utils.toJSONString(this.toMap(new HashMap<>()));
+  }
+
   public Cell copy() {
     return new Cell(index, name, val, val_);
   }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/07e630b7/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Clause.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Clause.java b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Clause.java
index 2da56e9..4fdd88a 100644
--- a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Clause.java
+++ b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Clause.java
@@ -18,16 +18,19 @@
 package org.apache.solr.cloud.autoscaling;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.function.Predicate;
 
+import org.apache.solr.client.solrj.request.UpdateRequest;
 import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.util.Utils;
 import org.apache.solr.cloud.autoscaling.Policy.ReplicaInfo;
@@ -124,6 +127,10 @@ public class Clause implements MapWriter, Comparable<Clause> {
       return op.match(val, row.getVal(name));
     }
 
+    TestStatus match(Object testVal) {
+      return op.match(this.val, testVal);
+    }
+
     boolean isPass(Object inputVal) {
       return op.match(val, inputVal) == PASS;
     }
@@ -169,44 +176,91 @@ public class Clause implements MapWriter, Comparable<Clause> {
     }
   }
 
+  public class Violation {
+    final String shard, coll, node;
+    private final int hash;
 
-  TestStatus test(Row row) {
-    AtomicReference<TestStatus> result = new AtomicReference<>(NOT_APPLICABLE);
-    if (isPerCollectiontag()) {
 
-      for (Map.Entry<String, Map<String, List<ReplicaInfo>>> colls : row.replicaInfo.entrySet()) {
-        if (result.get() == FAIL) break;
-        if (!collection.isPass(colls.getKey())) continue;
-        int count = 0;
-        for (Map.Entry<String, List<ReplicaInfo>> shards : colls.getValue().entrySet()) {
-          if (!shard.isPass(shards.getKey()) || result.get() == FAIL) break;
-          count += shards.getValue().size();
-          if (shard.val.equals(EACH)) testReplicaCount(row, result, count);
-          if (EACH.equals(shard.val)) count = 0;
+    private Violation(String coll, String shard, String node) {
+      this.shard = shard;
+      this.coll = coll;
+      this.node = node;
+      hash = ("" + coll + " " + shard + " " + node + " " + Utils.toJSONString(getClause().toMap(new HashMap<>()))).hashCode();
+    }
+
+    public Clause getClause() {
+      return Clause.this;
+    }
+
+    @Override
+    public int hashCode() {
+      return hash;
+    }
+
+    @Override
+    public boolean equals(Object that) {
+      if (that instanceof Violation) {
+        Violation ve = (Violation) that;
+        return Objects.equals(this.shard, (ve).shard) &&
+            Objects.equals(this.coll, (ve).coll) &&
+            Objects.equals(this.node, (ve).node);
+      }
+      return false;
+    }
+  }
+
+
+  public List<Violation> test(List<Row> allRows) {
+    List<Violation> errors = new ArrayList<>();
+    if (isPerCollectiontag()) {
+      Map<String, Map<String, Map<String, AtomicInteger>>> replicaCount = computeReplicaCounts(allRows);
+      for (Map.Entry<String, Map<String, Map<String, AtomicInteger>>> e : replicaCount.entrySet()) {
+        if (!collection.isPass(e.getKey())) continue;
+        for (Map.Entry<String, Map<String, AtomicInteger>> shardVsCount : e.getValue().entrySet()) {
+          if (!shard.isPass(shardVsCount.getKey())) continue;
+          for (Map.Entry<String, AtomicInteger> counts : shardVsCount.getValue().entrySet()) {
+            if (!replica.isPass(counts.getValue())) {
+              errors.add(new Violation(e.getKey(), shardVsCount.getKey(),
+                  tag.name.equals("node") ? counts.getKey() : null));
+            }
+          }
         }
-        if (shard.val.equals(ANY)) testReplicaCount(row, result, count);
       }
     } else {
-      if (!tag.isPass(row)) result.set(TestStatus.FAIL);
+      for (Row r : allRows) {
+        if (!tag.isPass(r)) {
+          errors.add(new Violation(null, null, r.node));
+        }
+      }
     }
-    if (result.get() == FAIL)
-      row.violations.add(this);
-    return result.get();
+    return errors;
 
   }
 
-  private void testReplicaCount(Row row, AtomicReference<TestStatus> result, int count) {
-    if ("node".equals(tag.name)) if (!tag.isPass(row.node)) return;
-    boolean checkCount = replica.op.match(replica.val, 0) != PASS || count > 0;
-    if (replica.op == WILDCARD && count > 0 && !tag.isPass(row)) {//nodeRole:'!overseer', strict:false
-      result.set(FAIL);
-    } else if (checkCount && !replica.isPass(count)) {
-      if (tag.op != WILDCARD && tag.isPass(row)) {
-        result.set(FAIL);
-      } else {
-        result.set(FAIL);
+
+  private Map<String, Map<String, Map<String, AtomicInteger>>> computeReplicaCounts(List<Row> allRows) {
+    Map<String, Map<String, Map<String, AtomicInteger>>> replicaCount = new HashMap<>();
+    for (Row row : allRows)
+      for (Map.Entry<String, Map<String, List<ReplicaInfo>>> colls : row.replicaInfo.entrySet()) {
+        String collectionName = colls.getKey();
+        if (!collection.isPass(collectionName)) continue;
+        replicaCount.putIfAbsent(collectionName, new HashMap<>());
+        Map<String, Map<String, AtomicInteger>> collMap = replicaCount.get(collectionName);
+        for (Map.Entry<String, List<ReplicaInfo>> shards : colls.getValue().entrySet()) {
+          String shardName = shards.getKey();
+          if (ANY.equals(shard.val)) shardName = ANY;
+          if (!shard.isPass(shardName)) break;
+          collMap.putIfAbsent(shardName, new HashMap<>());
+          Map<String, AtomicInteger> tagVsCount = collMap.get(shardName);
+          AtomicInteger count = null;
+          Object tagVal = row.getVal(tag.name);
+          if (tag.isPass(tagVal)) {
+            tagVsCount.put(String.valueOf(tagVal), count = tagVsCount.getOrDefault(tagVal, new AtomicInteger()));
+            count.addAndGet(shards.getValue().size());
+          }
+        }
       }
-    }
+    return replicaCount;
   }
 
   public boolean isStrict() {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/07e630b7/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/MoveReplicaSuggester.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/MoveReplicaSuggester.java b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/MoveReplicaSuggester.java
index bcc039c..2c295e2 100644
--- a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/MoveReplicaSuggester.java
+++ b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/MoveReplicaSuggester.java
@@ -17,19 +17,13 @@
 
 package org.apache.solr.cloud.autoscaling;
 
-import java.util.Map;
+import java.util.List;
 
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.autoscaling.Clause.Violation;
 import org.apache.solr.cloud.autoscaling.Policy.Suggester;
 import org.apache.solr.common.util.Pair;
-import org.apache.solr.common.util.Utils;
-
-import static org.apache.solr.common.cloud.ZkStateReader.COLLECTION_PROP;
-import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP;
-import static org.apache.solr.common.params.CollectionParams.CollectionAction.MOVEREPLICA;
-import static org.apache.solr.common.params.CoreAdminParams.NODE;
-import static org.apache.solr.common.params.CoreAdminParams.REPLICA;
 
 public class MoveReplicaSuggester extends Suggester {
 
@@ -53,29 +47,21 @@ public class MoveReplicaSuggester extends Suggester {
         continue;
       }
       tmpRow.violations.clear();
-      for (Clause clause : session.expandedClauses) {
-        if (strict || clause.strict) {
-          clause.test(tmpRow);
-        }
-      }
+
       final int i = getMatrix().indexOf(fromRow);
-      if (tmpRow.violations.isEmpty()) {
-        for (int j = getMatrix().size() - 1; j > i; j--) {
-          Row targetRow = getMatrix().get(j);
-          if (!isAllowed(targetRow.node, Hint.TARGET_NODE)) continue;
-          targetRow = targetRow.addReplica(coll, shard);
-          targetRow.violations.clear();
-          for (Clause clause : session.expandedClauses) {
-            if (strict || clause.strict) clause.test(targetRow);
-          }
-          if (targetRow.violations.isEmpty()) {
-            getMatrix().set(i, getMatrix().get(i).removeReplica(coll, shard).first());
-            getMatrix().set(j, getMatrix().get(j).addReplica(coll, shard));
-            return new CollectionAdminRequest.MoveReplica(
-                coll,
-                pair.second().name,
-                targetRow.node);
-          }
+      for (int j = getMatrix().size() - 1; j > i; j--) {
+        Row targetRow = getMatrix().get(j);
+        if (!isAllowed(targetRow.node, Hint.TARGET_NODE)) continue;
+        targetRow = targetRow.addReplica(coll, shard);
+        targetRow.violations.clear();
+        List<Violation> errs = testChangedRow(strict, getModifiedMatrix(getModifiedMatrix(getMatrix(), tmpRow, i), targetRow, j));
+        if (!containsNewErrors(errs)) {
+          getMatrix().set(i, getMatrix().get(i).removeReplica(coll, shard).first());
+          getMatrix().set(j, getMatrix().get(j).addReplica(coll, shard));
+          return new CollectionAdminRequest.MoveReplica(
+              coll,
+              pair.second().name,
+              targetRow.node);
         }
       }
     }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/07e630b7/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Operand.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Operand.java b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Operand.java
index 13c2589..6ff1ef5 100644
--- a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Operand.java
+++ b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Operand.java
@@ -85,7 +85,7 @@ public enum Operand {
     return operand + expectedVal.toString();
   }
 
-  Object checkNumeric(Object val) {
+  Integer checkNumeric(Object val) {
     if (val == null) return null;
     try {
       return Integer.parseInt(val.toString());

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/07e630b7/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Policy.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Policy.java b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Policy.java
index 69cb0f1..11a6250 100644
--- a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Policy.java
+++ b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Policy.java
@@ -34,6 +34,7 @@ import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
 import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.cloud.autoscaling.Clause.Violation;
 import org.apache.solr.common.IteratorWriter;
 import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.params.CollectionParams.CollectionAction;
@@ -122,6 +123,7 @@ public class Policy implements MapWriter {
     final List<Row> matrix;
     Set<String> collections = new HashSet<>();
     List<Clause> expandedClauses;
+    List<Violation> violations = new ArrayList<>();
     private List<String> paramsOfInterest;
 
     private Session(List<String> nodes, ClusterDataProvider dataProvider,
@@ -199,16 +201,13 @@ public class Policy implements MapWriter {
       }
 
       for (Clause clause : expandedClauses) {
-          for (Row row : matrix) {
-            clause.test(row);
-          }
-        }
+        List<Violation> errs = clause.test(matrix);
+        violations.addAll(errs);
+      }
     }
 
-    public Map<String, List<Clause>> getViolations() {
-      return matrix.stream()
-          .filter(row -> !row.violations.isEmpty())
-          .collect(Collectors.toMap(r -> r.node, r -> r.violations));
+    public List<Violation> getViolations() {
+      return violations;
     }
 
     public Suggester getSuggester(CollectionAction action) {
@@ -302,6 +301,7 @@ public class Policy implements MapWriter {
     protected final EnumMap<Hint, Object> hints = new EnumMap<>(Hint.class);
     Policy.Session session;
     SolrRequest operation;
+    protected List<Violation> originalViolations = new ArrayList<>();
     private boolean isInitialized = false;
 
     private void _init(Session session) {
@@ -318,6 +318,8 @@ public class Policy implements MapWriter {
 
     public SolrRequest getOperation() {
       if (!isInitialized) {
+        session.applyRules();
+        originalViolations.addAll(session.getViolations());
         this.operation = init();
         isInitialized = true;
       }
@@ -332,6 +334,13 @@ public class Policy implements MapWriter {
       return session.matrix;
 
     }
+    boolean containsNewErrors(List<Clause.Violation> errs){
+      for (Clause.Violation err : errs) {
+        if(!originalViolations.contains(err)) return true;
+      }
+      return false;
+    }
+
 
     List<Pair<ReplicaInfo, Row>> getValidReplicas(boolean sortDesc, boolean isSource, int until) {
       List<Pair<Policy.ReplicaInfo, Row>> allPossibleReplicas = new ArrayList<>();
@@ -356,6 +365,24 @@ public class Policy implements MapWriter {
         }
       }
     }
+    protected List<Violation> testChangedRow(boolean strict,List<Row> rows) {
+      List<Violation> errors = new ArrayList<>();
+      for (Clause clause : session.expandedClauses) {
+        if (strict || clause.strict) {
+          List<Violation> errs = clause.test(rows);
+          if (!errs.isEmpty()) {
+            errors.addAll(errs);
+          }
+        }
+      }
+      return errors;
+    }
+
+    ArrayList<Row> getModifiedMatrix(List<Row> matrix ,Row tmpRow, int i) {
+      ArrayList<Row> copy = new ArrayList<>(matrix);
+      copy.set(i, tmpRow);
+      return copy;
+    }
 
     protected boolean isAllowed(Object v, Hint hint) {
       Object hintVal = hints.get(hint);

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/07e630b7/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/PolicyHelper.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/PolicyHelper.java b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/PolicyHelper.java
index e7ad25e..2012be8 100644
--- a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/PolicyHelper.java
+++ b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/PolicyHelper.java
@@ -23,6 +23,9 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.function.BiConsumer;
+import java.util.function.Consumer;
+import java.util.function.Predicate;
 
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.common.SolrException;
@@ -51,7 +54,13 @@ public class PolicyHelper {
 
         @Override
         public Map<String, Map<String, List<Policy.ReplicaInfo>>> getReplicaInfo(String node, Collection<String> keys) {
-          return delegate.getReplicaInfo(node, keys);
+          Map<String, Map<String, List<Policy.ReplicaInfo>>> replicaInfo = delegate.getReplicaInfo(node, keys);
+          for (String s : optionalPolicyMapping.keySet()) {
+            if (!replicaInfo.containsKey(s)) {
+              replicaInfo.put(s, new HashMap<>());
+            }
+          }
+          return replicaInfo;
         }
 
         @Override
@@ -79,10 +88,10 @@ public class PolicyHelper {
             .hint(Hint.SHARD, shardName);
         SolrRequest op = suggester.getOperation();
         if (op == null) {
-          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "No node can satisfy the rules "+ Utils.toJSONString(policy));
+          throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "No node can satisfy the rules "+ Utils.toJSONString(Utils.getDeepCopy(session.expandedClauses, 4, true)));
         }
         session = suggester.getSession();
-        positionMapping.get(shardName).add((String) op.getParams().get(CoreAdminParams.NODE));
+        positionMapping.get(shardName).add(op.getParams().get(CoreAdminParams.NODE));
       }
     }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/07e630b7/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Row.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Row.java b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Row.java
index d5f94c9..b9304ba 100644
--- a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Row.java
+++ b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Row.java
@@ -93,9 +93,12 @@ class Row implements MapWriter {
     Row row = copy();
     Map<String, List<ReplicaInfo>> c = row.replicaInfo.get(coll);
     if (c == null) row.replicaInfo.put(coll, c = new HashMap<>());
-    List<ReplicaInfo> s = c.get(shard);
-    if (s == null) c.put(shard, s = new ArrayList<>());
-    s.add(new ReplicaInfo(""+new Random().nextInt(1000)+1000,coll,shard, new HashMap<>()));
+    List<ReplicaInfo> replicas = c.get(shard);
+    if (replicas == null) c.put(shard, replicas = new ArrayList<>());
+    replicas.add(new ReplicaInfo("" + new Random().nextInt(1000) + 1000, coll, shard, new HashMap<>()));
+    for (Cell cell : row.cells) {
+      if (cell.name.equals("cores")) cell.val = ((Number) cell.val).intValue() + 1;
+    }
     return row;
 
   }
@@ -103,10 +106,10 @@ class Row implements MapWriter {
   Pair<Row, ReplicaInfo> removeReplica(String coll, String shard) {
     Row row = copy();
     Map<String, List<ReplicaInfo>> c = row.replicaInfo.get(coll);
-    if(c == null) return null;
+    if (c == null) return null;
     List<ReplicaInfo> s = c.get(shard);
     if (s == null || s.isEmpty()) return null;
-    return new Pair(row,s.remove(0));
+    return new Pair(row, s.remove(0));
 
   }
 }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/07e630b7/solr/solrj/src/test/org/apache/solr/cloud/autoscaling/TestPolicy.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/cloud/autoscaling/TestPolicy.java b/solr/solrj/src/test/org/apache/solr/cloud/autoscaling/TestPolicy.java
index def2d47..9d1f12a 100644
--- a/solr/solrj/src/test/org/apache/solr/cloud/autoscaling/TestPolicy.java
+++ b/solr/solrj/src/test/org/apache/solr/cloud/autoscaling/TestPolicy.java
@@ -28,7 +28,9 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 
+import com.google.common.collect.ImmutableList;
 import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest.MoveReplica;
 import org.apache.solr.cloud.autoscaling.Policy.Suggester.Hint;
@@ -76,7 +78,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
         "    { 'minimize': 'cores', 'precision': 50}" +
         "  ]," +
         "  'cluster-policy': [" +
-        "    { 'replica': '#ANY', 'nodeRole': '!overseer'}," +
+        "    { 'replica': 0, 'nodeRole': 'overseer'}," +
         "    { 'replica': '<2', 'shard': '#EACH', 'node': '#ANY'}" +
         "  ]," +
         "  'policies': {" +
@@ -92,17 +94,17 @@ public class TestPolicy extends SolrTestCaseJ4 {
     Collections.sort(clauses);
     assertEquals(clauses.size(), 4);
     assertEquals("1", String.valueOf(clauses.get(0).original.get("replica")));
-    assertEquals("<2", String.valueOf(clauses.get(1).original.get("replica")));
+    assertEquals("0", String.valueOf(clauses.get(1).original.get("replica")));
     assertEquals("#ANY", clauses.get(3).original.get("shard"));
-    assertEquals("rack1",clauses.get(1).original.get("rack"));
-    assertEquals("!overseer", clauses.get(2).original.get("nodeRole"));
+    assertEquals("rack1",clauses.get(2).original.get("rack"));
+    assertEquals("overseer", clauses.get(1).original.get("nodeRole"));
   }
 
 
   public void testConditionsSort(){
     String rules = "{" +
         "    'cluster-policy':[" +
-        "      { 'nodeRole':'!overseer', 'strict':false}," +
+        "      { 'nodeRole':'overseer', replica: 0,  'strict':false}," +
         "      { 'replica':'<1', 'node':'node3', 'shard':'#EACH'}," +
         "      { 'replica':'<2', 'node':'#ANY', 'shard':'#EACH'}," +
         "      { 'replica':1, 'rack':'rack1'}]" +
@@ -110,7 +112,8 @@ public class TestPolicy extends SolrTestCaseJ4 {
     Policy p = new Policy((Map<String, Object>) Utils.fromJSONString(rules));
     List<Clause> clauses = new ArrayList<>(p.getClusterPolicy());
     Collections.sort(clauses);
-    assertEquals("rack", clauses.get(0).tag.name);
+    assertEquals("nodeRole", clauses.get(0).tag.name);
+    assertEquals("rack", clauses.get(1).tag.name);
   }
   public static String clusterState = "{'gettingstarted':{" +
       "    'router':{'name':'compositeId'}," +
@@ -157,7 +160,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
   public void testRules() throws IOException {
     String rules = "{" +
         "cluster-policy:[" +
-        "{nodeRole:'!overseer', strict:false}," +
+        "{nodeRole:'overseer',replica : 0 , strict:false}," +
         "{replica:'<1',node:node3}," +
         "{replica:'<2',node:'#ANY', shard:'#EACH'}]," +
         " cluster-preferences:[" +
@@ -183,7 +186,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
     assertEquals("node2", l.get(3).node);
 
 
-    Map<String, List<Clause>> violations = session.getViolations();
+    /*Map<String, List<Clause>> violations = session.getViolations();
     System.out.println(Utils.getDeepCopy(violations, 6));
     assertEquals(3, violations.size());
     List<Clause> v = violations.get("node4");
@@ -197,7 +200,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
     assertNotNull(v);
     assertEquals(v.get(0).replica.op, Operand.LESS_THAN);
     assertEquals(v.get(0).replica.val, 1);
-    assertEquals(v.get(0).tag.val, "node3");
+    assertEquals(v.get(0).tag.val, "node3");*/
     Policy.Suggester suggester = session.getSuggester(ADDREPLICA)
         .hint(Hint.COLL, "gettingstarted")
         .hint(Hint.SHARD, "r1");
@@ -212,11 +215,11 @@ public class TestPolicy extends SolrTestCaseJ4 {
         "node4:{cores:8, freedisk: 375, heap:16900, nodeRole:overseer}" +
         "}");
     session = policy.createSession(getClusterDataProvider(nodeValues, clusterState));
-    operation = session.getSuggester(MOVEREPLICA)
+    SolrRequest opReq = session.getSuggester(MOVEREPLICA)
         .hint(Hint.TARGET_NODE, "node5")
-        .getOperation()
-        .getParams();
-    assertEquals("node5", operation.get("targetNode"));
+        .getOperation();
+    assertNotNull(opReq);
+    assertEquals("node5", opReq.getParams().get("targetNode"));
 
 
   }
@@ -226,7 +229,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
         "      'cluster-policy':[" +
         "      {'cores':'<10','node':'#ANY'}," +
         "      {'replica':'<3','shard':'#EACH','node':'#ANY'}," +
-        "      {'nodeRole':'!overseer','replica':'#ANY'}]," +
+        "      {'nodeRole':'overseer','replica':'0'}]," +
         "      'cluster-preferences':[" +
         "      {'minimize':'cores', 'precision':3}," +
         "      {'maximize':'freedisk','precision':100}]}";
@@ -372,21 +375,22 @@ public class TestPolicy extends SolrTestCaseJ4 {
       };
   }
 
-/*  public void testMultiReplicaPlacement() {
+  public void testMultiReplicaPlacement() {
     String autoScaleJson ="{" +
         "  'cluster-preferences': [" +
-        "    { 'minimize': 'freedisk', 'precision': 50}" +
+        "    { maximize : freedisk , precision: 50}," +
+        "    { minimize : cores, precision: 2}" +
         "  ]," +
         "  'cluster-policy': [" +
-        "    { 'nodeRole': '!overseer'}," +
-        "    { 'replica': '<2', 'shard': '#EACH', 'node': '#ANY'" +
+        "    { replica : '0' , 'nodeRole': 'overseer'}," +
+        "    { 'replica': '<2', 'shard': '#ANY', 'node': '#ANY'" +
         "    }" +
         "  ]," +
         "  'policies': {" +
         "    'policy1': [" +
-        "      { 'replica': '<2', 'shard': '#ANY', 'node': '#ANY'}," +
-        "      { 'replica': '<2', 'shard': '#EACH', 'rack': 'rack1'}," +
-        "      { 'replica': '1', 'sysprop.fs': 'ssd', 'shard': '#EACH'}" +
+        "      { 'replica': '<2', 'shard': '#EACH', 'node': '#ANY'}," +
+        "      { 'replica': '<2', 'shard': '#EACH', 'rack': 'rack1'}" +
+//        "      { 'replica': '1', 'sysprop.fs': 'ssd', 'shard': '#EACH'}" +
         "    ]" +
         "  }" +
         "}";
@@ -396,7 +400,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
         "node1:{cores:12, freedisk: 334, heap:10480, rack:rack3}," +
         "node2:{cores:4, freedisk: 749, heap:6873, sysprop.fs : ssd, rack:rack1}," +
         "node3:{cores:7, freedisk: 262, heap:7834, rack:rack4}," +
-        "node4:{cores:8, freedisk: 375, heap:16900, nodeRole:overseer, rack:rack2}" +
+        "node4:{cores:0, freedisk: 900, heap:16900, nodeRole:overseer, rack:rack2}" +
         "}");
 
     ClusterDataProvider dataProvider = new ClusterDataProvider() {
@@ -425,10 +429,11 @@ public class TestPolicy extends SolrTestCaseJ4 {
     Map<String, List<String>> locations = PolicyHelper.getReplicaLocations(
         "newColl", (Map<String, Object>) Utils.fromJSONString(autoScaleJson),
         dataProvider, Collections.singletonMap("newColl", "policy1"), Arrays.asList("shard1", "shard2"), 3);
-    System.out.println(Utils.toJSONString(locations));
+    assertTrue(locations.get("shard1").containsAll(ImmutableList.of("node2","node1","node3")));
+    assertTrue(locations.get("shard2").containsAll(ImmutableList.of("node2","node1","node3")));
 
 
-  }*/
+  }
 
   public static Map<String, Map<String, List<Policy.ReplicaInfo>>> getReplicaDetails(String node, String s) {
     ValidatingJsonMap m = ValidatingJsonMap