diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 480bc45f9c9..b5d15c3cdf3 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -120,7 +120,7 @@ New Features * SOLR-13504: In autoscaling policies, use an explicit 'nodeset' attribute for filtering nodes instead of using them directly at the toplevel (noble) -* SOLR-13329: In autoscaling policies, use an explicit 'put : on-each' +* SOLR-13329: In autoscaling policies, use an explicit 'put : on-each-node' to specify the the rules is applied on each node (noble) * SOLR-13434: OpenTracing support for Solr (Cao Manh Dat) diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java index f83c1640772..7431f9bfdc2 100644 --- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java @@ -719,7 +719,7 @@ public class AutoScalingHandlerTest extends SolrCloudTestCase { " 'set-cluster-policy': [" + " {'cores':'<10', 'node':'#ANY'}," + " {'replica':'<2', 'shard': '#EACH', 'node': '#ANY'}," + - " {'replica':0, put : on-each, nodeset:{'nodeRole':'overseer'} }" + + " {'replica':0, put : on-each-node, nodeset:{'nodeRole':'overseer'} }" + " ]" + "}"; req = AutoScalingRequest.create(SolrRequest.METHOD.POST, setClusterPolicyCommand); diff --git a/solr/solr-ref-guide/src/solrcloud-autoscaling-api.adoc b/solr/solr-ref-guide/src/solrcloud-autoscaling-api.adoc index e8203ecfad5..ed51b1db944 100644 --- a/solr/solr-ref-guide/src/solrcloud-autoscaling-api.adoc +++ b/solr/solr-ref-guide/src/solrcloud-autoscaling-api.adoc @@ -334,7 +334,7 @@ If there is no autoscaling policy configured or if you wish to use a configurati ---- curl -X POST -H 'Content-type:application/json' -d '{ "cluster-policy": [ - {"replica": 0, "put" : "on-each", "nodeset": {"port" : "7574"}} + {"replica": 0, "put" : "on-each-node", "nodeset": {"port" : "7574"}} ] }' http://localhost:8983/solr/admin/autoscaling/suggestions?omitHeader=true ---- diff --git a/solr/solr-ref-guide/src/solrcloud-autoscaling-policy-preferences.adoc b/solr/solr-ref-guide/src/solrcloud-autoscaling-policy-preferences.adoc index e5a0b09205d..8262bda61e2 100644 --- a/solr/solr-ref-guide/src/solrcloud-autoscaling-policy-preferences.adoc +++ b/solr/solr-ref-guide/src/solrcloud-autoscaling-policy-preferences.adoc @@ -115,7 +115,7 @@ Per-collection rules have five parts: * <> * <> (`"replica": "..."`) * <> (optional) -* `put` (optional) specifies how to place these replicas on the selected nodes. All the selected nodes are considered as one bucket by default. `"put" : "on-each"` treats each selected node as a bucket +* `put` (optional) specifies how to place these replicas on the selected nodes. All the selected nodes are considered as one bucket by default. `"put" : "on-each-node"` treats each selected node as a bucket ==== Node Selector @@ -140,11 +140,11 @@ when using the `nodeset` attribute, an optional attribute `put` can be used to s example: _put one replica on each node with a system property zone=east_ [source,json] -{ "replica":1, "put" :"on-each", "nodeset":{"sysprop.zone":"east"}} +{ "replica":1, "put" :"on-each-node", "nodeset":{"sysprop.zone":"east"}} example: _put a total of 2 replicas on the set of nodes with property zone=east_ [source,json] -{ "replica":2, "put" :"on-each" "nodeset":{"sysprop.zone":"east"}} +{ "replica":2, "put" :"on-each-node" "nodeset":{"sysprop.zone":"east"}} @@ -392,7 +392,7 @@ For <> shard of each collection, distribute replicas equally Do not place any replica on any node that has the overseer role. Note that the role is added by the `addRole` collection API. It is *not* automatically the node which is currently the overseer. [source,json] -{"replica": 0, "put" :"on-each", "nodeset":{ "nodeRole": "overseer"}} +{"replica": 0, "put" :"on-each-node", "nodeset":{ "nodeRole": "overseer"}} ==== Place Replicas Based on Free Disk 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 adfbfcbde7b..f12ecce6a8c 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 @@ -18,6 +18,7 @@ package org.apache.solr.client.solrj.cloud.autoscaling; import java.io.IOException; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -150,14 +151,12 @@ public class Clause implements MapWriter, Comparable { if (!m.containsKey(NODESET)) return false; Object o = m.get(NODESET); if (o instanceof Map) { - Map map = (Map) o; - if (map.size() != 1) { - throwExp(m, "nodeset must only have one and only one key"); - } - String key = (String) map.keySet().iterator().next(); + String key = validateObjectInNodeset(m, (Map) o); parseCondition(key, o, m); } else if (o instanceof List) { List l = (List) o; + if(l.size()<2) throwExp(m, "nodeset [] must have atleast 2 items"); + if( checkMapArray(l, m)) return true; for (Object it : l) { if (it instanceof String) continue; else throwExp(m, "nodeset :[]must have only string values"); @@ -169,6 +168,46 @@ public class Clause implements MapWriter, Comparable { return true; } + private String validateObjectInNodeset(Map m, Map map) { + if (map.size() != 1) { + throwExp(m, "nodeset must only have one and only one key"); + } + String key = (String) map.keySet().iterator().next(); + Object val = map.get(key); + if(val instanceof String && ((String )val).trim().charAt(0) == '#'){ + throwExp(m, formatString("computed value {0} not allowed in nodeset", val)); + } + return key; + } + + private boolean checkMapArray(List l, Map m) { + List maps = null; + for (Object o : l) { + if (o instanceof Map) { + if (maps == null) maps = new ArrayList<>(); + maps.add((Map) o); + } + } + String key = null; + if (maps != null) { + if (maps.size() != l.size()) throwExp(m, "all elements of nodeset must be Objects"); + List tags = new ArrayList<>(maps.size()); + for (Map map : maps) { + String s = validateObjectInNodeset(m, map); + if(key == null) key = s; + if(!Objects.equals(key, s)){ + throwExp(m, "all element must have same key"); + } + tags.add(parse(s, m)); + } + if(this.put == Put.ON_EACH) throwExp(m, "cannot use put: ''on-each-node'' with an array value in nodeset "); + this.tag = new Condition(key, tags,Operand.IN, null,this); + return true; + } + return false; + + } + public Condition getThirdTag() { return globalTag == null ? tag : globalTag; } @@ -468,6 +507,15 @@ public class Clause implements MapWriter, Comparable { private Set getUniqueTags(Policy.Session session, ComputedValueEvaluator eval) { Set tags = new HashSet(); + + if(nodeSetPresent) { + if (tag.val instanceof List && ((List) tag.val).get(0) instanceof Condition) { + tags.addAll((List) tag.val); + } else { + tags.add(tag); + } + return tags; + } if(tag.op == WILDCARD){ for (Row row : session.matrix) { eval.node = row.node; @@ -488,7 +536,7 @@ public class Clause implements MapWriter, Comparable { if (tag.op == LESS_THAN || tag.op == GREATER_THAN || tag.op == RANGE_EQUAL || tag.op == NOT_EQUAL) { tags.add(tag); // eg: freedisk > 100 } else if (tag.val instanceof Collection) { - tags.addAll((Collection) tag.val); //e: sysprop.zone:[east,west] + tags.add(tag); //e: sysprop.zone:[east,west] } else { tags.add(tag.val);// } @@ -503,7 +551,18 @@ public class Clause implements MapWriter, Comparable { List nodes) { if (tag.varType.addViolatingReplicas(ctx)) return; for (Row row : nodes) { - if (tagVal.equals(row.getVal(tagName))) { + boolean isPass; + if (tagVal instanceof Condition) { + Condition condition = (Condition) tagVal; + if(condition.computedType != null){ + eval.node = row.node; + Object val = eval.apply(condition); + condition = new Condition(condition.name, val,condition.op, null, condition.clause); + } + isPass = condition.isPass(row); + } + else isPass = tagVal.equals(row.getVal(tagName)); + if (isPass) { row.forEachReplica(eval.collName, ri -> { if (Policy.ANY.equals(eval.shardName) || eval.shardName.equals(ri.getShard())) @@ -751,7 +810,7 @@ public class Clause implements MapWriter, Comparable { public static final String METRICS_PREFIX = "metrics:"; enum Put { - ON_ALL("on-all"), ON_EACH("on-each"); + ON_ALL(""), ON_EACH("on-each-node"); public final String val; diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaVariable.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaVariable.java index f0bfafad233..675382a9475 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaVariable.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaVariable.java @@ -19,6 +19,7 @@ package org.apache.solr.client.solrj.cloud.autoscaling; import java.util.Collection; import java.util.HashSet; +import java.util.List; import java.util.Set; import org.apache.solr.common.util.StrUtils; @@ -90,15 +91,29 @@ class ReplicaVariable extends VariableBase { @Override public String postValidate(Condition condition) { + Object val = condition.clause.getThirdTag().val; + boolean isNodesetObjectList = condition.clause.nodeSetPresent && (val instanceof List) && ((List)val).get(0) instanceof Condition ; + if(condition.clause.nodeSetPresent ){ + if(condition.computedType == ComputedType.EQUAL){ + if(!isNodesetObjectList) return " 'nodeset' must have an array value when 'replica': '#EQUAL` is used"; + } else { + if(isNodesetObjectList){ + return "cannot use array value for nodeset if replica : '#EQUAL' is not used"; + } + + } + + } + if (condition.computedType == ComputedType.EQUAL) { if (condition.getClause().tag != null && -// condition.getClause().tag.varType == NODE && (condition.getClause().tag.op == Operand.WILDCARD || condition.getClause().tag.op == Operand.IN)) { return null; } else { return "'replica': '#EQUAL` must be used with 'node':'#ANY'"; } } else if (condition.computedType == ComputedType.ALL) { + if(isNodesetObjectList) return "replica: '#ALL' cannot be used with a list of values in nodeset"; if (condition.getClause().tag != null && (condition.getClause().getTag().op == Operand.IN || condition.getClause().getTag().op == Operand.WILDCARD)) { return StrUtils.formatString("array value or wild card cannot be used for tag {0} with replica : '#ALL'", diff --git a/solr/solrj/src/test-files/solrj/solr/autoscaling/testSysPropSuggestions.json b/solr/solrj/src/test-files/solrj/solr/autoscaling/testSysPropSuggestions.json index aabcb62324d..d1d6cad04d4 100644 --- a/solr/solrj/src/test-files/solrj/solr/autoscaling/testSysPropSuggestions.json +++ b/solr/solrj/src/test-files/solrj/solr/autoscaling/testSysPropSuggestions.json @@ -112,8 +112,16 @@ { "minimize":"sysLoadAvg", "precision":10}], - "cluster-policy":[{ + "cluster-policy":[ + { "replica":"<3", "shard":"#EACH", - "sysprop.zone":["east", - "west"]}]}}} \ No newline at end of file + "sysprop.zone":"east"}, + { + "replica":"<3", + "shard":"#EACH", + "sysprop.zone":"west"} + + + + ]}}} \ 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 dacfc37c7c8..a7939950453 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 @@ -721,11 +721,28 @@ public class TestPolicy extends SolrTestCaseJ4 { clause = Clause.create("{'replica': '#ALL', 'nodeset': {'freedisk': '>700'}, 'strict': false}"); assertEquals(clause.put, Clause.Put.ON_ALL); assertEquals(Operand.GREATER_THAN , clause.tag.op); - clause = Clause.create("{'replica': '#ALL', put: on-each, 'nodeset': {sysprop.zone : east}}"); + clause = Clause.create("{'replica': '#ALL', put: on-each-node, 'nodeset': {sysprop.zone : east}}"); assertEquals(clause.put, Clause.Put.ON_EACH); exp = expectThrows(IllegalArgumentException.class, ()-> Clause.create("{'replica': '#ALL', put: on-Each, 'nodeset': {sysprop.zone : east}}")); assertTrue(exp.getMessage().contains("invalid value for put : on-Each")); + clause = Clause.create("{replica : '#EQUAL', nodeset : [{sysprop.zone : east}, {sysprop.zone : west}]}"); + assertTrue(((List)clause.tag.val).get(0) instanceof Condition); + assertTrue( Utils.fromJSONString(Utils.toJSONString(clause)) instanceof Map); + exp = expectThrows(IllegalArgumentException.class, ()-> Clause.create("{replica : '#EQUAL', nodeset : [{sysprop.zone : east}, {port : '8983'}]}")); + assertTrue(exp.getMessage().contains("all element must have same key")); + exp = expectThrows(IllegalArgumentException.class, ()-> Clause.create("{replica : '#EQUAL', nodeset : [{sysprop.zone : east}, {sysprop.zone : '#EACH'}]}")); + assertTrue(exp.getMessage().contains("computed value #EACH not allowed in nodeset")); + exp = expectThrows(IllegalArgumentException.class, ()-> Clause.create("{replica : '#EQUAL', nodeset : {sysprop.zone : east}}")); + assertTrue(exp.getMessage().contains("'nodeset' must have an array value when 'replica': '#EQUAL` is used")); + exp = expectThrows(IllegalArgumentException.class, ()-> Clause.create("{replica : '#ALL', nodeset : [{sysprop.zone : east}, {sysprop.zone : west}]}")); + assertTrue(exp.getMessage().contains("cannot use array value for nodeset if replica : '#EQUAL' is not used")); + exp = expectThrows(IllegalArgumentException.class, ()-> Clause.create("{replica : '50%', nodeset : [{sysprop.zone : east}, {sysprop.zone : west}]}")); + assertTrue(exp.getMessage().contains("cannot use array value for nodeset if replica : '#EQUAL' is not used")); + exp = expectThrows(IllegalArgumentException.class, ()-> Clause.create("{replica : 3, nodeset : [{sysprop.zone : east}, {sysprop.zone : west}]}")); + assertTrue(exp.getMessage().contains("cannot use array value for nodeset if replica : '#EQUAL' is not used")); + exp = expectThrows(IllegalArgumentException.class, ()-> Clause.create("{replica : '#EQUAL', put: on-each-node, nodeset : [{sysprop.zone : east}, {sysprop.zone : west}]}")); + assertTrue(exp.getMessage().contains("cannot use put: 'on-each-node' with an array value in nodeset ")); } @@ -1246,7 +1263,7 @@ public class TestPolicy extends SolrTestCaseJ4 { " { 'replica': 0, nodeset : {'nodeRole': 'overseer'}}" + " { 'replica': '<2', 'shard': '#EACH', 'node': '#ANY'}," + " { 'replica': 0, 'shard': '#EACH', nodeset : { sysprop.fs : '!ssd'}, type : TLOG }" + - " { 'replica': 0, 'shard': '#EACH', put:'on-each' nodeset : {sysprop.fs : '!slowdisk'} , type : PULL }" + + " { 'replica': 0, 'shard': '#EACH', put:'on-each-node' nodeset : {sysprop.fs : '!slowdisk'} , type : PULL }" + " ]" + "}"); @@ -1371,8 +1388,8 @@ public class TestPolicy extends SolrTestCaseJ4 { " { 'replica': '<2', 'shard': '#EACH', 'node': '#ANY', 'collection':'newColl'}," + " { 'replica': '<2', 'shard': '#EACH', 'node': '#ANY', 'collection':'newColl2', type : PULL}," + " { 'replica': '<3', 'shard': '#EACH', 'node': '#ANY', 'collection':'newColl2'}," + - " { 'replica': 0, 'shard': '#EACH', put: on-each , nodeset:{ sysprop.fs : '!ssd'}, type : TLOG }" + - " { 'replica': 0, 'shard': '#EACH', put: on-each ,nodeset : {sysprop.fs : '!slowdisk'} , type : PULL }" + + " { 'replica': 0, 'shard': '#EACH', put: on-each-node , nodeset:{ sysprop.fs : '!ssd'}, type : TLOG }" + + " { 'replica': 0, 'shard': '#EACH', put: on-each-node ,nodeset : {sysprop.fs : '!slowdisk'} , type : PULL }" + " ]" + "}"); @@ -2394,8 +2411,8 @@ public class TestPolicy extends SolrTestCaseJ4 { " cluster-preferences :[{ minimize : cores, precision : 2 }]}"; if(useNodeset){ autoScalingjson = " { cluster-policy:[" + - " { replica :'0', put:on-each , nodeset:{ freedisk:'<1000'}}," + - " { replica :0, put : on-each , nodeset : {nodeRole : overseer}}]," + + " { replica :'0', put:on-each-node , nodeset:{ freedisk:'<1000'}}," + + " { replica :0, put : on-each-node , nodeset : {nodeRole : overseer}}]," + " cluster-preferences :[{ minimize : cores, precision : 2 }]}"; } AutoScalingConfig cfg = new AutoScalingConfig((Map) Utils.fromJSONString(autoScalingjson)); @@ -2427,7 +2444,7 @@ public class TestPolicy extends SolrTestCaseJ4 { if(useNodeset){ autoScalingjson = " { cluster-policy:[" + " { replica :'#ALL', nodeset:{ freedisk:'>1000'}}," + - " { replica :0 , put: on-each , nodeset : {nodeRole : overseer}}]," + + " { replica :0 , put: on-each-node , nodeset : {nodeRole : overseer}}]," + " cluster-preferences :[{ minimize : cores, precision : 2 }]}"; } cfg = new AutoScalingConfig((Map) Utils.fromJSONString(autoScalingjson)); 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 2bceeab88e9..63b7da4b366 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 @@ -63,11 +63,13 @@ public class TestPolicy2 extends SolrTestCaseJ4 { public void testEqualOnNonNode() { List l = (List) loadFromResource("testEqualOnNonNode.json"); String autoScalingjson = "{cluster-policy:[" + - " { replica : '<3' , shard : '#EACH', sysprop.zone: [east,west] } ]," + + " { replica : '<3' , shard : '#EACH', sysprop.zone: east}," + + "{ replica : '<3' , shard : '#EACH', sysprop.zone: west} ]," + " 'cluster-preferences':[{ minimize : cores},{maximize : freedisk, precision : 50}]}"; if(useNodeset){ autoScalingjson = "{cluster-policy:[" + - " { replica : '<3' , shard : '#EACH', nodeset:{ sysprop.zone: [east,west] }} ]," + + "{ replica : '<3' , shard : '#EACH', nodeset:{ sysprop.zone: east}}," + + "{ replica : '<3' , shard : '#EACH', nodeset:{ sysprop.zone: west}} ]," + " 'cluster-preferences':[{ minimize : cores},{maximize : freedisk, precision : 50}]}"; } @@ -87,7 +89,8 @@ public class TestPolicy2 extends SolrTestCaseJ4 { " 'cluster-preferences':[{ minimize : cores},{maximize : freedisk, precision : 50}]}"; if(useNodeset){ autoScalingjson = "{cluster-policy:[" + - " { replica : '<3' , shard : '#EACH', nodeset:{sysprop.zone: [east , west]} } ]," + + "{ replica : '<3' , shard : '#EACH', nodeset:{sysprop.zone: east}} , " + + "{ replica : '<3' , shard : '#EACH', nodeset:{sysprop.zone: west}} ]," + " 'cluster-preferences':[{ minimize : cores},{maximize : freedisk, precision : 50}]}"; } policy = new Policy((Map) Utils.fromJSONString(autoScalingjson)); @@ -362,10 +365,14 @@ public class TestPolicy2 extends SolrTestCaseJ4 { " {" + " 'minimize':'sysLoadAvg'," + " 'precision':10}]," + - " 'cluster-policy':[{" + - " 'replica':'<3'," + + " 'cluster-policy':[" + + "{'replica':'<3'," + " 'shard':'#EACH'," + - " nodeset: {'sysprop.zone':['east','west']}}]}"); + " nodeset: {'sysprop.zone':'east'}}, " + + "{'replica':'<3'," + + " 'shard':'#EACH'," + + " nodeset: {'sysprop.zone':'west'}} " + + " ]}"); } Policy policy = new Policy(conf); SolrCloudManager cloudManagerFromDiagnostics = createCloudManagerFromDiagnostics(m);