You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by no...@apache.org on 2019/06/10 07:14:13 UTC

[lucene-solr] branch jira/SOLR-13229 created (now 23f019be)

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

noble pushed a change to branch jira/SOLR-13229
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.


      at 23f019be SOLR-13329: change put: on-each to put: on-each-node

This branch includes the following new commits:

     new 23f019be SOLR-13329: change put: on-each to put: on-each-node

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



[lucene-solr] 01/01: SOLR-13329: change put: on-each to put: on-each-node

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

noble pushed a commit to branch jira/SOLR-13229
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 23f019be169faa5659babcc41cd6f083eaa0f2c1
Author: Noble Paul <no...@apache.org>
AuthorDate: Mon Jun 10 17:13:42 2019 +1000

    SOLR-13329: change put: on-each to put: on-each-node
---
 .../src/solrcloud-autoscaling-api.adoc             |  2 +-
 .../solrcloud-autoscaling-policy-preferences.adoc  |  8 +--
 .../client/solrj/cloud/autoscaling/Clause.java     | 60 +++++++++++++++++++---
 .../solrj/cloud/autoscaling/ReplicaVariable.java   | 17 +++++-
 .../client/solrj/cloud/autoscaling/TestPolicy.java | 31 ++++++++---
 5 files changed, 99 insertions(+), 19 deletions(-)

diff --git a/solr/solr-ref-guide/src/solrcloud-autoscaling-api.adoc b/solr/solr-ref-guide/src/solrcloud-autoscaling-api.adoc
index e8203ec..ed51b1d 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 e5a0b09..8262bda 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 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
 
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 adfbfcb..f89b28a 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<Clause> {
     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<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;
@@ -751,7 +799,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;
 
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 f0bfafa..675382a 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/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java b/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java
index dacfc37..a793995 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<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));