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/06/22 15:04:12 UTC

lucene-solr:jira/SOLR-10496: SOLR-10496: MOVEREPLICA suggester for dead node is not working

Repository: lucene-solr
Updated Branches:
  refs/heads/jira/SOLR-10496 cb665a1b3 -> b78e7ff58


SOLR-10496: MOVEREPLICA suggester for dead node is not working


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

Branch: refs/heads/jira/SOLR-10496
Commit: b78e7ff589510315463488cd1ed79244d9d6e6ad
Parents: cb665a1
Author: Noble Paul <no...@apache.org>
Authored: Fri Jun 23 00:34:01 2017 +0930
Committer: Noble Paul <no...@apache.org>
Committed: Fri Jun 23 00:34:01 2017 +0930

----------------------------------------------------------------------
 .../solrj/impl/SolrClientDataProvider.java      |   4 +-
 .../cloud/autoscaling/AddReplicaSuggester.java  |   1 +
 .../cloud/autoscaling/MoveReplicaSuggester.java |   4 +
 .../apache/solr/cloud/autoscaling/Policy.java   |   4 +-
 .../solr/cloud/autoscaling/Preference.java      |   3 +
 .../org/apache/solr/cloud/autoscaling/Row.java  |  17 ++-
 .../solr/cloud/autoscaling/TestPolicy.java      | 118 ++++++++++++++++++-
 7 files changed, 143 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/b78e7ff5/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientDataProvider.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientDataProvider.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientDataProvider.java
index e40f32b..f77e54e 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientDataProvider.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientDataProvider.java
@@ -29,12 +29,14 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.function.Function;
 
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.SolrServerException;
 import org.apache.solr.client.solrj.request.GenericSolrRequest;
 import org.apache.solr.client.solrj.response.SimpleSolrResponse;
 import org.apache.solr.cloud.autoscaling.ClusterDataProvider;
+import org.apache.solr.cloud.autoscaling.Policy;
 import org.apache.solr.cloud.autoscaling.Policy.ReplicaInfo;
 import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.SolrException;
