You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2020/07/27 15:04:19 UTC

[lucene-solr] branch branch_8_6 updated (7ed874a -> 7c84279)

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

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


    from 7ed874a  Add back-compat indices for 8.6.0
     new 1a7ca4e  Revert "SOLR-12845: Properly clear default policy between tests."
     new 7c84279  Revert "SOLR-12845: Add a default autoscaling cluster policy."

The 2 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.


Summary of changes:
 solr/CHANGES.txt                                   |  4 +-
 .../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 +++-
 .../solr/cloud/autoscaling/TestPolicyCloud.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 ++------------
 16 files changed, 26 insertions(+), 139 deletions(-)


[lucene-solr] 02/02: Revert "SOLR-12845: Add a default autoscaling cluster policy."

Posted by ho...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 7c8427907300256e7b3bcf6835e43986a712ffac
Author: Houston Putman <ho...@apache.org>
AuthorDate: Wed Jul 22 16:44:33 2020 -0400

    Revert "SOLR-12845: Add a default autoscaling cluster policy."
    
    To fix the regressions found in  SOLR-14665.
    
    This reverts commit 8e0eae260a0c38fa03e2eaf682d0db9d8b0b6374.
---
 solr/CHANGES.txt                                   |  4 +-
 .../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 +++-
 .../solr/cloud/autoscaling/TestPolicyCloud.java    |  3 --
 .../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 ++------------
 16 files changed, 26 insertions(+), 134 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 3266c1f..6bc0b55 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -10,7 +10,9 @@ Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this r
 
 Bug Fixes
 ---------------------
-(No changes)
+
+* SOLR-14665: Revert SOLR-12845 adding of default autoscaling cluster policy, due to performance
+ issues. (Ishan Chattopadhyaya, Houston Putman)
 
 ==================  8.6.0 ==================
 
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 f9bfa62..76f9803 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/TestPolicyCloud.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java
index 3916e8e..a17ffc3 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java
@@ -85,9 +85,6 @@ public class TestPolicyCloud extends SolrCloudTestCase {
     cluster.deleteAllCollections();
     cluster.getSolrClient().getZkStateReader().getZkClient().setData(ZkStateReader.SOLR_AUTOSCALING_CONF_PATH,
         "{}".getBytes(StandardCharsets.UTF_8), true);
-    // remove default policy
-    String commands =  "{set-cluster-policy : []}";
-    cluster.getSolrClient().request(AutoScalingRequest.create(SolrRequest.METHOD.POST, commands));
   }
 
   public void testCreateCollection() throws Exception  {
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 a47f612..09c44ad 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"));
@@ -2242,7 +2238,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'}]}}";
@@ -3328,24 +3324,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));
   }
 }


[lucene-solr] 01/02: Revert "SOLR-12845: Properly clear default policy between tests."

Posted by ho...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 1a7ca4e2300538b1f577bfaf0e40ec55e48600a9
Author: Houston Putman <ho...@apache.org>
AuthorDate: Wed Jul 22 16:30:15 2020 -0400

    Revert "SOLR-12845: Properly clear default policy between tests."
    
    To fix the regressions found in  SOLR-14665.
    
    This reverts commit 789c97be5fb66b61210cff9dafb89daabec9fe39.
---
 .../org/apache/solr/cloud/autoscaling/TestPolicyCloud.java    | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java
index 404e986..3916e8e 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java
@@ -59,7 +59,6 @@ import org.apache.solr.common.util.TimeSource;
 import org.apache.solr.common.util.Utils;
 import org.apache.solr.util.TimeOut;
 import org.junit.After;
-import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.rules.ExpectedException;
 import org.slf4j.Logger;
@@ -81,18 +80,14 @@ public class TestPolicyCloud extends SolrCloudTestCase {
         .configure();
   }
 
-  @Before
-  public void before() throws Exception {
-    // remove default policy
-    String commands =  "{set-cluster-policy : []}";
-    cluster.getSolrClient().request(AutoScalingRequest.create(SolrRequest.METHOD.POST, commands));
-  }
-
   @After
   public void after() throws Exception {
     cluster.deleteAllCollections();
     cluster.getSolrClient().getZkStateReader().getZkClient().setData(ZkStateReader.SOLR_AUTOSCALING_CONF_PATH,
         "{}".getBytes(StandardCharsets.UTF_8), true);
+    // remove default policy
+    String commands =  "{set-cluster-policy : []}";
+    cluster.getSolrClient().request(AutoScalingRequest.create(SolrRequest.METHOD.POST, commands));
   }
 
   public void testCreateCollection() throws Exception  {