You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ab...@apache.org on 2018/04/10 14:12:29 UTC

[50/50] lucene-solr:jira/solr-12181: SOLR-12181: Support mixed bytes / docs bounds.

SOLR-12181: Support mixed bytes / docs bounds.


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

Branch: refs/heads/jira/solr-12181
Commit: 751987d53df34884c71d8ba8e1a76f9e48290f1c
Parents: 221f749
Author: Andrzej Bialecki <ab...@apache.org>
Authored: Tue Apr 10 16:11:07 2018 +0200
Committer: Andrzej Bialecki <ab...@apache.org>
Committed: Tue Apr 10 16:11:07 2018 +0200

----------------------------------------------------------------------
 .../cloud/autoscaling/IndexSizeTrigger.java     | 121 ++++++++++++-------
 .../cloud/autoscaling/SearchRateTrigger.java    |   5 +-
 .../org/apache/solr/cloud/CloudTestUtils.java   |  10 +-
 .../cloud/autoscaling/IndexSizeTriggerTest.java |  19 +--
 .../ScheduledMaintenanceTriggerTest.java        |   2 +-
 .../sim/SimClusterStateProvider.java            |  37 ++++--
 .../autoscaling/sim/SimSolrCloudTestCase.java   |   6 -
 .../cloud/autoscaling/sim/TestLargeCluster.java |  14 ++-
 .../cloud/autoscaling/SplitShardSuggester.java  |   2 +-
 9 files changed, 130 insertions(+), 86 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/751987d5/solr/core/src/java/org/apache/solr/cloud/autoscaling/IndexSizeTrigger.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/IndexSizeTrigger.java b/solr/core/src/java/org/apache/solr/cloud/autoscaling/IndexSizeTrigger.java
