From 419560ef2a5a04ab9d77d3428459e1aa08253764 Mon Sep 17 00:00:00 2001 From: Noble Paul Date: Thu, 25 Jun 2020 10:33:28 +1000 Subject: [PATCH] =?UTF-8?q?SOLR-14409:=20Existing=20violations=20allow=20b?= =?UTF-8?q?ypassing=20policy=20rules=20when=20add=E2=80=A6=20(#1598)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * SOLR-14409: Existing violations allow bypassing policy rules when adding new replicas --- solr/CHANGES.txt | 2 + .../solrj/cloud/autoscaling/Suggester.java | 2 +- .../autoscaling/testAddTooManyPerPolicy.json | 129 ++++++++++++++++++ .../solrj/cloud/autoscaling/TestPolicy2.java | 12 ++ 4 files changed, 144 insertions(+), 1 deletion(-) create mode 100644 solr/solrj/src/test-files/solrj/solr/autoscaling/testAddTooManyPerPolicy.json diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 8e488843a6f..d28855dab87 100644 --- a/solr/CHANGES.txt +++ b/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) diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggester.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggester.java index 28460cdba51..b9d5faf7e43 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggester.java +++ b/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; } diff --git a/solr/solrj/src/test-files/solrj/solr/autoscaling/testAddTooManyPerPolicy.json b/solr/solrj/src/test-files/solrj/solr/autoscaling/testAddTooManyPerPolicy.json new file mode 100644 index 00000000000..23ab471f9b6 --- /dev/null +++ b/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}]}}} \ No newline at end of file diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy2.java b/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy2.java index 16328889af3..b61719366c9 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy2.java +++ b/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 m = (Map) loadFromResource("testAddTooManyPerPolicy.json"); + SolrCloudManager cloudManagerFromDiagnostics = createCloudManagerFromDiagnostics(m); + AutoScalingConfig autoScalingConfig = new AutoScalingConfig((Map) 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);