@@ -103,7 +105,7 @@ public class SolrClientDataProvider implements ClusterDataProvider, MapWriter {
 
   @Override
   public Map<String, Map<String, List<ReplicaInfo>>> getReplicaInfo(String node, Collection<String> keys) {
-    return data.getOrDefault(node, Collections.emptyMap());//todo fill other details
+    return data.computeIfAbsent(node, s -> Collections.emptyMap());//todo fill other details
   }
 
   @Override

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/b78e7ff5/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/AddReplicaSuggester.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/AddReplicaSuggester.java b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/AddReplicaSuggester.java
index 354851e..5870777 100644
--- a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/AddReplicaSuggester.java
+++ b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/AddReplicaSuggester.java
@@ -42,6 +42,7 @@ class AddReplicaSuggester extends Suggester {
     Integer targetNodeIndex = null;
     for (int i = getMatrix().size() - 1; i >= 0; i--) {
       Row row = getMatrix().get(i);
+      if(!row.isLive) continue;
       if (!isAllowed(row.node, Hint.TARGET_NODE)) continue;
       Row tmpRow = row.addReplica(coll, shard);
       tmpRow.violations.clear();

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/b78e7ff5/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/MoveReplicaSuggester.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/MoveReplicaSuggester.java b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/MoveReplicaSuggester.java
index 97aef51..99c5c3d 100644
--- a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/MoveReplicaSuggester.java
+++ b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/MoveReplicaSuggester.java
@@ -17,6 +17,7 @@
 
 package org.apache.solr.cloud.autoscaling;
 
+import java.util.LinkedHashMap;
 import java.util.List;
 
 import org.apache.solr.client.solrj.SolrRequest;
@@ -24,7 +25,9 @@ import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.cloud.autoscaling.Clause.Violation;
 import org.apache.solr.cloud.autoscaling.Policy.ReplicaInfo;
 import org.apache.solr.cloud.autoscaling.Policy.Suggester;
+import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.util.Pair;
+import org.apache.solr.common.util.Utils;
 
 public class MoveReplicaSuggester extends Suggester {
 
@@ -57,6 +60,7 @@ public class MoveReplicaSuggester extends Suggester {
       final int i = getMatrix().indexOf(fromRow);
       for (int j = getMatrix().size() - 1; j > i; j--) {
         Row targetRow = getMatrix().get(j);
+        if(!targetRow.isLive) continue;
         if (!isAllowed(targetRow.node, Hint.TARGET_NODE)) continue;
         targetRow = targetRow.addReplica(coll, shard);
         targetRow.violations.clear();

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/b78e7ff5/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Policy.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Policy.java b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Policy.java
index e264b67..33088e2 100644
--- a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Policy.java
+++ b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Policy.java
@@ -32,6 +32,7 @@ import java.util.Objects;
 import java.util.Set;
 import java.util.SortedSet;
 import java.util.TreeSet;
+import java.util.function.Predicate;
 import java.util.function.Supplier;
 import java.util.stream.Collectors;
 
@@ -379,9 +380,10 @@ public class Policy implements MapWriter {
           }
         }
         Set<String> srcNodes = (Set<String>) hints.get(Hint.SRC_NODE);
-        if (srcNodes != null && !srcNodes.isEmpty() && session.matrix.stream().noneMatch(row -> srcNodes.contains(row.node))) {
+        if (srcNodes != null && !srcNodes.isEmpty()) {
           // the source node is dead so live nodes may not have it
           for (String srcNode : srcNodes) {
+            if(session.matrix.stream().noneMatch(row -> row.node.equals(srcNode)))
             session.matrix.add(new Row(srcNode, session.getPolicy().params, session.dataProvider));
           }
         }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/b78e7ff5/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Preference.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Preference.java b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Preference.java
index 0566d25..16ece02 100644
--- a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Preference.java
+++ b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Preference.java
@@ -53,6 +53,9 @@ class Preference implements MapWriter {
   // recursive, it uses the precision to tie & when there is a tie use the next preference to compare
   // in non-recursive mode, precision is not taken into consideration and sort is done on actual value
   int compare(Row r1, Row r2, boolean useApprox) {
+    if (!r1.isLive && !r2.isLive) return 0;
+    if (!r1.isLive) return -1;
+    if (!r2.isLive) return 1;
     Object o1 = useApprox ? r1.cells[idx].approxVal : r1.cells[idx].val;
     Object o2 = useApprox ? r2.cells[idx].approxVal : r2.cells[idx].val;
     int result = 0;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/b78e7ff5/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Row.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Row.java b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Row.java
index a13633d..7c7b467 100644
--- a/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Row.java
+++ b/solr/solrj/src/java/org/apache/solr/cloud/autoscaling/Row.java
@@ -19,6 +19,7 @@ package org.apache.solr.cloud.autoscaling;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -47,7 +48,7 @@ class Row implements MapWriter {
     this.node = node;
     cells = new Cell[params.size()];
     isLive = dataProvider.getNodes().contains(node);
-    Map<String, Object> vals = dataProvider.getNodeValues(node, params);
+    Map<String, Object> vals = isLive ? dataProvider.getNodeValues(node, params) : Collections.emptyMap();
     for (int i = 0; i < params.size(); i++) {
       String s = params.get(i);
       cells[i] = new Cell(i, s, Clause.validate(s,vals.get(s), false));
@@ -56,8 +57,10 @@ class Row implements MapWriter {
     }
   }
 
-  Row(String node, Cell[] cells, boolean anyValueMissing, Map<String, Map<String, List<ReplicaInfo>>> collectionVsShardVsReplicas, List<Clause> violations) {
+  Row(String node, Cell[] cells, boolean anyValueMissing, Map<String,
+      Map<String, List<ReplicaInfo>>> collectionVsShardVsReplicas, List<Clause> violations, boolean isLive) {
     this.node = node;
+    this.isLive = isLive;
     this.cells = new Cell[cells.length];
     for (int i = 0; i < this.cells.length; i++) {
       this.cells[i] = cells[i].copy();
@@ -77,7 +80,7 @@ class Row implements MapWriter {
   }
 
   Row copy() {
-    return new Row(node, cells, anyValueMissing, Utils.getDeepCopy(collectionVsShardVsReplicas, 3), new ArrayList<>(violations));
+    return new Row(node, cells, anyValueMissing, Utils.getDeepCopy(collectionVsShardVsReplicas, 3), new ArrayList<>(violations), isLive);
   }
 
   Object getVal(String name) {
@@ -97,7 +100,9 @@ class Row implements MapWriter {
     List<ReplicaInfo> replicas = c.computeIfAbsent(shard, k -> new ArrayList<>());
     replicas.add(new ReplicaInfo("" + new Random().nextInt(1000) + 1000, coll, shard, new HashMap<>()));
     for (Cell cell : row.cells) {
-      if (cell.name.equals("cores")) cell.val = ((Number) cell.val).longValue() + 1;
+      if (cell.name.equals("cores")) {
+        cell.val = cell.val == null ? 0 : ((Number) cell.val).longValue() + 1;
+      }
     }
     return row;
 
@@ -110,7 +115,9 @@ class Row implements MapWriter {
     List<ReplicaInfo> s = c.get(shard);
     if (s == null || s.isEmpty()) return null;
     for (Cell cell : row.cells) {
-      if (cell.name.equals("cores")) cell.val = ((Number) cell.val).longValue() -1;
+      if (cell.name.equals("cores")) {
+        cell.val = cell.val == null ? 0 : ((Number) cell.val).longValue() - 1;
+      }
     }
     return new Pair(row, s.remove(0));
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/b78e7ff5/solr/solrj/src/test/org/apache/solr/cloud/autoscaling/TestPolicy.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/cloud/autoscaling/TestPolicy.java b/solr/solrj/src/test/org/apache/solr/cloud/autoscaling/TestPolicy.java
index 8c296b9..18c150a 100644
--- a/solr/solrj/src/test/org/apache/solr/cloud/autoscaling/TestPolicy.java
+++ b/solr/solrj/src/test/org/apache/solr/cloud/autoscaling/TestPolicy.java
@@ -27,6 +27,7 @@ import java.util.HashMap;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.function.BiConsumer;
 
 import com.google.common.collect.ImmutableList;
 import org.apache.solr.SolrTestCaseJ4;
@@ -188,9 +189,124 @@ public class TestPolicy extends SolrTestCaseJ4 {
     assertFalse(c.tag.isPass("12.6"));
     assertFalse(c.tag.isPass(12.6d));
   }
+  
+  public void testNodeLost() {
+    String dataproviderdata = " {'liveNodes':[" +
+        "    '127.0.0.1:65417_solr'," +
+        "    '127.0.0.1:65434_solr']," +
+        "  'replicaInfo':{" +
+        "    '127.0.0.1:65427_solr':{'testNodeLost':{'shard1':[{'core_node2':{}}]}}," +
+        "    '127.0.0.1:65417_solr':{'testNodeLost':{'shard1':[{'core_node1':{}}]}}," +
+        "    '127.0.0.1:65434_solr':{}}," +
+        "  'nodeValues':{" +
+        "    '127.0.0.1:65417_solr':{" +
+        "      'node':'127.0.0.1:65417_solr'," +
+        "      'cores':1," +
+        "      'freedisk':884.7097854614258}," +
+        "    '127.0.0.1:65434_solr':{" +
+        "      'node':'127.0.0.1:65434_solr'," +
+        "      'cores':0," +
+        "      'freedisk':884.7097854614258}}}";
+   /* String stateJson = "{'testNodeLost':{" +
+        "           'pullReplicas':'0'," +
+        "           'replicationFactor':'2'," +
+        "           'router':{'name':'compositeId'}," +
+        "           'maxShardsPerNode':'1'," +
+        "           'autoAddReplicas':'false'," +
+        "           'nrtReplicas':'2'," +
+        "           'tlogReplicas':'0'," +
+        "           'shards':{'shard1':{" +
+        "               'range':'80000000-7fffffff'," +
+        "               'state':'active'," +
+        "               'replicas':{" +
+        "                 'core_node1':{" +
+        "                   'core':'testNodeLost_shard1_replica_n1'," +
+        "                   'base_url':'http://127.0.0.1:65417/solr'," +
+        "                   'node_name':'127.0.0.1:65417_solr'," +
+        "                   'state':'active'," +
+        "                   'type':'NRT'," +
+        "                   'leader':'true'}," +
+        "                 'core_node2':{" +
+        "                   'core':'testNodeLost_shard1_replica_n2'," +
+        "                   'base_url':'http://127.0.0.1:65427/solr'," +
+        "                   'node_name':'127.0.0.1:65427_solr'," +
+        "                   'state':'down'," +
+        "                   'type':'NRT'}}}}}}";*/
+
+    String autoScalingjson = "{" +
+        "       'cluster-policy':[" +
+        "         {" +
+        "           'cores':'<10'," +
+        "           'node':'#ANY'}," +
+        "         {" +
+        "           'replica':'<2'," +
+        "           'shard':'#EACH'," +
+        "           'node':'#ANY'}," +
+        "         {" +
+        "           'nodeRole':'overseer'," +
+        "           'replica':0}]," +
+        "       'cluster-preferences':[" +
+        "         {" +
+        "           'minimize':'cores'," +
+        "           'precision':3}," +
+        "         {" +
+        "           'maximize':'freedisk'," +
+        "           'precision':100}]}";
+    
+    Policy policy = new Policy((Map<String, Object>) Utils.fromJSONString(autoScalingjson));
+    Policy.Session session = policy.createSession(dataProviderWithData(dataproviderdata));
+    SolrRequest op = session.getSuggester(MOVEREPLICA).hint(Hint.SRC_NODE, "127.0.0.1:65427_solr").getOperation();
+    assertNotNull(op);
+    assertEquals( "127.0.0.1:65434_solr",op.getParams().get("targetNode") );
+  }
+
+  private static ClusterDataProvider dataProviderWithData(String data){
+    final Map m = (Map) Utils.fromJSONString(data);
+    Map replicaInfo = (Map) m.get("replicaInfo");
+    replicaInfo.forEach((node, val) -> {
+      Map m1 = (Map) val;
+      m1.forEach((coll, val2) -> {
+        Map m2 = (Map) val2;
+        m2.forEach((shard, val3) -> {
+          List l3 = (List) val3;
+          for (int i = 0; i < l3.size(); i++) {
+            Object o = l3.get(i);
+            Map m3 = (Map) o;
+            l3.set(i, new Policy.ReplicaInfo(m3.keySet().iterator().next().toString()
+                ,coll.toString(), shard.toString(),new HashMap<>()));
+          }
+        });
+
+      });
+
+    });
+    return new ClusterDataProvider(){
+      @Override
+      public Map<String, Object> getNodeValues(String node, Collection<String> tags) {
+        return (Map<String, Object>) Utils.getObjectByPath(m,false, Arrays.asList("nodeValues", node));
+      }
+
+      @Override
+      public Map<String, Map<String, List<Policy.ReplicaInfo>>> getReplicaInfo(String node, Collection<String> keys) {
+        return (Map<String, Map<String, List<Policy.ReplicaInfo>>>) Utils.getObjectByPath(m,false, Arrays.asList("replicaInfo", node));
+      }
+
+      @Override
+      public Collection<String> getNodes() {
+        return (Collection<String>) m.get("liveNodes");
+      }
+
+      @Override
+      public String getPolicyNameByCollection(String coll) {
+        return null;
+      }
+    };
+
+
+  }
 
   public void testRow() {
-    Row row = new Row("nodex", new Cell[]{new Cell(0, "node", "nodex")}, false, new HashMap<>(), new ArrayList<>());
+    Row row = new Row("nodex", new Cell[]{new Cell(0, "node", "nodex")}, false, new HashMap<>(), new ArrayList<>(), true);
     Row r1 = row.addReplica("c1", "s1");
     Row r2 = r1.addReplica("c1", "s1");
     assertEquals(1, r1.collectionVsShardVsReplicas.get("c1").get("s1").size());