index 7bfda9a..725da62 100644
--- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/IndexSizeTrigger.java
+++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/IndexSizeTrigger.java
@@ -20,12 +20,10 @@ package org.apache.solr.cloud.autoscaling;
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
@@ -55,64 +53,85 @@ import org.slf4j.LoggerFactory;
 public class IndexSizeTrigger extends TriggerBase {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
-  public static final String ABOVE_PROP = "above";
+  public static final String ABOVE_BYTES_PROP = "aboveBytes";
+  public static final String ABOVE_DOCS_PROP = "aboveDocs";
   public static final String ABOVE_OP_PROP = "aboveOp";
-  public static final String BELOW_PROP = "below";
+  public static final String BELOW_BYTES_PROP = "belowBytes";
+  public static final String BELOW_DOCS_PROP = "belowDocs";
   public static final String BELOW_OP_PROP = "belowOp";
-  public static final String UNIT_PROP = "unit";
   public static final String COLLECTIONS_PROP = "collections";
 
-  public static final String SIZE_PROP = "__indexSize__";
+  public static final String BYTES_SIZE_PROP = "__bytesSize__";
+  public static final String DOCS_SIZE_PROP = "__docsSize__";
   public static final String ABOVE_SIZE_PROP = "aboveSize";
   public static final String BELOW_SIZE_PROP = "belowSize";
 
   public enum Unit { bytes, docs }
 
-  private long above, below;
+  private long aboveBytes, aboveDocs, belowBytes, belowDocs;
   private CollectionParams.CollectionAction aboveOp, belowOp;
-  private Unit unit;
   private final Set<String> collections = new HashSet<>();
   private final Map<String, Long> lastEventMap = new ConcurrentHashMap<>();
 
   public IndexSizeTrigger(String name) {
     super(TriggerEventType.INDEXSIZE, name);
     TriggerUtils.validProperties(validProperties,
-        ABOVE_PROP, BELOW_PROP, UNIT_PROP, COLLECTIONS_PROP);
+        ABOVE_BYTES_PROP, ABOVE_DOCS_PROP, BELOW_BYTES_PROP, BELOW_DOCS_PROP, COLLECTIONS_PROP);
   }
 
   @Override
   public void configure(SolrResourceLoader loader, SolrCloudManager cloudManager, Map<String, Object> properties) throws TriggerValidationException {
     super.configure(loader, cloudManager, properties);
-    String unitStr = String.valueOf(properties.getOrDefault(UNIT_PROP, Unit.bytes));
+    String aboveStr = String.valueOf(properties.getOrDefault(ABOVE_BYTES_PROP, Long.MAX_VALUE));
+    String belowStr = String.valueOf(properties.getOrDefault(BELOW_BYTES_PROP, -1));
     try {
-      unit = Unit.valueOf(unitStr.toLowerCase(Locale.ROOT));
+      aboveBytes = Long.parseLong(aboveStr);
+      if (aboveBytes <= 0) {
+        throw new Exception("value must be > 0");
+      }
     } catch (Exception e) {
-      throw new TriggerValidationException(getName(), UNIT_PROP, "invalid unit, must be one of " + Arrays.toString(Unit.values()));
+      throw new TriggerValidationException(getName(), ABOVE_BYTES_PROP, "invalid value '" + aboveStr + "': " + e.toString());
+    }
+    try {
+      belowBytes = Long.parseLong(belowStr);
+      if (belowBytes < 0) {
+        belowBytes = -1;
+      }
+    } catch (Exception e) {
+      throw new TriggerValidationException(getName(), BELOW_BYTES_PROP, "invalid value '" + belowStr + "': " + e.toString());
+    }
+    // below must be at least 2x smaller than above, otherwise splitting a shard
+    // would immediately put the shard below the threshold and cause the mergeshards action
+    if (belowBytes > 0 && (belowBytes * 2 > aboveBytes)) {
+      throw new TriggerValidationException(getName(), BELOW_BYTES_PROP,
+          "invalid value " + belowBytes + ", should be less than half of '" + ABOVE_BYTES_PROP + "' value, which is " + aboveBytes);
     }
-    String aboveStr = String.valueOf(properties.getOrDefault(ABOVE_PROP, Long.MAX_VALUE));
-    String belowStr = String.valueOf(properties.getOrDefault(BELOW_PROP, -1));
+    // do the same for docs bounds
+    aboveStr = String.valueOf(properties.getOrDefault(ABOVE_DOCS_PROP, Long.MAX_VALUE));
+    belowStr = String.valueOf(properties.getOrDefault(BELOW_DOCS_PROP, -1));
     try {
-      above = Long.parseLong(aboveStr);
-      if (above <= 0) {
+      aboveDocs = Long.parseLong(aboveStr);
+      if (aboveDocs <= 0) {
         throw new Exception("value must be > 0");
       }
     } catch (Exception e) {
-      throw new TriggerValidationException(getName(), ABOVE_PROP, "invalid value '" + aboveStr + "': " + e.toString());
+      throw new TriggerValidationException(getName(), ABOVE_DOCS_PROP, "invalid value '" + aboveStr + "': " + e.toString());
     }
     try {
-      below = Long.parseLong(belowStr);
-      if (below < 0) {
-        below = -1;
+      belowDocs = Long.parseLong(belowStr);
+      if (belowDocs < 0) {
+        belowDocs = -1;
       }
     } catch (Exception e) {
-      throw new TriggerValidationException(getName(), BELOW_PROP, "invalid value '" + belowStr + "': " + e.toString());
+      throw new TriggerValidationException(getName(), BELOW_DOCS_PROP, "invalid value '" + belowStr + "': " + e.toString());
     }
     // below must be at least 2x smaller than above, otherwise splitting a shard
     // would immediately put the shard below the threshold and cause the mergeshards action
-    if (below > 0 && (below * 2 > above)) {
-      throw new TriggerValidationException(getName(), BELOW_PROP,
-          "invalid value " + below + ", should be less than half of '" + ABOVE_PROP + "' value, which is " + above);
+    if (belowDocs > 0 && (belowDocs * 2 > aboveDocs)) {
+      throw new TriggerValidationException(getName(), BELOW_DOCS_PROP,
+          "invalid value " + belowDocs + ", should be less than half of '" + ABOVE_DOCS_PROP + "' value, which is " + aboveDocs);
     }
+
     String collectionsString = (String) properties.get(COLLECTIONS_PROP);
     if (collectionsString != null && !collectionsString.isEmpty()) {
       collections.addAll(StrUtils.splitSmart(collectionsString, ','));
@@ -209,17 +228,9 @@ public class IndexSizeTrigger extends TriggerBase {
               replicaName = info.getName(); // which is actually coreNode name...
             }
             String registry = SolrCoreMetricManager.createRegistryName(true, coll, sh, replicaName, null);
-            String tag;
-            switch (unit) {
-              case bytes:
-                tag = "metrics:" + registry + ":INDEX.size";
-                break;
-              case docs:
-                tag = "metrics:" + registry + ":SEARCHER.searcher.numDocs";
-                break;
-              default:
-                throw new UnsupportedOperationException("Unit " + unit + " not supported");
-            }
+            String tag = "metrics:" + registry + ":INDEX.sizeInBytes";
+            metricTags.put(tag, info);
+            tag = "metrics:" + registry + ":SEARCHER.searcher.numDocs";
             metricTags.put(tag, info);
           });
         });
@@ -228,7 +239,7 @@ public class IndexSizeTrigger extends TriggerBase {
         }
         Map<String, Object> sizes = cloudManager.getNodeStateProvider().getNodeValues(node, metricTags.keySet());
         sizes.forEach((tag, size) -> {
-          ReplicaInfo info = metricTags.get(tag);
+          final ReplicaInfo info = metricTags.get(tag);
           if (info == null) {
             log.warn("Missing replica info for response tag " + tag);
           } else {
@@ -237,9 +248,13 @@ public class IndexSizeTrigger extends TriggerBase {
               log.warn("invalid size value - not a number: '" + size + "' is " + size.getClass().getName());
               return;
             }
-            info = (ReplicaInfo)info.clone();
-            info.getVariables().put(SIZE_PROP, ((Number) size).longValue());
-            currentSizes.put(info.getCore(), info);
+
+            ReplicaInfo currentInfo = currentSizes.computeIfAbsent(info.getCore(), k -> (ReplicaInfo)info.clone());
+            if (tag.contains("INDEX")) {
+              currentInfo.getVariables().put(BYTES_SIZE_PROP, ((Number) size).longValue());
+            } else {
+              currentInfo.getVariables().put(DOCS_SIZE_PROP, ((Number) size).longValue());
+            }
           }
         });
       }
