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 2017/01/06 15:13:49 UTC

lucene-solr:branch_6x: SOLR-9503: NPE in Replica Placement Rules when using Overseer Role with other rules

Repository: lucene-solr
Updated Branches:
  refs/heads/branch_6x 7ae9ca85d -> 2b66d0cb1


SOLR-9503: NPE in Replica Placement Rules when using Overseer Role with other rules


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/2b66d0cb
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/2b66d0cb
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/2b66d0cb

Branch: refs/heads/branch_6x
Commit: 2b66d0cb127b5e3e92a0f988aa7ba10690227ac3
Parents: 7ae9ca8
Author: Noble Paul <no...@apache.org>
Authored: Sat Jan 7 01:40:47 2017 +1030
Committer: Noble Paul <no...@apache.org>
Committed: Sat Jan 7 01:43:29 2017 +1030

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  2 ++
 .../apache/solr/cloud/rule/ReplicaAssigner.java | 31 ++++----------------
 .../java/org/apache/solr/cloud/rule/Rule.java   |  4 +--
 .../apache/solr/cloud/rule/RuleEngineTest.java  |  2 +-
 4 files changed, 10 insertions(+), 29 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/2b66d0cb/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 60a2836..2bc09ae 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -277,6 +277,8 @@ Bug Fixes
 * SOLR-9931: JSON Facet API hll (hyper-log-log) function returned 0 for non-empty buckets with no field values
   in local mode, but nothing for distributed mode.  Both modes now return 0.  (yonik)
 
+* SOLR-9503: NPE in Replica Placement Rules when using Overseer Role with other rules (Tim Owen via noble)
+
 Other Changes
 ----------------------
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/2b66d0cb/solr/core/src/java/org/apache/solr/cloud/rule/ReplicaAssigner.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/rule/ReplicaAssigner.java b/solr/core/src/java/org/apache/solr/cloud/rule/ReplicaAssigner.java
index 4ecda47..3eab8b4 100644
--- a/solr/core/src/java/org/apache/solr/cloud/rule/ReplicaAssigner.java
+++ b/solr/core/src/java/org/apache/solr/cloud/rule/ReplicaAssigner.java
@@ -17,7 +17,6 @@
 package org.apache.solr.cloud.rule;
 
 import java.lang.invoke.MethodHandles;
-
 import java.util.ArrayList;
 import java.util.BitSet;
 import java.util.Collections;
@@ -39,7 +38,6 @@ import org.apache.solr.common.cloud.Slice;
 import org.apache.solr.common.cloud.rule.ImplicitSnitch;
 import org.apache.solr.common.cloud.rule.Snitch;
 import org.apache.solr.common.cloud.rule.SnitchContext;
-import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.common.util.Utils;
 import org.apache.solr.core.CoreContainer;
 import org.slf4j.Logger;
@@ -48,7 +46,6 @@ import org.slf4j.LoggerFactory;
 import static java.util.Collections.singletonList;
 import static org.apache.solr.cloud.rule.Rule.MatchStatus.*;
 import static org.apache.solr.cloud.rule.Rule.Phase.*;
-import static org.apache.solr.common.util.StrUtils.formatString;
 import static org.apache.solr.common.util.Utils.getDeepCopy;
 
 public class ReplicaAssigner {
@@ -103,7 +100,6 @@ public class ReplicaAssigner {
     this.participatingLiveNodes = new ArrayList<>(participatingLiveNodes);
     this.nodeVsTags = getTagsForNodes(cc, snitches);
     this.shardVsNodes = getDeepCopy(shardVsNodes, 2);
-    validateTags(nodeVsTags);
 
     if (clusterState != null) {
       Map<String, DocCollection> collections = clusterState.getCollectionsMap();
@@ -284,21 +280,6 @@ public class ReplicaAssigner {
     return result;
   }
 
-  private void validateTags(Map<String, Map<String, Object>> nodeVsTags) {
-    List<String> errors = new ArrayList<>();
-    for (Rule rule : rules) {
-      for (Map.Entry<String, Map<String, Object>> e : nodeVsTags.entrySet()) {
-        if (e.getValue().get(rule.tag.name) == null) {
-          errors.add(formatString("The value for tag {0} is not available for node {1}", rule.tag.name, e.getKey()));
-        }
-      }
-    }
-    if (!errors.isEmpty()) {
-      throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, StrUtils.join(errors, ','));
-    }
-  }
-
-
   /**
    * get all permutations for the int[] whose items are 0..level
    */
@@ -422,14 +403,12 @@ public class ReplicaAssigner {
           context.exception = new SolrException(SolrException.ErrorCode.SERVER_ERROR,
               "Not all tags were obtained from node " + node);
         } else {
-          if (context.getTags().keySet().containsAll(context.snitchInfo.getTagNames())) {
-            Map<String, Object> tags = result.get(node);
-            if (tags == null) {
-              tags = new HashMap<>();
-              result.put(node, tags);
-            }
-            tags.putAll(context.getTags());
+          Map<String, Object> tags = result.get(node);
+          if (tags == null) {
+            tags = new HashMap<>();
+            result.put(node, tags);
           }
+          tags.putAll(context.getTags());
         }
       }
     }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/2b66d0cb/solr/core/src/java/org/apache/solr/cloud/rule/Rule.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/rule/Rule.java b/solr/core/src/java/org/apache/solr/cloud/rule/Rule.java
index 97947cf..87bbe69 100644
--- a/solr/core/src/java/org/apache/solr/cloud/rule/Rule.java
+++ b/solr/core/src/java/org/apache/solr/cloud/rule/Rule.java
@@ -202,7 +202,7 @@ public class Rule {
 
       @Override
       public boolean canMatch(Object ruleVal, Object testVal) {
-        return compareNum(ruleVal, testVal) == 1;
+        return testVal != null && compareNum(ruleVal, testVal) == 1;
       }
 
     },
@@ -214,7 +214,7 @@ public class Rule {
 
       @Override
       public boolean canMatch(Object ruleVal, Object testVal) {
-        return compareNum(ruleVal, testVal) == -1;
+        return testVal != null && compareNum(ruleVal, testVal) == -1;
       }
 
       @Override

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/2b66d0cb/solr/core/src/test/org/apache/solr/cloud/rule/RuleEngineTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/rule/RuleEngineTest.java b/solr/core/src/test/org/apache/solr/cloud/rule/RuleEngineTest.java
index 7c33541..8b0a788 100644
--- a/solr/core/src/test/org/apache/solr/cloud/rule/RuleEngineTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/rule/RuleEngineTest.java
@@ -85,7 +85,7 @@ public class RuleEngineTest extends SolrTestCaseJ4{
         new HashMap(), new ArrayList<>(MockSnitch.nodeVsTags.keySet()), null, null ).getNodeMappings();
     assertNotNull(mapping);
 
-    rules = parseRules("[{role:'!overseer'}]" );
+    rules = parseRules("[{role:'!overseer'}, {'freedisk':'>1'}]" );
     Map<String, Object> snitchSession = new HashMap<>();
     List<String> preferredOverseerNodes = ImmutableList.of("127.0.0.1:49947_", "127.0.0.1:49952_");
     ReplicaAssigner replicaAssigner = new ReplicaAssigner(