SOLR-13329: changed the put:on-each to put: on-each-node

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)

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);

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
 ----

solr/solr-ref-guide/src/solrcloud-autoscaling-policy-preferences.adoc

@@ -115,7 +115,7 @@ Per-collection rules have five parts:
 * <<Replica Selector and Rule Evaluation Context>>
 * <<Replica Count Constraint>> (`"replica": "..."`)
 * <<Rule Strictness>> (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 <<each-function,each>> 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
 

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<Clause> {
     if (!m.containsKey(NODESET)) return false;
     Object o = m.get(NODESET);
     if (o instanceof Map) {
-      Map map = (Map) o;
+      String key = validateObjectInNodeset(m, (Map) o);
-      if (map.size() != 1) {
-        throwExp(m, "nodeset must only have one and only one key");
-      }
-      String key = (String) map.keySet().iterator().next();
       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<Clause> {
     return true;
   }
 
+  private String validateObjectInNodeset(Map<String, Object> 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<String, Object> m) {
+    List<Map> 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<Condition> 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<Clause> {
 
   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<Clause> {
       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<Clause> {
                                     List<Row> 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<Clause> {
   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;
 

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'",

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",
+      "sysprop.zone":"east"},
-        "west"]}]}}}
+     {
+      "replica":"<3",
+      "shard":"#EACH",
+      "sysprop.zone":"west"}
+
+
+
+    ]}}}

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-node , 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 }" +
           "  ]" +
           "}");
 
@@ -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-node ,   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 }]}";
     }
     AutoScalingConfig cfg = new AutoScalingConfig((Map<String, Object>) 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<String, Object>) Utils.fromJSONString(autoScalingjson));

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<Map> l = (List<Map>) 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<String, Object>) Utils.fromJSONString(autoScalingjson));
@@ -362,10 +365,14 @@ public class TestPolicy2 extends SolrTestCaseJ4 {
           "      {" +
           "        'minimize':'sysLoadAvg'," +
           "        'precision':10}]," +
-          "    'cluster-policy':[{" +
+          "    'cluster-policy':[" +
-          "      'replica':'<3'," +
+          "{'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);