@@ -255,22 +270,30 @@ public class IndexSizeTrigger extends TriggerBase {
     // collection / list(info)
     Map<String, List<ReplicaInfo>> aboveSize = new HashMap<>();
     currentSizes.entrySet().stream()
-        .filter(e -> (Long)e.getValue().getVariable(SIZE_PROP) > above &&
-            waitForElapsed(e.getKey(), now, lastEventMap))
+        .filter(e -> (
+            (Long)e.getValue().getVariable(BYTES_SIZE_PROP) > aboveBytes ||
+            (Long)e.getValue().getVariable(DOCS_SIZE_PROP) > aboveDocs
+            ) && waitForElapsed(e.getKey(), now, lastEventMap))
         .forEach(e -> {
           ReplicaInfo info = e.getValue();
           List<ReplicaInfo> infos = aboveSize.computeIfAbsent(info.getCollection(), c -> new ArrayList<>());
-          infos.add(info);
+          if (!infos.contains(info)) {
+            infos.add(info);
+          }
         });
     // collection / list(info)
     Map<String, List<ReplicaInfo>> belowSize = new HashMap<>();
     currentSizes.entrySet().stream()
-        .filter(e -> (Long)e.getValue().getVariable(SIZE_PROP) < below &&
-            waitForElapsed(e.getKey(), now, lastEventMap))
+        .filter(e -> (
+            (Long)e.getValue().getVariable(BYTES_SIZE_PROP) < belowBytes ||
+            (Long)e.getValue().getVariable(DOCS_SIZE_PROP) < belowDocs
+            ) && waitForElapsed(e.getKey(), now, lastEventMap))
         .forEach(e -> {
           ReplicaInfo info = e.getValue();
           List<ReplicaInfo> infos = belowSize.computeIfAbsent(info.getCollection(), c -> new ArrayList<>());
-          infos.add(info);
+          if (!infos.contains(info)) {
+            infos.add(info);
+          }
         });
 
     if (aboveSize.isEmpty() && belowSize.isEmpty()) {
@@ -299,7 +322,11 @@ public class IndexSizeTrigger extends TriggerBase {
       }
       // sort by increasing size
       replicas.sort((r1, r2) -> {
-        long delta = (Long) r1.getVariable(SIZE_PROP) - (Long) r2.getVariable(SIZE_PROP);
+        // XXX this is not quite correct - if BYTES_SIZE_PROP decided that replica got here
+        // then we should be sorting by BYTES_SIZE_PROP. However, since DOCS and BYTES are
+        // loosely correlated it's simpler to sort just by docs (which better reflects the "too small"
+        // condition than index size, due to possibly existing deleted docs that still occupy space)
+        long delta = (Long) r1.getVariable(DOCS_SIZE_PROP) - (Long) r2.getVariable(DOCS_SIZE_PROP);
         if (delta > 0) {
           return 1;
         } else if (delta < 0) {
@@ -308,7 +335,7 @@ public class IndexSizeTrigger extends TriggerBase {
           return 0;
         }
       });
-      // take top two smallest
+      // take the top two smallest
       TriggerEvent.Op op = new TriggerEvent.Op(belowOp);
       op.addHint(Suggester.Hint.COLL_SHARD, new Pair(coll, replicas.get(0).getShard()));
       op.addHint(Suggester.Hint.COLL_SHARD, new Pair(coll, replicas.get(1).getShard()));

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/751987d5/solr/core/src/java/org/apache/solr/cloud/autoscaling/SearchRateTrigger.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/SearchRateTrigger.java b/solr/core/src/java/org/apache/solr/cloud/autoscaling/SearchRateTrigger.java
index 00bc6d8..02a2d0c 100644
--- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/SearchRateTrigger.java
+++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/SearchRateTrigger.java
@@ -181,10 +181,11 @@ public class SearchRateTrigger extends TriggerBase {
         } else {
           Map<String, List<ReplicaInfo>> perCollection = collectionRates.computeIfAbsent(info.getCollection(), s -> new HashMap<>());
           List<ReplicaInfo> perShard = perCollection.computeIfAbsent(info.getShard(), s -> new ArrayList<>());
-          info.getVariables().put(AutoScalingParams.RATE, rate);
+          info = (ReplicaInfo)info.clone();
+          info.getVariables().put(AutoScalingParams.RATE, ((Number)rate).doubleValue());
           perShard.add(info);
           AtomicDouble perNode = nodeRates.computeIfAbsent(node, s -> new AtomicDouble());
-          perNode.addAndGet((Double)rate);
+          perNode.addAndGet(((Number)rate).doubleValue());
         }
       });
     }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/751987d5/solr/core/src/test/org/apache/solr/cloud/CloudTestUtils.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/CloudTestUtils.java b/solr/core/src/test/org/apache/solr/cloud/CloudTestUtils.java
