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 2020/07/28 10:18:44 UTC

[lucene-solr] branch branch_8x updated: Revert "SOLR-12845: Add a default autoscaling cluster policy."

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

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


The following commit(s) were added to refs/heads/branch_8x by this push:
     new ffaeaf6  Revert "SOLR-12845: Add a default autoscaling cluster policy."
ffaeaf6 is described below

commit ffaeaf6382857f06e1421ab85394398a2ae6ad1b
Author: Andrzej Bialecki <ab...@apache.org>
AuthorDate: Mon Jul 27 18:37:41 2020 +0200

    Revert "SOLR-12845: Add a default autoscaling cluster policy."
    
    This reverts commit 8e0eae260a0c38fa03e2eaf682d0db9d8b0b6374.
---
 solr/CHANGES.txt                                   |  3 ++
 .../apache/solr/cloud/api/collections/Assign.java  |  2 +-
 .../cloud/autoscaling/OverseerTriggerThread.java   | 13 +-----
 .../autoscaling/sim/SimNodeStateProvider.java      | 11 +-----
 .../solr/cloud/autoscaling/sim/SimScenario.java    |  4 +-
 .../org/apache/solr/cloud/TestUtilizeNode.java     |  8 +++-
 .../cloud/autoscaling/sim/TestSimLargeCluster.java |  4 --
 .../cloud/autoscaling/sim/TestSimPolicyCloud.java  |  3 --
 .../cloud/autoscaling/sim/TestSimScenario.java     |  3 --
 .../src/solrcloud-autoscaling-overview.adoc        | 21 ----------
 .../client/solrj/cloud/autoscaling/Clause.java     |  5 +--
 .../client/solrj/cloud/autoscaling/Policy.java     | 46 +++-------------------
 .../client/solrj/cloud/autoscaling/Suggestion.java |  1 +
 .../autoscaling/testSuggestionsRebalance2.json     |  3 +-
 .../client/solrj/cloud/autoscaling/TestPolicy.java | 29 ++------------
 15 files changed, 26 insertions(+), 130 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index a3d3536..a59d945 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -44,6 +44,9 @@ Bug Fixes
 * SOLR-11656: TLOG replication doesn't work properly after rebalancing leaders. (Yuki Yano via
   Erick Erickson)
 
+* SOLR-14665: Revert changes from SOLR-12845 (Add a default cluster policy) to fix a
+  collection creation performance regression. (ishan, houston, ab)
+
 Other Changes
 ---------------------
 
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
index cfc401d..8f3b247 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
@@ -299,7 +299,7 @@ public class Assign {
     // if no autoscaling configuration exists then obviously we cannot use the policy framework
     if (autoScalingConfig.getPolicy().isEmpty()) return false;
     // do custom preferences exist
-    if (!autoScalingConfig.getPolicy().hasEmptyPreferences()) return true;
+    if (!autoScalingConfig.getPolicy().isEmptyPreferences()) return true;
     // does a cluster policy exist
     if (!autoScalingConfig.getPolicy().getClusterPolicy().isEmpty()) return true;
     // finally we check if the current collection has a policy
diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/OverseerTriggerThread.java b/solr/core/src/java/org/apache/solr/cloud/autoscaling/OverseerTriggerThread.java
index a5b25cd..81780ec 100644
--- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/OverseerTriggerThread.java
+++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/OverseerTriggerThread.java
@@ -33,7 +33,6 @@ import org.apache.solr.client.solrj.cloud.DistribStateManager;
 import org.apache.solr.client.solrj.cloud.autoscaling.AutoScalingConfig;
 import org.apache.solr.client.solrj.cloud.autoscaling.BadVersionException;
 import org.apache.solr.client.solrj.cloud.SolrCloudManager;
-import org.apache.solr.client.solrj.cloud.autoscaling.Policy;
 import org.apache.solr.client.solrj.cloud.autoscaling.TriggerEventType;
 import org.apache.solr.common.AlreadyClosedException;
 import org.apache.solr.common.SolrCloseable;
@@ -147,8 +146,7 @@ public class OverseerTriggerThread implements Runnable, SolrCloseable {
           break;
         }
         AutoScalingConfig autoScalingConfig = cloudManager.getDistribStateManager().getAutoScalingConfig();
-        AutoScalingConfig updatedConfig = withDefaultPolicy(autoScalingConfig);
-        updatedConfig = withAutoAddReplicasTrigger(updatedConfig);
+        AutoScalingConfig updatedConfig = withAutoAddReplicasTrigger(autoScalingConfig);
         updatedConfig = withScheduledMaintenanceTrigger(updatedConfig);
         if (updatedConfig.equals(autoScalingConfig)) break;
         log.debug("Adding .auto_add_replicas and .scheduled_maintenance triggers");
@@ -350,15 +348,6 @@ public class OverseerTriggerThread implements Runnable, SolrCloseable {
     }
   }
 
