You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by sh...@apache.org on 2018/07/24 07:23:33 UTC

lucene-solr:jira/solr-11990: SOLR-11990: Pass strict-ness parameter to the ConditionType so that WITH_COLLECTION can choose not to project add replica in strict mode.

Repository: lucene-solr
Updated Branches:
  refs/heads/jira/solr-11990 0d616ed9e -> 1de2a4f52


SOLR-11990: Pass strict-ness parameter to the ConditionType so that WITH_COLLECTION can choose not to project add replica in strict mode.

This ensures that add replica or move replica suggesters always choose nodes that already have withCollection replicas first unless there are violations in doing so. Only if the first pass fails to find a suitable replica, do we go to the other nodes in the cluster. This also removes the need for the majority of changes in AddReplicaSuggester and so they've been reverted.


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

Branch: refs/heads/jira/solr-11990
Commit: 1de2a4f52a59afca28de75bfa5156a3d6567a4f5
Parents: 0d616ed
Author: Shalin Shekhar Mangar <sh...@apache.org>
Authored: Tue Jul 24 12:53:26 2018 +0530
Committer: Shalin Shekhar Mangar <sh...@apache.org>
Committed: Tue Jul 24 12:53:26 2018 +0530

----------------------------------------------------------------------
 .../cloud/autoscaling/AddReplicaSuggester.java  | 61 ++------------------
 .../cloud/autoscaling/MoveReplicaSuggester.java |  4 +-
 .../client/solrj/cloud/autoscaling/Row.java     | 15 +++--
 .../solrj/cloud/autoscaling/Suggestion.java     | 13 +++--
 4 files changed, 25 insertions(+), 68 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/1de2a4f5/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AddReplicaSuggester.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AddReplicaSuggester.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AddReplicaSuggester.java
index a15a4cc..2a31e71 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AddReplicaSuggester.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/AddReplicaSuggester.java
@@ -17,8 +17,6 @@
 
 package org.apache.solr.client.solrj.cloud.autoscaling;
 