index d51e6ca..5590252 100644
--- a/solr/core/src/test/org/apache/solr/cloud/CloudTestUtils.java
+++ b/solr/core/src/test/org/apache/solr/cloud/CloudTestUtils.java
@@ -19,6 +19,7 @@ package org.apache.solr.cloud;
 
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
+import java.util.Collection;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -113,16 +114,21 @@ public class CloudTestUtils {
    * number of shards and replicas
    */
   public static CollectionStatePredicate clusterShape(int expectedShards, int expectedReplicas) {
+    return clusterShape(expectedShards, expectedReplicas, false);
+  }
+
+  public static CollectionStatePredicate clusterShape(int expectedShards, int expectedReplicas, boolean withInactive) {
     return (liveNodes, collectionState) -> {
       if (collectionState == null) {
         log.debug("-- null collection");
         return false;
       }
-      if (collectionState.getActiveSlices().size() != expectedShards) {
+      Collection<Slice> slices = withInactive ? collectionState.getSlices() : collectionState.getActiveSlices();
+      if (slices.size() != expectedShards) {
         log.debug("-- wrong number of active slices, expected=" + expectedShards + ", found=" + collectionState.getSlices().size());
         return false;
       }
-      for (Slice slice : collectionState.getActiveSlices()) {
+      for (Slice slice : slices) {
         int activeReplicas = 0;
         for (Replica replica : slice) {
           if (replica.isActive(liveNodes))

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/751987d5/solr/core/src/test/org/apache/solr/cloud/autoscaling/IndexSizeTriggerTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/IndexSizeTriggerTest.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/IndexSizeTriggerTest.java
index 19edfbf..a24b447 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/IndexSizeTriggerTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/IndexSizeTriggerTest.java
@@ -19,7 +19,6 @@ package org.apache.solr.cloud.autoscaling;
 
 import java.lang.invoke.MethodHandles;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -41,9 +40,7 @@ import org.apache.solr.cloud.CloudTestUtils;
 import org.apache.solr.cloud.SolrCloudTestCase;
 import org.apache.solr.cloud.autoscaling.sim.SimCloudManager;
 import org.apache.solr.common.SolrInputDocument;
-import org.apache.solr.common.cloud.ClusterState;
 import org.apache.solr.common.cloud.ZkNodeProps;
-import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.params.CollectionParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.Pair;
@@ -53,7 +50,6 @@ import org.apache.solr.core.SolrResourceLoader;
 import org.apache.solr.util.LogLevel;
 import org.junit.After;
 import org.junit.AfterClass;
-import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.slf4j.Logger;
@@ -246,9 +242,8 @@ public class IndexSizeTriggerTest extends SolrCloudTestCase {
         "'name' : 'index_size_trigger'," +
         "'event' : 'indexSize'," +
         "'waitFor' : '" + waitForSeconds + "s'," +
-        "'unit' : 'docs'," +
-        "'above' : 10," +
-        "'below' : 4," +
+        "'aboveDocs' : 10," +
+        "'belowDocs' : 4," +
         "'enabled' : true," +
         "'actions' : [{'name' : 'compute_plan', 'class' : 'solr.ComputePlanAction'}," +
         "{'name' : 'execute_plan', 'class' : '" + ExecutePlanAction.class.getName() + "'}]" +
@@ -354,9 +349,8 @@ public class IndexSizeTriggerTest extends SolrCloudTestCase {
         "'name' : 'index_size_trigger'," +
         "'event' : 'indexSize'," +
         "'waitFor' : '" + waitForSeconds + "s'," +
-        "'unit' : 'docs'," +
-        "'above' : 40," +
-        "'below' : 4," +
+        "'aboveDocs' : 40," +
+        "'belowDocs' : 4," +
         "'enabled' : true," +
         "'actions' : [{'name' : 'compute_plan', 'class' : 'solr.ComputePlanAction'}," +
         "{'name' : 'execute_plan', 'class' : '" + ExecutePlanAction.class.getName() + "'}]" +
@@ -438,9 +432,8 @@ public class IndexSizeTriggerTest extends SolrCloudTestCase {
     props.put("event", "indexSize");
     props.put("waitFor", waitForSeconds);
     props.put("enabled", true);
-    props.put(IndexSizeTrigger.UNIT_PROP, IndexSizeTrigger.Unit.docs.toString());
-    props.put(IndexSizeTrigger.ABOVE_PROP, 10);
-    props.put(IndexSizeTrigger.BELOW_PROP, 2);
+    props.put(IndexSizeTrigger.ABOVE_DOCS_PROP, 10);
+    props.put(IndexSizeTrigger.BELOW_DOCS_PROP, 2);
     List<Map<String, String>> actions = new ArrayList<>(3);
     Map<String, String> map = new HashMap<>(2);
     map.put("name", "compute_plan");

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/751987d5/solr/core/src/test/org/apache/solr/cloud/autoscaling/ScheduledMaintenanceTriggerTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/ScheduledMaintenanceTriggerTest.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/ScheduledMaintenanceTriggerTest.java
index ffcab4d..164db8f 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/ScheduledMaintenanceTriggerTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/ScheduledMaintenanceTriggerTest.java
@@ -165,7 +165,7 @@ public class ScheduledMaintenanceTriggerTest extends SolrCloudTestCase {
         .setShardName("shard1");
     split1.process(solrClient);
     CloudTestUtils.waitForState(cloudManager, "failed to split " + collection1, collection1,
-        CloudTestUtils.clusterShape(3, 1));
+        CloudTestUtils.clusterShape(3, 1, true));
 
     String setListenerCommand = "{" +
         "'set-listener' : " +

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/751987d5/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
index b8da5b0..9b3782a 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimClusterStateProvider.java
@@ -900,7 +900,12 @@ public class SimClusterStateProvider implements ClusterStateProvider {
     if (sessionWrapper != null) sessionWrapper.release();
 
     // adjust numDocs / deletedDocs / maxDoc
-    String numDocsStr = parentSlice.getLeader().getStr("SEARCHER.searcher.numDocs", "0");
+    Replica leader = parentSlice.getLeader();
+    // XXX leader election may not have happened yet - should we require it?
+    if (leader == null) {
+      leader = parentSlice.getReplicas().iterator().next();
+    }
+    String numDocsStr = leader.getStr("SEARCHER.searcher.numDocs", "0");
     long numDocs = Long.parseLong(numDocsStr);
     long newNumDocs = numDocs / subSlices.size();
     long remainder = numDocs % subSlices.size();
@@ -1004,21 +1009,22 @@ public class SimClusterStateProvider implements ClusterStateProvider {
   }
 
   /**
-   * Simulate an update by increasing replica metrics.
-   * <p>The following core metrics are updated:
+   * Simulate an update by modifying replica metrics.
+   * The following core metrics are updated:
    * <ul>
-   *   <li></li>
+   *   <li><code>SEARCHER.searcher.numDocs</code> - increased by added docs, decreased by deleteById and deleteByQuery</li>
+   *   <li><code>SEARCHER.searcher.deletedDocs</code> - decreased by deleteById and deleteByQuery by up to <code>numDocs</code></li>
+   *   <li><code>SEARCHER.searcher.maxDoc</code> - always increased by the number of added docs.</li>
    * </ul>
-   * </p>
-   * <p>IMPORTANT limitations:
+   * <p>IMPORTANT limitations:</p>
    * <ul>
    *   <li>document replacements are always counted as new docs</li>
-   *   <li>delete by ID always succeeds (unless there are 0 documents)</li>
-   *   <li>deleteByQuery never matches unless the query is <code>*:*</code></li>
-   * </ul></p>
-   * @param req
-   * @return
-   * @throws SolrException
+   *   <li>delete by ID always succeeds (unless numDocs == 0)</li>
+   *   <li>deleteByQuery is not supported unless the query is <code>*:*</code></li>
+   * </ul>
+   * @param req update request. This request MUST have the <code>collection</code> param set.
+   * @return {@link UpdateResponse}
+   * @throws SolrException on errors, such as nonexistent collection or unsupported deleteByQuery
    */
   public UpdateResponse simUpdate(UpdateRequest req) throws SolrException, InterruptedException, IOException {
     String collection = req.getCollection();
@@ -1028,7 +1034,8 @@ public class SimClusterStateProvider implements ClusterStateProvider {
     if (!simListCollections().contains(collection)) {
       throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Collection '" + collection + "' doesn't exist");
     }
-    // always reset first to get the current metrics
+    // always reset first to get the current metrics - it's easier than to keep matching
+    // Replica with ReplicaInfo where the current real counts are stored
     collectionsStatesRef.set(null);
     DocCollection coll = getClusterState().getCollection(collection);
     DocRouter router = coll.getRouter();
@@ -1041,6 +1048,7 @@ public class SimClusterStateProvider implements ClusterStateProvider {
       if (deletes != null && !deletes.isEmpty()) {
         for (String id : deletes) {
           Slice s = router.getTargetSlice(id, null, null, req.getParams(), coll);
+          // NOTE: we don't use getProperty because it uses PROPERTY_PROP_PREFIX
           String numDocsStr = s.getLeader().getStr("SEARCHER.searcher.numDocs");
           if (numDocsStr == null) {
             LOG.debug("-- no docs in " + s.getLeader());
@@ -1100,6 +1108,7 @@ public class SimClusterStateProvider implements ClusterStateProvider {
             simSetShardValue(collection, s.getName(), "SEARCHER.searcher.numDocs", 1, true, false);
             simSetShardValue(collection, s.getName(), "SEARCHER.searcher.maxDoc", 1, true, false);
             // Policy reuses this value and expects it to be in GB units!!!
+            // the idea here is to increase the index size by 500 bytes with each doc
             // simSetShardValue(collection, s.getName(), "INDEX.sizeInBytes", 500, true, false);
           } catch (Exception e) {
             throw new IOException(e);
@@ -1391,6 +1400,8 @@ public class SimClusterStateProvider implements ClusterStateProvider {
     return state;
   }
 
+  // this method uses a simple cache in collectionsStatesRef. Operations that modify
+  // cluster state should always reset this cache so that the changes become visible
   private Map<String, DocCollection> getCollectionStates() {
     Map<String, DocCollection> collectionStates = collectionsStatesRef.get();
     if (collectionStates != null) {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/751987d5/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimSolrCloudTestCase.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimSolrCloudTestCase.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimSolrCloudTestCase.java
index 1c56b74..757e297 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimSolrCloudTestCase.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/SimSolrCloudTestCase.java
@@ -22,15 +22,9 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
-import java.util.Locale;
-import java.util.Map;
-import java.util.Set;
-import java.util.TreeMap;
-import java.util.concurrent.atomic.AtomicInteger;
 import java.util.function.Predicate;
 
 import org.apache.solr.SolrTestCaseJ4;
-import org.apache.solr.client.solrj.cloud.autoscaling.ReplicaInfo;
 import org.apache.solr.common.cloud.DocCollection;
 import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.cloud.Slice;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/751987d5/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestLargeCluster.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestLargeCluster.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestLargeCluster.java
index 2c4d8d3..129f18c 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestLargeCluster.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestLargeCluster.java
@@ -592,7 +592,19 @@ public class TestLargeCluster extends SimSolrCloudTestCase {
     ops.forEach(op -> {
       assertEquals(CollectionParams.CollectionAction.ADDREPLICA, op.getAction());
       assertEquals(1, op.getHints().size());
-      Pair<String, String> hint = (Pair<String, String>)op.getHints().get(Suggester.Hint.COLL_SHARD);
+      Object o = op.getHints().get(Suggester.Hint.COLL_SHARD);
+      // this may be a pair or a HashSet of pairs with size 1
+      Pair<String, String> hint = null;
+      if (o instanceof Pair) {
+        hint = (Pair<String, String>)o;
+      } else if (o instanceof Set) {
+        assertEquals("unexpected number of hints: " + o, 1, ((Set)o).size());
+        o = ((Set)o).iterator().next();
+        assertTrue("unexpected hint: " + o, o instanceof Pair);
+        hint = (Pair<String, String>)o;
+      } else {
+        fail("unexpected hints: " + o);
+      }
       assertNotNull(hint);
       assertEquals(collectionName, hint.first());
       assertEquals("shard1", hint.second());

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/751987d5/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/SplitShardSuggester.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/SplitShardSuggester.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/SplitShardSuggester.java
index dc371d2..2a42d27 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/SplitShardSuggester.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/SplitShardSuggester.java
@@ -24,7 +24,7 @@ import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.common.util.Pair;
 
 /**
- * This suggester produces a SPLITSHARD request using provided {@link Hint#COLL_SHARD} value.
+ * This suggester produces a SPLITSHARD request using provided {@link org.apache.solr.client.solrj.cloud.autoscaling.Suggester.Hint#COLL_SHARD} value.
  */
 class SplitShardSuggester extends Suggester {