-  private AutoScalingConfig withDefaultPolicy(AutoScalingConfig autoScalingConfig) {
-    Policy policy = autoScalingConfig.getPolicy();
-    if (policy.hasEmptyClusterPolicy()) {
-      policy = policy.withClusterPolicy(Policy.DEFAULT_CLUSTER_POLICY);
-      autoScalingConfig = autoScalingConfig.withPolicy(policy);
-    }
-    return autoScalingConfig;
-  }
-
   private AutoScalingConfig withAutoAddReplicasTrigger(AutoScalingConfig autoScalingConfig) {
     Map<String, Object> triggerProps = AutoScaling.AUTO_ADD_REPLICAS_TRIGGER_PROPS;
     return withDefaultTrigger(triggerProps, autoScalingConfig);
diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/sim/SimNodeStateProvider.java b/solr/core/src/java/org/apache/solr/cloud/autoscaling/sim/SimNodeStateProvider.java
index 17b6d28..8e6bb9c 100644
--- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/sim/SimNodeStateProvider.java
+++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/sim/SimNodeStateProvider.java
@@ -19,7 +19,6 @@ package org.apache.solr.cloud.autoscaling.sim;
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -235,11 +234,6 @@ public class SimNodeStateProvider implements NodeStateProvider {
     return nodeValues;
   }
 
-  /** Get all values for a selected node. */
-  public Map<String, Object> simGetNodeValues(String node) {
-    return nodeValues.getOrDefault(node, Collections.emptyMap());
-  }
-
   private void saveRoles() {
     final Map<String, Set<String>> roles = new HashMap<>();
     nodeValues.forEach((n, values) -> {
@@ -326,10 +320,7 @@ public class SimNodeStateProvider implements NodeStateProvider {
     if (values == null) {
       return result;
     }
-    result.putAll(values.entrySet().stream()
-        .filter(e -> tags.contains(e.getKey()))
-        .filter(e -> e.getValue() != null)
-        .collect(Collectors.toMap(e -> e.getKey(), e -> e.getValue())));
+    result.putAll(values.entrySet().stream().filter(e -> tags.contains(e.getKey())).collect(Collectors.toMap(e -> e.getKey(), e -> e.getValue())));
     return result;
   }
 
diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/sim/SimScenario.java b/solr/core/src/java/org/apache/solr/cloud/autoscaling/sim/SimScenario.java
index 3b28324..771ce21 100644
--- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/sim/SimScenario.java
+++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/sim/SimScenario.java
@@ -814,9 +814,7 @@ public class SimScenario implements AutoCloseable {
         values.put(key, val);
       }
       for (String node : nodes) {
-        Map<String, Object> newValues = new HashMap<>(scenario.cluster.getSimNodeStateProvider().simGetNodeValues(node));
-        newValues.putAll(values);
-        scenario.cluster.getSimNodeStateProvider().simSetNodeValues(node, newValues);
+        scenario.cluster.getSimNodeStateProvider().simSetNodeValues(node, values);
       }
     }
   }
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestUtilizeNode.java b/solr/core/src/test/org/apache/solr/cloud/TestUtilizeNode.java
index cac7f76..ab9585b 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestUtilizeNode.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestUtilizeNode.java
@@ -72,12 +72,12 @@ public class TestUtilizeNode extends SolrCloudTestCase {
 
   @Test
   public void test() throws Exception {
-    cluster.waitForAllNodes(5);
+    int REPLICATION = 2;
     String coll = "utilizenodecoll";
     CloudSolrClient cloudClient = cluster.getSolrClient();
     
     log.info("Creating Collection...");
-    CollectionAdminRequest.Create create = CollectionAdminRequest.createCollection(coll, "conf1", 2, 2)
+    CollectionAdminRequest.Create create = CollectionAdminRequest.createCollection(coll, "conf1", 2, REPLICATION)
         .setMaxShardsPerNode(2);
     cloudClient.request(create);
 
@@ -137,6 +137,10 @@ public class TestUtilizeNode extends SolrCloudTestCase {
     cloudClient.request(new CollectionAdminRequest.UtilizeNode(jettyY.getNodeName()));
 
     assertSomeReplicas("jettyY should now be utilized: ", coll, jettyY);
+    
+    assertNoReplicas("jettyX should no longer be utilized: ", coll, jettyX); 
+    
+
   }
 
   /**
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimLargeCluster.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimLargeCluster.java
index 01336ef..921a496 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimLargeCluster.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimLargeCluster.java
@@ -100,8 +100,6 @@ public class TestSimLargeCluster extends SimSolrCloudTestCase {
     // disable metrics history collection
     cluster.disableMetricsHistory();
 
-    // turn off the default policy to avoid slowdowns due to the costly #EQUAL rules
-    CloudTestUtils.assertAutoScalingRequest(cluster, "{'set-cluster-policy': []}");
     // disable .scheduled_maintenance (once it exists)
     CloudTestUtils.waitForTriggerToBeScheduled(cluster, ".scheduled_maintenance");
     CloudTestUtils.suspendTrigger(cluster, ".scheduled_maintenance");
@@ -307,8 +305,6 @@ public class TestSimLargeCluster extends SimSolrCloudTestCase {
   }
   
   @Test
-  // impossible to complete due to the slowness of policy calculations
-  @AwaitsFix( bugUrl = "https://issues.apache.org/jira/browse/SOLR-14275")
   public void testAddNode() throws Exception {
     SolrClient solrClient = cluster.simGetSolrClient();
     assertAutoScalingRequest
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimPolicyCloud.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimPolicyCloud.java
index 915aa3f..8502851 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimPolicyCloud.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimPolicyCloud.java
@@ -63,9 +63,6 @@ public class TestSimPolicyCloud extends SimSolrCloudTestCase {
   @Before
   public void setupCluster() throws Exception {
     configureCluster(5, TimeSource.get("simTime:50"));
-    // reset autoscaling policy to empty
-    String commands =  "{set-cluster-policy : []}";
-    cluster.simGetSolrClient().request(AutoScalingRequest.create(SolrRequest.METHOD.POST, commands));
   }
   
   @After
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimScenario.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimScenario.java
index 13554bf..4e2f7d6 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimScenario.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/sim/TestSimScenario.java
@@ -63,7 +63,6 @@ public class TestSimScenario extends SimSolrCloudTestCase {
 
   String testSuggestionsScenario =
       "create_cluster numNodes=2\n" +
-      "load_autoscaling json={'cluster-policy':[]}\n" +
       "solr_request /admin/collections?action=CREATE&autoAddReplicas=true&name=testCollection&numShards=2&replicationFactor=2&maxShardsPerNode=2\n" +
       "wait_collection collection=testCollection&shards=2&replicas=2\n" +
       "ctx_set key=myNode&value=${_random_node_}\n" +
@@ -122,7 +121,6 @@ public class TestSimScenario extends SimSolrCloudTestCase {
 
   String indexingScenario =
       "create_cluster numNodes=100\n" +
-      "load_autoscaling json={'cluster-policy':[]}\n" +
       "solr_request /admin/collections?action=CREATE&autoAddReplicas=true&name=testCollection&numShards=2&replicationFactor=2&maxShardsPerNode=2\n" +
       "wait_collection collection=testCollection&shards=2&replicas=2\n" +
       "solr_request /admin/autoscaling?httpMethod=POST&stream.body=" +
@@ -146,7 +144,6 @@ public class TestSimScenario extends SimSolrCloudTestCase {
 
   String splitShardScenario =
       "create_cluster numNodes=2\n" +
-          "load_autoscaling json={'cluster-policy':[]}\n" +
           "solr_request /admin/collections?action=CREATE&name=testCollection&numShards=2&replicationFactor=2&maxShardsPerNode=5\n" +
           "wait_collection collection=testCollection&shards=2&replicas=2\n" +
           "set_shard_metrics collection=testCollection&shard=shard1&INDEX.sizeInBytes=1000000000\n" +
diff --git a/solr/solr-ref-guide/src/solrcloud-autoscaling-overview.adoc b/solr/solr-ref-guide/src/solrcloud-autoscaling-overview.adoc
index 222ca62..f6d29b5 100644
--- a/solr/solr-ref-guide/src/solrcloud-autoscaling-overview.adoc
+++ b/solr/solr-ref-guide/src/solrcloud-autoscaling-overview.adoc
@@ -64,27 +64,6 @@ There are many metrics on which the rule can be based, e.g., system load average
 
 When a node, shard, or collection does not satisfy a policy rule, we call it a *violation*.   By default, cluster management operations will fail if there is even one violation.  You can allow operations to succeed in the face of a violation by marking the corresponding rule with <<solrcloud-autoscaling-policy-preferences.adoc#rule-strictness,`"strict":false`>>.  When you do this, Solr ensures that cluster management operations minimize the number of violations.
 
-The default cluster policy, if none is specified, is the following:
-
-[source,json]
-----
-[
-  { "replica" : "<2", "shard" : "#EACH", "node" : "#ANY", "strict" : false},
-  { "replica" : "#EQUAL", "node" : "#ANY", "strict" : false},
-  { "cores" : "#EQUAL", "node" : "#ANY", "strict" : false}
-]
-----
-
-These rules mean that:
-
-* Each shard should not have more than one replica on the same node, if possible (strict: false).
-* Each collection's replicas should be equally distributed among nodes.
-* All cores should be equally distributed among nodes.
-
-
-NOTE: You can remove this default policy by specifying an empty rule-set in the autoscaling
-admin request, like this: `{set-cluster-policy : []}`.
-
 Solr also supports <<solrcloud-autoscaling-policy-preferences.adoc#collection-specific-policy,collection-specific policies>>, which operate in tandem with the cluster policy.
 
 == Triggers
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 b5c956a..8117107 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
@@ -118,7 +118,7 @@ public class Clause implements MapWriter, Comparable<Clause> {
     Optional<String> globalTagName = m.keySet().stream().filter(Policy.GLOBAL_ONLY_TAGS::contains).findFirst();
     if (globalTagName.isPresent()) {
       globalTag = parse(globalTagName.get(), m);
-      if (m.size() > 3) {
+      if (m.size() > 2) {
         throw new RuntimeException("Only one extra tag supported for the tag " + globalTagName.get() + " in " + toJSONString(m));
       }
       tag = parse(m.keySet().stream()
@@ -686,8 +686,7 @@ public class Clause implements MapWriter, Comparable<Clause> {
       for (Row r : session.matrix) {
         computedValueEvaluator.node = r.node;
         SealedClause sealedClause = getSealedClause(computedValueEvaluator);
-        // check only live nodes
-        if (r.isLive() && !sealedClause.getGlobalTag().isPass(r)) {
+        if (!sealedClause.getGlobalTag().isPass(r)) {
           ctx.resetAndAddViolation(r.node, null, new Violation(sealedClause, null, null, r.node, r.getVal(sealedClause.globalTag.name),
               sealedClause.globalTag.delta(r.getVal(globalTag.name)), r.node));
           addViolatingReplicasForGroup(sealedClause.globalTag, computedValueEvaluator, ctx, Type.CORES.tagName, r.node, ctx.currentViolation, session.matrix);
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 cf326e2..64aa1f7 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
@@ -18,7 +18,6 @@
 package org.apache.solr.client.solrj.cloud.autoscaling;
 
 import java.io.IOException;
-import java.io.PrintWriter;
 import java.io.StringWriter;
 import java.lang.invoke.MethodHandles;
 import java.util.ArrayList;
@@ -88,18 +87,6 @@ public class Policy implements MapWriter {
           new Preference((Map<String, Object>) Utils.fromJSONString("{minimize : cores, precision:1}")),
           new Preference((Map<String, Object>) Utils.fromJSONString("{maximize : freedisk}"))));
 
-  public static final List<Map<String, Object>> DEFAULT_CLUSTER_POLICY_JSON = Collections.unmodifiableList(
-      Arrays.asList(
-          Utils.makeMap("replica","<2", "shard","#EACH", "node", "#ANY", "strict", "false"),
-          Utils.makeMap("replica", "#EQUAL", "node", "#ANY", "strict", "false"),
-          Utils.makeMap("cores", "#EQUAL", "node","#ANY", "strict", "false")
-      )
-  );
-
-  public static final List<Clause> DEFAULT_CLUSTER_POLICY = DEFAULT_CLUSTER_POLICY_JSON.stream()
-      .map(Clause::create)
-      .collect(collectingAndThen(toList(), Collections::unmodifiableList));
-
   /**
    * These parameters are always fetched for all nodes regardless of whether they are used in preferences or not
    */
@@ -121,12 +108,6 @@ public class Policy implements MapWriter {
    */
   private final boolean emptyPreferences;
 
-  /**
-   * True if cluster policy was originally empty, false otherwise. It is used to figure out if the
-   * current policy was implicitly added or not.
-   */
-  final boolean emptyClusterPolicy;
-
   public Policy() {
     this(Collections.emptyMap());
   }
@@ -154,12 +135,7 @@ public class Policy implements MapWriter {
     final SortedSet<String> paramsOfInterest = new TreeSet<>(DEFAULT_PARAMS_OF_INTEREST);
     clusterPreferences.forEach(preference -> paramsOfInterest.add(preference.name.toString()));
     List<String> newParams = new ArrayList<>(paramsOfInterest);
-
-    // if json map has CLUSTER_POLICY and even if its size is 0, we consider it as a custom cluster policy
-    // and do not add the implicit policy clauses
-    emptyClusterPolicy = !jsonMap.containsKey(CLUSTER_POLICY);
-
-    clusterPolicy = ((List<Map<String, Object>>) jsonMap.getOrDefault(CLUSTER_POLICY, DEFAULT_CLUSTER_POLICY_JSON)).stream()
+    clusterPolicy = ((List<Map<String, Object>>) jsonMap.getOrDefault(CLUSTER_POLICY, emptyList())).stream()
         .map(Clause::create)
         .filter(clause -> {
           clause.addTags(newParams);
@@ -169,7 +145,7 @@ public class Policy implements MapWriter {
 
     for (String newParam : new ArrayList<>(newParams)) {
       Type t = VariableBase.getTagType(newParam);
-      if(t != null && !t.associatedPerNodeValues.isEmpty()) {
+      if(t != null && !t.associatedPerNodeValues.isEmpty()){
         for (String s : t.associatedPerNodeValues) {
           if(!newParams.contains(s)) newParams.add(s);
         }
@@ -198,7 +174,6 @@ public class Policy implements MapWriter {
     this.empty = policies == null && clusterPolicy == null && clusterPreferences == null;
     this.zkVersion = version;
     this.policies = policies != null ? Collections.unmodifiableMap(policies) : Collections.emptyMap();
-    this.emptyClusterPolicy = clusterPolicy == null;
     this.clusterPolicy = clusterPolicy != null ? Collections.unmodifiableList(clusterPolicy) : Collections.emptyList();
     this.emptyPreferences = clusterPreferences == null;
     this.clusterPreferences = emptyPreferences ? DEFAULT_PREFERENCES : Collections.unmodifiableList(clusterPreferences);
@@ -266,7 +241,7 @@ public class Policy implements MapWriter {
         for (Preference p : clusterPreferences) iw.add(p);
       });
     }
-    if (!emptyClusterPolicy) {
+    if (!clusterPolicy.isEmpty()) {
       ew.put(CLUSTER_POLICY, (IteratorWriter) iw -> {
         for (Clause c : clusterPolicy) {
           iw.add(c);
@@ -337,13 +312,11 @@ public class Policy implements MapWriter {
               PolicyHelper.writeNodes(ew, matrixCopy);
               ew.put("config", matrix.get(0).session.getPolicy());
             });
-            StringWriter exc = new StringWriter();
-            e.printStackTrace(new PrintWriter(exc));
-            log.error("Exception during matrix sorting! prefs = {}, recent r1 = {}, r2 = {}, matrix = {}, exception={}",
+            log.error("Exception! prefs = {}, recent r1 = {}, r2 = {}, matrix = {}",
                 clusterPreferences,
                 lastComparison[0].node,
                 lastComparison[1].node,
-                Utils.writeJson(m, new StringWriter(), true).toString(), exc.toString()); // logOk
+                Utils.writeJson(m, new StringWriter(), true).toString());
           } catch (IOException e1) {
             //
           }
@@ -535,17 +508,10 @@ public class Policy implements MapWriter {
   /**
    * @return true if no preferences were specified by the user, false otherwise
    */
-  public boolean hasEmptyPreferences() {
+  public boolean isEmptyPreferences() {
     return emptyPreferences;
   }
 
-  /**
-   * @return true if no cluster policy was specified by the user, false otherwise
-   */
-  public boolean hasEmptyClusterPolicy() {
-    return emptyClusterPolicy;
-  }
-
   /*This stores the logical state of the system, given a policy and
    * a cluster state.
    *
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 29fea84..6259435 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
@@ -71,6 +71,7 @@ public class Suggestion {
 
     public boolean hasTimedOut() {
       return session.cloudManager.getTimeSource().getTimeNs() >= endTime;
+
     }
 
     public boolean needMore() {
diff --git a/solr/solrj/src/test-files/solrj/solr/autoscaling/testSuggestionsRebalance2.json b/solr/solrj/src/test-files/solrj/solr/autoscaling/testSuggestionsRebalance2.json
index d976303..958efc0 100644
--- a/solr/solrj/src/test-files/solrj/solr/autoscaling/testSuggestionsRebalance2.json
+++ b/solr/solrj/src/test-files/solrj/solr/autoscaling/testSuggestionsRebalance2.json
@@ -127,5 +127,4 @@
         "minimize":"cores",
         "precision":1}
       ,{
-          "maximize":"freedisk"}],
-      "cluster-policy": []}}}
\ No newline at end of file
+          "maximize":"freedisk"}]}}}
\ No newline at end of file
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 d3e3a8f..2ef6abb 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
@@ -77,7 +77,6 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
-import static org.apache.solr.client.solrj.cloud.autoscaling.Policy.CLUSTER_POLICY;
 import static org.apache.solr.client.solrj.cloud.autoscaling.Policy.CLUSTER_PREFERENCES;
 import static org.apache.solr.client.solrj.cloud.autoscaling.TestPolicy2.loadFromResource;
 import static org.apache.solr.client.solrj.cloud.autoscaling.Variable.Type.CORES;
@@ -1339,7 +1338,6 @@ public class TestPolicy extends SolrTestCaseJ4 {
     Map<String, Map> nodeValues = (Map<String, Map>) Utils.fromJSONString("{" +
         "node1:{cores:2}," +
         "node3:{cores:4}" +
-        "node2:{cores:2}" +
         "}");
     Policy policy = new Policy(new HashMap<>());
     @SuppressWarnings({"unchecked"})
@@ -1348,8 +1346,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
         .getSuggester(MOVEREPLICA)
         .hint(Hint.COLL, "collection1")
         .hint(Hint.COLL, "collection2")
-        .hint(Suggester.Hint.SRC_NODE, "node2")
-        .forceOperation(true);
+        .hint(Suggester.Hint.SRC_NODE, "node2");
     @SuppressWarnings({"rawtypes"})
     SolrRequest op = suggester.getSuggestion();
     assertNotNull(op);
@@ -1362,8 +1359,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
         .getSuggester(MOVEREPLICA)
         .hint(Hint.COLL, "collection1")
         .hint(Hint.COLL, "collection2")
-        .hint(Suggester.Hint.SRC_NODE, "node2")
-        .forceOperation(true);
+        .hint(Suggester.Hint.SRC_NODE, "node2");
     op = suggester.getSuggestion();
     assertNotNull(op);
     assertEquals("collection2", op.getParams().get("collection"));
@@ -2246,7 +2242,7 @@ public class TestPolicy extends SolrTestCaseJ4 {
   }
 
   public void testEmptyClusterState() {
-    String autoScaleJson = " {'cluster-policy':[], 'policies':{'c1':[{" +
+    String autoScaleJson = " {'policies':{'c1':[{" +
         "        'replica':1," +
         "        'shard':'#EACH'," +
         "        'port':'50096'}]}}";
@@ -3332,24 +3328,5 @@ public class TestPolicy extends SolrTestCaseJ4 {
     // since the user explicitly added those preferences, they should be written by MapWriter
     assertEquals(1, writtenKeys.size());
     assertTrue(writtenKeys.contains(CLUSTER_PREFERENCES));
-
-    // reset
-    writtenKeys.clear();
-    // now we create a cluster policy that is intentionally empty which should prevent the implicit
-    // cluster policy from being written but should emit an empty key/val pair for cluster policy
-    policy = new Policy(Utils.makeMap(CLUSTER_POLICY, Collections.emptyList()));
-    // sanity checks
-    assertFalse(policy.isEmpty());
-    assertTrue(policy.hasEmptyPreferences());
-    assertFalse(policy.hasEmptyClusterPolicy());
-    policy.writeMap(new MapWriter.EntryWriter() {
-      @Override
-      public MapWriter.EntryWriter put(CharSequence k, Object v) throws IOException {
-        writtenKeys.add(k.toString());
-        return this;
-      }
-    });
-    assertEquals(1, writtenKeys.size());
-    assertTrue(writtenKeys.contains(CLUSTER_POLICY));
   }
 }