-import java.lang.invoke.MethodHandles;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
@@ -28,65 +26,14 @@ import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.common.cloud.Replica;
 import org.apache.solr.common.params.CollectionParams;
 import org.apache.solr.common.util.Pair;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import static org.apache.solr.common.params.CollectionParams.CollectionAction.ADDREPLICA;
 
 class AddReplicaSuggester extends Suggester {
-  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   SolrRequest init() {
-    Set<String> collections = (Set<String>) hints.getOrDefault(Hint.COLL, Collections.emptySet());
-    Set<Pair<String, String>> s = (Set<Pair<String, String>>) hints.getOrDefault(Hint.COLL_SHARD, Collections.emptySet());
-
-    String withCollection = findWithCollection(collections, s);
-
-    // If withCollection is present, try to allocate replicas to nodes which already have the withCollection
-    // replicas and do not have  any violations
-    Collection originalTargetNodesCopy = setupWithCollectionTargetNodes(collections, s, withCollection);
-
-    log.debug("Generating suggestions with hints: {}", hints);
-
     SolrRequest operation = tryEachNode(true);
-    if (operation == null && withCollection != null) {
-      // it was not possible to place the replica in nodes which already have that withCollection.
-      // try to place the replica in any node. This may add replica of withCollection to that node (if not present)
-      // to do that, we must restore original target nodes and try again
-      if (originalTargetNodesCopy != null) {
-        hints.put(Hint.TARGET_NODE, originalTargetNodesCopy);
-      } else {
-        hints.remove(Hint.TARGET_NODE);
-      }
-
-      operation = tryEachNode(true);
-      // we still can't get a valid node
-      if (operation == null) {
-        for (Row row : getMatrix()) {
-          row.forEachReplica(r -> {
-            if (withCollection.equals(r.getCollection()))
-              hint(Hint.TARGET_NODE, row.node);
-          });
-        }
-
-        // previously, we tried to allocate replicas without ignoring the non strict rules
-        // (but they have the withCollection replicas)
-        // now we are going to try by ignoring the no strict rules
-        operation = tryEachNode(false);
-        if (operation == null) {
-          // we still can't get a node where we can place a replica
-          // restore original target nodes and try again
-          if (originalTargetNodesCopy != null) {
-            hints.put(Hint.TARGET_NODE, originalTargetNodesCopy);
-          } else {
-            hints.remove(Hint.TARGET_NODE);
-          }
-          operation = tryEachNode(false);
-        }
-      }
-    } else if (operation == null) {
-      operation = tryEachNode(false);
-    }
+    if (operation == null) operation = tryEachNode(false);
     return operation;
   }
 
@@ -97,13 +44,13 @@ class AddReplicaSuggester extends Suggester {
     }
     for (Pair<String, String> shard : shards) {
       Replica.Type type = Replica.Type.get((String) hints.get(Hint.REPLICATYPE));
-      // iterate through nodes and identify the least loaded
+      //iterate through  nodes and identify the least loaded
       List<Violation> leastSeriousViolation = null;
       Row bestNode = null;
       for (int i = getMatrix().size() - 1; i >= 0; i--) {
         Row row = getMatrix().get(i);
         if (!isNodeSuitableForReplicaAddition(row)) continue;
-        Row tmpRow = row.addReplica(shard.first(), shard.second(), type);
+        Row tmpRow = row.addReplica(shard.first(), shard.second(), type, strict);
         List<Violation> errs = testChangedMatrix(strict, tmpRow.session);
 
         if (!containsNewErrors(errs)) {
@@ -130,4 +77,4 @@ class AddReplicaSuggester extends Suggester {
   public CollectionParams.CollectionAction getAction() {
     return ADDREPLICA;
   }
-}
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/1de2a4f5/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 026b667..fffaee2 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
@@ -60,10 +60,10 @@ public class MoveReplicaSuggester extends Suggester {
         targetRow = session.matrix.get(j);
         if (targetRow.node.equals(fromRow.node)) continue;
         if (!isNodeSuitableForReplicaAddition(targetRow)) continue;
-        targetRow = targetRow.addReplica(ri.getCollection(), ri.getShard(), ri.getType());//add replica to target first
+        targetRow = targetRow.addReplica(ri.getCollection(), ri.getShard(), ri.getType(), strict); // add replica to target first
         Row srcRowModified = targetRow.session.getNode(fromRow.node).removeReplica(ri.getCollection(), ri.getShard(), ri.getType());//then remove replica from source node
         List<Violation> errs = testChangedMatrix(strict, srcRowModified.session);
-        srcRowModified.session.applyRules();// now resort the nodes with the new values
+        srcRowModified.session.applyRules(); // now resort the nodes with the new values
         Policy.Session tmpSession = srcRowModified.session;
 
         if (!containsNewErrors(errs) &&

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/1de2a4f5/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Row.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Row.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Row.java
index a2c1cd2..4b860cd 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Row.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Row.java
@@ -116,7 +116,11 @@ public class Row implements MapWriter {
   }
 
   public Row addReplica(String coll, String shard, Replica.Type type) {
-    return addReplica(coll, shard, type, 0);
+    return addReplica(coll, shard, type, 0, true);
+  }
+
+  public Row addReplica(String coll, String shard, Replica.Type type, boolean strictMode) {
+    return addReplica(coll, shard, type, 0, strictMode);
   }
 
   /**
@@ -127,12 +131,13 @@ public class Row implements MapWriter {
    * @param coll  collection name
    * @param shard shard name
    * @param type  replica type
+   * @param recursionCount the number of times we have recursed to add more replicas
+   * @param strictMode whether suggester is operating in strict mode or not
    */
-  Row addReplica(String coll, String shard, Replica.Type type, int recursionCount) {
+  Row addReplica(String coll, String shard, Replica.Type type, int recursionCount, boolean strictMode) {
     if (recursionCount > 3) {
       log.error("more than 3 levels of recursion ", new RuntimeException());
       return this;
-
     }
     List<OperationInfo> furtherOps = new LinkedList<>();
     Consumer<OperationInfo> opCollector = it -> furtherOps.add(it);
@@ -146,11 +151,11 @@ public class Row implements MapWriter {
         Utils.makeMap(ZkStateReader.REPLICA_TYPE, type != null ? type.toString() : Replica.Type.NRT.toString()));
     replicas.add(ri);
     for (Cell cell : row.cells) {
-      cell.type.projectAddReplica(cell, ri, opCollector);
+      cell.type.projectAddReplica(cell, ri, opCollector, strictMode);
     }
     for (OperationInfo op : furtherOps) {
       if (op.isAdd) {
-        row = row.session.getNode(op.node).addReplica(op.coll, op.shard, op.type, recursionCount + 1);
+        row = row.session.getNode(op.node).addReplica(op.coll, op.shard, op.type, recursionCount + 1, strictMode);
       } else {
         row.session.getNode(op.node).removeReplica(op.coll, op.shard, op.type, recursionCount+1);
       }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/1de2a4f5/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggestion.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggestion.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggestion.java
index 18b3296..115b6ef 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggestion.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggestion.java
@@ -338,7 +338,7 @@ public class Suggestion {
 
       //When a replica is added, freedisk should be incremented
       @Override
-      public void projectAddReplica(Cell cell, ReplicaInfo ri, Consumer<Row.OperationInfo> ops) {
+      public void projectAddReplica(Cell cell, ReplicaInfo ri, Consumer<Row.OperationInfo> ops, boolean strictMode) {
         //go through other replicas of this shard and copy the index size value into this
         for (Row row : cell.getRow().session.matrix) {
           row.forEachReplica(replicaInfo -> {
@@ -427,7 +427,7 @@ public class Suggestion {
       }
 
       @Override
-      public void projectAddReplica(Cell cell, ReplicaInfo ri, Consumer<Row.OperationInfo> ops) {
+      public void projectAddReplica(Cell cell, ReplicaInfo ri, Consumer<Row.OperationInfo> ops, boolean strictMode) {
         cell.val = cell.val == null ? 0 : ((Number) cell.val).longValue() + 1;
       }
 
@@ -529,7 +529,12 @@ public class Suggestion {
       }
 
       @Override
-      public void projectAddReplica(Cell cell, ReplicaInfo ri, Consumer<Row.OperationInfo> opCollector) {
+      public void projectAddReplica(Cell cell, ReplicaInfo ri, Consumer<Row.OperationInfo> opCollector, boolean strictMode) {
+        if (strictMode) {
+          // we do not want to add a replica of the 'withCollection' in strict mode
+          return;
+        }
+
         Map<String, String> withCollectionMap = (Map<String, String>) cell.val;
         if (withCollectionMap == null || withCollectionMap.isEmpty()) return;
 
@@ -730,7 +735,7 @@ public class Suggestion {
     /**
      * Simulate a replica addition to a node in the cluster
      */
-    public void projectAddReplica(Cell cell, ReplicaInfo ri, Consumer<Row.OperationInfo> opCollector) {
+    public void projectAddReplica(Cell cell, ReplicaInfo ri, Consumer<Row.OperationInfo> opCollector, boolean strictMode) {
     }
 
     public void projectRemoveReplica(Cell cell, ReplicaInfo ri, Consumer<Row.OperationInfo> opCollector) {