SOLR-14409: Existing violations allow bypassing policy rules when add… (#1598)

* SOLR-14409: Existing violations allow bypassing policy rules when adding new replicas

solr/CHANGES.txt

@@ -262,6 +262,8 @@ Bug Fixes
 * SOLR-14577: Return 400 BAD REQUEST when field is missing on a Terms query parser request
   (Tomás Fernández Löbbe)
 
+* SOLR-14409: Existing violations allow bypassing policy rules when adding new replicas (noble, ab)
+
 * SOLR-14584: Correct SOLR_SSL_KEY_STORE and SOLR_SSL_TRUST_STORE example comments in solr.in.sh and solr.in.cmd files
   (Aren Cambre via Christine Poerschke)
 

solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggester.java

@@ -291,7 +291,7 @@ public abstract class Suggester implements MapWriter {
       //the computed value can change over time. So it's better to evaluate it in the end
       if (isTxOpen && v.getClause().hasComputedValue) continue;
       int idx = originalViolations.indexOf(v);
-      if (idx < 0 /*|| originalViolations.get(idx).isLessSerious(v)*/) return true;
+      if (idx < 0 || originalViolations.get(idx).isLessSerious(v)) return true;
     }
     return false;
   }

solr/solrj/src/test-files/solrj/solr/autoscaling/testAddTooManyPerPolicy.json

@@ -0,0 +1,129 @@
+{
+  "diagnostics": {
+    "sortedNodes": [
+      {
+        "node": "127.0.0.1:61737_solr",
+        "isLive": true,
+        "cores": 2.0,
+        "freedisk": 184.94042205810547,
+        "totaldisk": 465.62699127197266,
+        "replicas": {
+          "TooManyPerPolicy": {
+            "shard1": [
+              {
+                "core_node3": {
+                  "core": "TooManyPerPolicy_shard1_replica_n1",
+                  "shard": "shard1",
+                  "collection": "TooManyPerPolicy",
+                  "node_name": "127.0.0.1:61737_solr",
+                  "type": "NRT",
+                  "leader": "true",
+                  "base_url": "http://127.0.0.1:61737/solr",
+                  "state": "active",
+                  "force_set_state": "false",
+                  "INDEX.sizeInGB": 6.426125764846802E-8
+                }
+              },
+              {
+                "core_node8": {
+                  "core": "TooManyPerPolicy_shard1_replica_n7",
+                  "shard": "shard1",
+                  "collection": "TooManyPerPolicy",
+                  "node_name": "127.0.0.1:61737_solr",
+                  "type": "NRT",
+                  "base_url": "http://127.0.0.1:61737/solr",
+                  "state": "down",
+                  "force_set_state": "false",
+                  "INDEX.sizeInGB": 6.426125764846802E-8
+                }}]}}},
+      {
+        "node": "127.0.0.1:61738_solr",
+        "isLive": true,
+        "cores": 1.0,
+        "freedisk": 184.94042205810547,
+        "totaldisk": 465.62699127197266,
+        "replicas": {
+          "TooManyPerPolicy": {
+            "shard3": [
+              {
+                "core_node6": {
+                  "core": "TooManyPerPolicy_shard3_replica_n4",
+                  "shard": "shard3",
+                  "collection": "TooManyPerPolicy",
+                  "node_name": "127.0.0.1:61738_solr",
+                  "type": "NRT",
+                  "leader": "true",
+                  "base_url": "http://127.0.0.1:61738/solr",
+                  "state": "active",
+                  "force_set_state": "false",
+                  "INDEX.sizeInGB": 6.426125764846802E-8
+                }}]}}},
+      {
+        "node": "127.0.0.1:61739_solr",
+        "isLive": true,
+        "cores": 1.0,
+        "freedisk": 184.94042205810547,
+        "totaldisk": 465.62699127197266,
+        "replicas": {
+          "TooManyPerPolicy": {
+            "shard2": [
+              {
+                "core_node5": {
+                  "core": "TooManyPerPolicy_shard2_replica_n2",
+                  "shard": "shard2",
+                  "collection": "TooManyPerPolicy",
+                  "node_name": "127.0.0.1:61739_solr",
+                  "type": "NRT",
+                  "leader": "true",
+                  "base_url": "http://127.0.0.1:61739/solr",
+                  "state": "active",
+                  "force_set_state": "false",
+                  "INDEX.sizeInGB": 6.426125764846802E-8
+                }}]}}}
+    ],
+    "liveNodes": [
+      "127.0.0.1:61738_solr",
+      "127.0.0.1:61739_solr",
+      "127.0.0.1:61737_solr"
+    ],
+    "violations": [
+      {
+        "collection": "TooManyPerPolicy",
+        "node": "127.0.0.1:61737_solr",
+        "tagKey": "127.0.0.1:61737_solr",
+        "violation": {
+          "replica": {"NRT": 2, "count": 2},
+          "delta": 1.0},
+        "clause": {
+          "replica": "<2", "shard": "#ANY", "node": "#ANY", "strict": true, "collection": "TooManyPerPolicy"},
+        "violatingReplicas": [
+          {
+            "core_node3": {
+              "core": "TooManyPerPolicy_shard1_replica_n1",
+              "shard": "shard1",
+              "collection": "TooManyPerPolicy",
+              "node_name": "127.0.0.1:61737_solr",
+              "type": "NRT",
+              "leader": "true",
+              "base_url": "http://127.0.0.1:61737/solr",
+              "state": "active",
+              "force_set_state": "false",
+              "INDEX.sizeInGB": 6.426125764846802E-8}},
+          {
+            "core_node8": {
+              "core": "TooManyPerPolicy_shard1_replica_n7",
+              "shard": "shard1",
+              "collection": "TooManyPerPolicy",
+              "node_name": "127.0.0.1:61737_solr",
+              "type": "NRT",
+              "base_url": "http://127.0.0.1:61737/solr",
+              "state": "down",
+              "force_set_state": "false",
+              "INDEX.sizeInGB": 6.426125764846802E-8
+            }}]}],
+    "config": {
+      "cluster-preferences": [
+        {"minimize": "cores", "precision": 1},
+        {"maximize": "freedisk"}],
+      "cluster-policy": [
+        {"replica": "<2", "shard": "#ANY", "node": "#ANY", "strict": true}]}}}

solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy2.java

@@ -40,6 +40,7 @@ import org.apache.solr.client.solrj.cloud.NodeStateProvider;
 import org.apache.solr.client.solrj.cloud.SolrCloudManager;
 import org.apache.solr.client.solrj.impl.ClusterStateProvider;
 import org.apache.solr.client.solrj.impl.SolrClientNodeStateProvider;
+import org.apache.solr.common.SolrException;
 import org.apache.solr.common.cloud.ClusterState;
 import org.apache.solr.common.cloud.ReplicaPosition;
 import org.apache.solr.common.util.Utils;
@@ -516,6 +517,17 @@ public class TestPolicy2 extends SolrTestCaseJ4 {
     System.out.println(suggestions);
   }
 
+  @SuppressWarnings({"unchecked"})
+  public void testAddTooManyPerPolicy() {
+    Map<String, Object> m = (Map<String, Object>) loadFromResource("testAddTooManyPerPolicy.json");
+    SolrCloudManager cloudManagerFromDiagnostics = createCloudManagerFromDiagnostics(m);
+    AutoScalingConfig autoScalingConfig = new AutoScalingConfig((Map<String, Object>) getObjectByPath(m, false, "diagnostics/config"));
+    SolrException exp =  expectThrows(SolrException.class, () -> PolicyHelper.getReplicaLocations("TooManyPerPolicy", autoScalingConfig, cloudManagerFromDiagnostics,
+            EMPTY_MAP, Collections.singletonList("shard1"), 1, 0, 0, null));
+    assertTrue(exp.getMessage().contains("No node can satisfy the rules"));
+
+  }
+
   public static Object loadFromResource(String file)  {
     try (InputStream is = TestPolicy2.class.getResourceAsStream("/solrj/solr/autoscaling/" + file)) {
       return Utils.fromJSON(is, MAPOBJBUILDER);