You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by er...@apache.org on 2015/07/11 07:11:32 UTC
svn commit: r1690348 - in /lucene/dev/branches/branch_5x: ./ solr/
solr/core/ solr/core/src/java/org/apache/solr/cloud/
solr/core/src/test/org/apache/solr/cloud/ solr/solrj/
solr/solrj/src/java/org/apache/solr/common/params/
Author: erick
Date: Sat Jul 11 05:11:31 2015
New Revision: 1690348
URL: http://svn.apache.org/r1690348
Log:
SOLR-7172: addreplica API fails with incorrect error msg 'cannot create collection'
Added:
lucene/dev/branches/branch_5x/solr/core/src/test/org/apache/solr/cloud/CollectionTooManyReplicasTest.java
- copied unchanged from r1690341, lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/CollectionTooManyReplicasTest.java
Modified:
lucene/dev/branches/branch_5x/ (props changed)
lucene/dev/branches/branch_5x/solr/ (props changed)
lucene/dev/branches/branch_5x/solr/CHANGES.txt (contents, props changed)
lucene/dev/branches/branch_5x/solr/core/ (props changed)
lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/cloud/Assign.java
lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java
lucene/dev/branches/branch_5x/solr/solrj/ (props changed)
lucene/dev/branches/branch_5x/solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java
Modified: lucene/dev/branches/branch_5x/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/CHANGES.txt?rev=1690348&r1=1690347&r2=1690348&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/CHANGES.txt (original)
+++ lucene/dev/branches/branch_5x/solr/CHANGES.txt Sat Jul 11 05:11:31 2015
@@ -161,6 +161,9 @@ Bug Fixes
* SOLR-7132: The Collections API ADDREPLICA command property.name is not reflected
in the clusterstate until after Solr restarts (Erick Erickson)
+* SOLR-7172: addreplica API fails with incorrect error msg "cannot create collection"
+ (Erick Erickson)
+
Optimizations
----------------------
Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/cloud/Assign.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/cloud/Assign.java?rev=1690348&r1=1690347&r2=1690348&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/cloud/Assign.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/cloud/Assign.java Sat Jul 11 05:11:31 2015
@@ -24,6 +24,7 @@ import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
+import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.regex.Matcher;
@@ -72,10 +73,10 @@ public class Assign {
return "core_node" + (max + 1);
}
-
+
/**
* Assign a new unique id up to slices count - then add replicas evenly.
- *
+ *
* @return the assigned shard id
*/
public static String assignShard(String collection, ClusterState state, Integer numShards) {
@@ -124,7 +125,7 @@ public class Assign {
static String buildCoreName(DocCollection collection, String shard) {
Slice slice = collection.getSlice(shard);
int replicaNum = slice.getReplicas().size();
- for (;;) {
+ for (; ; ) {
String replicaName = collection.getName() + "_" + shard + "_replica" + replicaNum;
boolean exists = false;
for (Replica replica : slice.getReplicas()) {
@@ -139,12 +140,12 @@ public class Assign {
return collection.getName() + "_" + shard + "_replica" + replicaNum;
}
- static class Node {
+ static class ReplicaCount {
public final String nodeName;
public int thisCollectionNodes = 0;
public int totalNodes = 0;
- Node(String nodeName) {
+ ReplicaCount(String nodeName) {
this.nodeName = nodeName;
}
@@ -153,32 +154,112 @@ public class Assign {
}
}
- public static List<Node> getNodesForNewShard(ClusterState clusterState, String collectionName,String shard,int numberOfNodes,
- String createNodeSetStr, CoreContainer cc) {
+ // Only called from createShard and addReplica (so far).
+ //
+ // Gets a list of candidate nodes to put the required replica(s) on. Throws errors if not enough replicas
+ // could be created on live nodes given maxShardsPerNode, Replication factor (if from createShard) etc.
+ public static List<ReplicaCount> getNodesForNewReplicas(ClusterState clusterState, String collectionName,
+ String shard, int numberOfNodes,
+ String createNodeSetStr, CoreContainer cc) {
DocCollection coll = clusterState.getCollection(collectionName);
Integer maxShardsPerNode = coll.getInt(MAX_SHARDS_PER_NODE, 1);
- Integer repFactor = coll.getInt(REPLICATION_FACTOR, 1);
- int numSlices = coll.getSlices().size();
List<String> createNodeList = createNodeSetStr == null ? null: StrUtils.splitSmart(createNodeSetStr, ",", true);
+ HashMap<String, ReplicaCount> nodeNameVsShardCount = getNodeNameVsShardCount(collectionName, clusterState, createNodeList);
+
+ if (createNodeList == null) { // We only care if we haven't been told to put new replicas on specific nodes.
+ int availableSlots = 0;
+ for (Map.Entry<String, ReplicaCount> ent : nodeNameVsShardCount.entrySet()) {
+ //ADDREPLICA can put more than maxShardsPerNode on an instnace, so this test is necessary.
+ if (maxShardsPerNode > ent.getValue().thisCollectionNodes) {
+ availableSlots += (maxShardsPerNode - ent.getValue().thisCollectionNodes);
+ }
+ }
+ if (availableSlots < numberOfNodes) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+ String.format(Locale.ROOT, "Cannot create %d new replicas for collection %s given the current number of live nodes and a maxShardsPerNode of %d",
+ numberOfNodes, collectionName, maxShardsPerNode));
+ }
+ }
+
+ List l = (List) coll.get(DocCollection.RULE);
+ if (l != null) {
+ return getNodesViaRules(clusterState, shard, numberOfNodes, cc, coll, createNodeList, l);
+ }
+
+ ArrayList<ReplicaCount> sortedNodeList = new ArrayList<>(nodeNameVsShardCount.values());
+ Collections.sort(sortedNodeList, new Comparator<ReplicaCount>() {
+ @Override
+ public int compare(ReplicaCount x, ReplicaCount y) {
+ return (x.weight() < y.weight()) ? -1 : ((x.weight() == y.weight()) ? 0 : 1);
+ }
+ });
+ return sortedNodeList;
+
+ }
+
+ private static List<ReplicaCount> getNodesViaRules(ClusterState clusterState, String shard, int numberOfNodes,
+ CoreContainer cc, DocCollection coll, List<String> createNodeList, List l) {
+ ArrayList<Rule> rules = new ArrayList<>();
+ for (Object o : l) rules.add(new Rule((Map) o));
+ Map<String, Map<String, Integer>> shardVsNodes = new LinkedHashMap<>();
+ for (Slice slice : coll.getSlices()) {
+ LinkedHashMap<String, Integer> n = new LinkedHashMap<>();
+ shardVsNodes.put(slice.getName(), n);
+ for (Replica replica : slice.getReplicas()) {
+ Integer count = n.get(replica.getNodeName());
+ if (count == null) count = 0;
+ n.put(replica.getNodeName(), ++count);
+ }
+ }
+ List snitches = (List) coll.get(DocCollection.SNITCH);
+ List<String> nodesList = createNodeList == null ?
+ new ArrayList<>(clusterState.getLiveNodes()) :
+ createNodeList;
+ Map<ReplicaAssigner.Position, String> positions = new ReplicaAssigner(
+ rules,
+ Collections.singletonMap(shard, numberOfNodes),
+ snitches,
+ shardVsNodes,
+ nodesList, cc, clusterState).getNodeMappings();
+
+ List<ReplicaCount> repCounts = new ArrayList<>();
+ for (String s : positions.values()) {
+ repCounts.add(new ReplicaCount(s));
+ }
+ return repCounts;
+ }
+
+ private static HashMap<String, ReplicaCount> getNodeNameVsShardCount(String collectionName,
+ ClusterState clusterState, List<String> createNodeList) {
Set<String> nodes = clusterState.getLiveNodes();
List<String> nodeList = new ArrayList<>(nodes.size());
nodeList.addAll(nodes);
if (createNodeList != null) nodeList.retainAll(createNodeList);
-
- HashMap<String,Node> nodeNameVsShardCount = new HashMap<>();
- for (String s : nodeList) nodeNameVsShardCount.put(s,new Node(s));
+ HashMap<String, ReplicaCount> nodeNameVsShardCount = new HashMap<>();
+ for (String s : nodeList) {
+ nodeNameVsShardCount.put(s, new ReplicaCount(s));
+ }
+ if (createNodeList != null) { // Overrides petty considerations about maxShardsPerNode
+ if (createNodeList.size() != nodeNameVsShardCount.size()) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+ "At least one of the node(s) specified are not currently active, no action taken.");
+ }
+ return nodeNameVsShardCount;
+ }
+ DocCollection coll = clusterState.getCollection(collectionName);
+ Integer maxShardsPerNode = coll.getInt(MAX_SHARDS_PER_NODE, 1);
for (String s : clusterState.getCollections()) {
DocCollection c = clusterState.getCollection(s);
//identify suitable nodes by checking the no:of cores in each of them
for (Slice slice : c.getSlices()) {
Collection<Replica> replicas = slice.getReplicas();
for (Replica replica : replicas) {
- Node count = nodeNameVsShardCount.get(replica.getNodeName());
+ ReplicaCount count = nodeNameVsShardCount.get(replica.getNodeName());
if (count != null) {
- count.totalNodes++;
+ count.totalNodes++; // Used ot "weigh" whether this node should be used later.
if (s.equals(collectionName)) {
count.thisCollectionNodes++;
if (count.thisCollectionNodes >= maxShardsPerNode) nodeNameVsShardCount.remove(replica.getNodeName());
@@ -188,77 +269,8 @@ public class Assign {
}
}
- if (nodeNameVsShardCount.size() <= 0) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Cannot create collection " + collectionName
- + ". No live Solr-instances" + ((createNodeList != null)?" among Solr-instances specified in " + CREATE_NODE_SET + ":" + createNodeSetStr:""));
- }
-
- if (repFactor > nodeNameVsShardCount.size()) {
- log.warn("Specified "
- + ZkStateReader.REPLICATION_FACTOR
- + " of "
- + repFactor
- + " on collection "
- + collectionName
- + " is higher than or equal to the number of Solr instances currently live or part of your " + CREATE_NODE_SET + "("
- + nodeList.size()
- + "). It's unusual to run two replica of the same slice on the same Solr-instance.");
- }
-
- int maxCoresAllowedToCreate = maxShardsPerNode * nodeList.size();
- int requestedCoresToCreate = numSlices * repFactor;
- int minCoresToCreate = requestedCoresToCreate;
- if (maxCoresAllowedToCreate < minCoresToCreate) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Cannot create shards " + collectionName + ". Value of "
- + MAX_SHARDS_PER_NODE + " is " + maxShardsPerNode
- + ", and the number of live nodes is " + nodeList.size()
- + ". This allows a maximum of " + maxCoresAllowedToCreate
- + " to be created. Value of " + NUM_SLICES + " is " + numSlices
- + " and value of " + ZkStateReader.REPLICATION_FACTOR + " is " + repFactor
- + ". This requires " + requestedCoresToCreate
- + " shards to be created (higher than the allowed number)");
- }
-
- List l = (List) coll.get(DocCollection.RULE);
- if(l != null) {
- ArrayList<Rule> rules = new ArrayList<>();
- for (Object o : l) rules.add(new Rule((Map) o));
- Map<String, Map<String,Integer>> shardVsNodes = new LinkedHashMap<>();
- for (Slice slice : coll.getSlices()) {
- LinkedHashMap<String, Integer> n = new LinkedHashMap<>();
- shardVsNodes.put(slice.getName(), n);
- for (Replica replica : slice.getReplicas()) {
- Integer count = n.get(replica.getNodeName());
- if(count == null) count = 0;
- n.put(replica.getNodeName(),++count);
- }
- }
- List snitches = (List) coll.get(DocCollection.SNITCH);
- List<String> nodesList = createNodeList == null ?
- new ArrayList<>(clusterState.getLiveNodes()) :
- createNodeList ;
- Map<ReplicaAssigner.Position, String> positions = new ReplicaAssigner(
- rules,
- Collections.singletonMap(shard, numberOfNodes),
- snitches,
- shardVsNodes,
- nodesList, cc, clusterState).getNodeMappings();
-
- List<Node> n = new ArrayList<>();
- for (String s : positions.values()) n.add(new Node(s));
- return n;
-
- }else {
-
- ArrayList<Node> sortedNodeList = new ArrayList<>(nodeNameVsShardCount.values());
- Collections.sort(sortedNodeList, new Comparator<Node>() {
- @Override
- public int compare(Node x, Node y) {
- return (x.weight() < y.weight()) ? -1 : ((x.weight() == y.weight()) ? 0 : 1);
- }
- });
- return sortedNodeList;
- }
+ return nodeNameVsShardCount;
}
+
}
Modified: lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java?rev=1690348&r1=1690347&r2=1690348&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java (original)
+++ lucene/dev/branches/branch_5x/solr/core/src/java/org/apache/solr/cloud/OverseerCollectionProcessor.java Sat Jul 11 05:11:31 2015
@@ -44,7 +44,7 @@ import org.apache.solr.client.solrj.requ
import org.apache.solr.client.solrj.request.CoreAdminRequest;
import org.apache.solr.client.solrj.request.UpdateRequest;
import org.apache.solr.client.solrj.response.UpdateResponse;
-import org.apache.solr.cloud.Assign.Node;
+import org.apache.solr.cloud.Assign.ReplicaCount;
import org.apache.solr.cloud.DistributedQueue.QueueEvent;
import org.apache.solr.cloud.Overseer.LeaderStatus;
import org.apache.solr.cloud.overseer.ClusterStateMutator;
@@ -95,7 +95,7 @@ import org.apache.zookeeper.data.Stat;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import static org.apache.solr.cloud.Assign.getNodesForNewShard;
+import static org.apache.solr.cloud.Assign.getNodesForNewReplicas;
import static org.apache.solr.common.cloud.DocCollection.SNITCH;
import static org.apache.solr.common.cloud.ZkNodeProps.makeMap;
import static org.apache.solr.common.cloud.ZkStateReader.BASE_URL_PROP;
@@ -1285,7 +1285,7 @@ public class OverseerCollectionProcessor
DocCollection collection = clusterState.getCollection(collectionName);
int repFactor = message.getInt(REPLICATION_FACTOR, collection.getInt(REPLICATION_FACTOR, 1));
String createNodeSetStr = message.getStr(CREATE_NODE_SET);
- List<Node> sortedNodeList = getNodesForNewShard(clusterState, collectionName, sliceName, repFactor,
+ List<ReplicaCount> sortedNodeList = getNodesForNewReplicas(clusterState, collectionName, sliceName, repFactor,
createNodeSetStr, overseer.getZkController().getCoreContainer());
Overseer.getInQueue(zkStateReader.getZkClient()).offer(ZkStateReader.toJSON(message));
@@ -2490,7 +2490,7 @@ public class OverseerCollectionProcessor
private void addReplica(ClusterState clusterState, ZkNodeProps message, NamedList results)
throws KeeperException, InterruptedException {
String collection = message.getStr(COLLECTION_PROP);
- String node = message.getStr("node");
+ String node = message.getStr(CoreAdminParams.NODE);
String shard = message.getStr(SHARD_ID_PROP);
String coreName = message.getStr(CoreAdminParams.NAME);
if (StringUtils.isBlank(coreName)) {
@@ -2508,13 +2508,12 @@ public class OverseerCollectionProcessor
"Collection: " + collection + " shard: " + shard + " does not exist");
}
ShardHandler shardHandler = shardHandlerFactory.getShardHandler();
-
- if (node == null) {
- node = getNodesForNewShard(clusterState, collection, shard, 1, null,
- overseer.getZkController().getCoreContainer()).get(0).nodeName;
- log.info("Node not provided, Identified {} for creating new replica", node);
- }
-
+
+ // Kind of unnecessary, but it does put the logic of whether to override maxShardsPerNode in one place.
+ node = getNodesForNewReplicas(clusterState, collection, shard, 1, node,
+ overseer.getZkController().getCoreContainer()).get(0).nodeName;
+ log.info("Node not provided, Identified {} for creating new replica", node);
+
if (!clusterState.liveNodesContain(node)) {
throw new SolrException(ErrorCode.BAD_REQUEST, "Node: " + node + " is not live");
}
Modified: lucene/dev/branches/branch_5x/solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java
URL: http://svn.apache.org/viewvc/lucene/dev/branches/branch_5x/solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java?rev=1690348&r1=1690347&r2=1690348&view=diff
==============================================================================
--- lucene/dev/branches/branch_5x/solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java (original)
+++ lucene/dev/branches/branch_5x/solr/solrj/src/java/org/apache/solr/common/params/CoreAdminParams.java Sat Jul 11 05:11:31 2015
@@ -116,6 +116,9 @@ public abstract class CoreAdminParams
public static final String TRANSIENT = "transient";
+ // Node to create a replica on for ADDREPLICA at least.
+ public static final String NODE = "node";
+
public enum CoreAdminAction {
STATUS,
LOAD,