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 2018/01/22 13:13:03 UTC

lucene-solr:master: SOLR-11871: MoveReplicaSuggester should not suggest leader if other replicas are available

Repository: lucene-solr
Updated Branches:
  refs/heads/master f72a5dbdc -> 876ecd87f


SOLR-11871: MoveReplicaSuggester should not suggest leader if other replicas are available


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

Branch: refs/heads/master
Commit: 876ecd87fb44b828785681238915eb0e1965ad58
Parents: f72a5db
Author: Noble Paul <no...@apache.org>
Authored: Tue Jan 23 00:12:51 2018 +1100
Committer: Noble Paul <no...@apache.org>
Committed: Tue Jan 23 00:12:51 2018 +1100

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  2 ++
 .../cloud/autoscaling/MoveReplicaSuggester.java | 10 +++++++-
 .../solrj/cloud/autoscaling/ReplicaInfo.java    |  7 +++++
 .../apache/solr/common/cloud/ZkNodeProps.java   | 11 ++++----
 .../solrj/cloud/autoscaling/TestPolicy.java     | 27 ++++++++++++++++++++
 5 files changed, 51 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/876ecd87/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 7f63679..9d3cbcb 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -163,6 +163,8 @@ Other Changes
 
 * SOLR-11747: Pause triggers until actions finish executing and the cool down period expires. (shalin)
 
+* SOLR-11871: MoveReplicaSuggester should not suggest leader if other replicas are available (noble)
+
 ==================  7.2.1 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/876ecd87/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/MoveReplicaSuggester.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/MoveReplicaSuggester.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/MoveReplicaSuggester.java
index 25e75ad..d5918e5 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/MoveReplicaSuggester.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/MoveReplicaSuggester.java
@@ -18,6 +18,7 @@
 package org.apache.solr.client.solrj.cloud.autoscaling;
 
 import java.io.IOException;
+import java.util.Comparator;
 import java.util.List;
 
 import org.apache.solr.client.solrj.SolrRequest;
@@ -40,7 +41,9 @@ public class MoveReplicaSuggester extends Suggester {
     Integer targetNodeIndex = null;
     Integer sourceNodeIndex = null;
     ReplicaInfo sourceReplicaInfo = null;
-    for (Pair<ReplicaInfo, Row> fromReplica : getValidReplicas(true, true, -1)) {
+    List<Pair<ReplicaInfo, Row>> validReplicas = getValidReplicas(true, true, -1);
+    validReplicas.sort(leaderLast);
+    for (Pair<ReplicaInfo, Row> fromReplica : validReplicas) {
       Row fromRow = fromReplica.second();
       ReplicaInfo replicaInfo = fromReplica.first();
       String coll = replicaInfo.getCollection();
@@ -79,6 +82,11 @@ public class MoveReplicaSuggester extends Suggester {
     }
     return null;
   }
+  static Comparator<Pair<ReplicaInfo, Row>> leaderLast = (r1, r2) -> {
+    if (r1.first().isLeader) return 1;
+    if (r2.first().isLeader) return -1;
+    return 0;
+  };
 
 
   @Override

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/876ecd87/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaInfo.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaInfo.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaInfo.java
index 930ede8..cfcd956 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaInfo.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/ReplicaInfo.java
@@ -18,6 +18,7 @@
 package org.apache.solr.client.solrj.cloud.autoscaling;
 
 import java.io.IOException;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
@@ -27,6 +28,8 @@ import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.util.Utils;
 
+import static org.apache.solr.common.cloud.ZkStateReader.LEADER_PROP;
+
 
 public class ReplicaInfo implements MapWriter {
 //  private final Replica replica;
@@ -34,6 +37,7 @@ public class ReplicaInfo implements MapWriter {
   private String core, collection, shard;
   private Replica.Type type;
   private String node;
+  public final boolean isLeader;
   private final Map<String, Object> variables = new HashMap<>();
 
   public ReplicaInfo(String coll, String shard, Replica r, Map<String, Object> vals) {
@@ -42,6 +46,7 @@ public class ReplicaInfo implements MapWriter {
     this.collection = coll;
     this.shard = shard;
     this.type = r.getType();
+    this.isLeader = r.getBool(LEADER_PROP, false);
     if (vals != null) {
       this.variables.putAll(vals);
     }
@@ -49,10 +54,12 @@ public class ReplicaInfo implements MapWriter {
   }
 
   public ReplicaInfo(String name, String core, String coll, String shard, Replica.Type type, String node, Map<String, Object> vals) {
+    if(vals==null) vals = Collections.emptyMap();
     this.name = name;
     if (vals != null) {
       this.variables.putAll(vals);
     }
+    this.isLeader = "true".equals(String.valueOf(vals.getOrDefault(LEADER_PROP, "false")));
     this.collection = coll;
     this.shard = shard;
     this.type = type;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/876ecd87/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java
index 94a673e..93fe59a 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkNodeProps.java
@@ -16,15 +16,15 @@
  */
 package org.apache.solr.common.cloud;
 
-import org.apache.solr.common.util.Utils;
-import org.noggit.JSONUtil;
-import org.noggit.JSONWriter;
-
 import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Set;
 
+import org.apache.solr.common.util.Utils;
+import org.noggit.JSONUtil;
+import org.noggit.JSONWriter;
+
 /**
  * ZkNodeProps contains generic immutable properties.
  */
@@ -148,7 +148,8 @@ public class ZkNodeProps implements JSONWriter.Writable {
 
   public boolean getBool(String key, boolean b) {
     Object o = propMap.get(key);
-    if(o==null) return b;
+    if (o == null) return b;
+    if (o instanceof Boolean) return (boolean) o;
     return Boolean.parseBoolean(o.toString());
   }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/876ecd87/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java
----------------------------------------------------------------------
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 2c119f3..2e509bb 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
@@ -1744,5 +1744,32 @@ public class TestPolicy extends SolrTestCaseJ4 {
         cloudManager, null, Arrays.asList("shard1", "shard2"), 1, 0, 0, null);
     assertTrue(locations.stream().allMatch(it -> "node3".equals(it.node)));
   }
+  public void testMoveReplicaLeaderlast(){
+
+    List<Pair<ReplicaInfo, Row>> validReplicas =  new ArrayList<>();
+    Replica replica = new Replica("r1", Utils.makeMap("leader", "true"));
+    ReplicaInfo replicaInfo = new ReplicaInfo("c1", "s1", replica, new HashMap<>());
+    validReplicas.add(new Pair<>(replicaInfo, null));
+
+    replicaInfo = new ReplicaInfo("r4", "c1_s2_r1","c1", "s2", Replica.Type.NRT, "n1", Collections.singletonMap("leader", "true"));
+    validReplicas.add(new Pair<>(replicaInfo, null));
+
+
+    replica = new Replica("r2", Utils.makeMap("leader", false));
+    replicaInfo = new ReplicaInfo("c1", "s1", replica, new HashMap<>());
+    validReplicas.add(new Pair<>(replicaInfo, null));
+
+    replica = new Replica("r3", Utils.makeMap("leader", false));
+    replicaInfo = new ReplicaInfo("c1", "s1", replica, new HashMap<>());
+    validReplicas.add(new Pair<>(replicaInfo, null));
+
+
+    validReplicas.sort(MoveReplicaSuggester.leaderLast);
+    assertEquals("r2", validReplicas.get(0).first().getName());
+    assertEquals("r3", validReplicas.get(1).first().getName());
+    assertEquals("r1", validReplicas.get(2).first().getName());
+    assertEquals("r4", validReplicas.get(3).first().getName());
+
+  }
 
 }