Apache
/
lucene
mirror of https://github.com/apache/lucene.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Revert "Refactor for code clarity, add some comments." 

This reverts commit ebd40918 for which I apparently ran the tests one less time than I needed to
This commit is contained in:
Gus Heck 2020-06-07 13:02:45 -04:00
parent 12280819a1
commit ae6fe8d826
1 changed files with 22 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
View File

@ -200,7 +200,7 @@ public class Assign {
}
return defaultValue;
}
private static int defaultCounterValue(DocCollection collection, boolean newCollection) {
if (newCollection) return 0;
int defaultValue = collection.getReplicas().size();
@ -332,7 +332,7 @@ public class Assign {
, shard, nrtReplicas, tlogReplicas, pullReplicas, createNodeSet);
DocCollection coll = clusterState.getCollection(collectionName);
int maxShardsPerNode = coll.getMaxShardsPerNode() == -1 ? Integer.MAX_VALUE : coll.getMaxShardsPerNode();
List<String> createNodeList;
List<String> createNodeList = null;
if (createNodeSet instanceof List) {
createNodeList = (List<String>) createNodeSet;
@ -341,13 +341,9 @@ public class Assign {
createNodeList = createNodeSet == null ? null : new ArrayList<>(new LinkedHashSet<>(StrUtils.splitSmart((String) createNodeSet, ",", true)));
}
// produces clear message when down nodes are the root cause, without this the user just
// gets a log message of detail about the nodes that are up, and a message that policies could not
// be satisfied which then requires study to diagnose the issue.
throwIfAnyNotLive(createNodeList,clusterState);
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.
HashMap<String, ReplicaCount> nodeNameVsShardCount = getNodeNameVsShardCount(collectionName, clusterState, null);
long availableSlots = 0;
for (Map.Entry<String, ReplicaCount> ent : nodeNameVsShardCount.entrySet()) {
//ADDREPLICA can put more than maxShardsPerNode on an instance, so this test is necessary.
@ -415,17 +411,24 @@ public class Assign {
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, ReplicaCount> nodeNameVsShardCount = new HashMap<>();
for (String s : throwIfAnyNotLive(createNodeList, clusterState)) {
for (String s : nodeList) {
nodeNameVsShardCount.put(s, new ReplicaCount(s));
}
// if we were given a list, just use that, don't worry about counts
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 " + createNodeList + " are not currently active in "
+ nodeNameVsShardCount.keySet() + ", no action taken.");
}
return nodeNameVsShardCount;
}
// if we get here we were not given a createNodeList, build a map with real counts.
DocCollection coll = clusterState.getCollection(collectionName);
int maxShardsPerNode = coll.getMaxShardsPerNode() == -1 ? Integer.MAX_VALUE : coll.getMaxShardsPerNode();
Map<String, DocCollection> collections = clusterState.getCollectionsMap();
@ -450,19 +453,6 @@ public class Assign {
return nodeNameVsShardCount;
}
private static List<String> throwIfAnyNotLive(List<String> createNodeList, ClusterState clusterState) {
if (createNodeList != null) {
if (!clusterState.getLiveNodes().containsAll(createNodeList)) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
"At least one of the node(s) specified " + createNodeList + " are not currently active in "
+ createNodeList + ", no action taken.");
}
}
// the logic that was extracted to this method used to create a defensive copy but no code
// was modifying the copy, if this method is made protected or public we want to go back to that
return createNodeList; // unmodified, but return for inline use
}
/**
* Thrown if there is an exception while assigning nodes for replicas
*/
@ -560,12 +550,10 @@ public class Assign {
@Override
public List<ReplicaPosition> assign(SolrCloudManager solrCloudManager, AssignRequest assignRequest) throws Assign.AssignmentException, IOException, InterruptedException {
ClusterState clusterState = solrCloudManager.getClusterStateProvider().getClusterState();
List<String> nodeList = assignRequest.nodes; // can this be empty list?
List<String> nodeList = assignRequest.nodes;
HashMap<String, Assign.ReplicaCount> nodeNameVsShardCount = Assign.getNodeNameVsShardCount(assignRequest.collectionName, clusterState, assignRequest.nodes);
if (nodeList == null || nodeList.isEmpty()) {
HashMap<String, Assign.ReplicaCount> nodeNameVsShardCount =
Assign.getNodeNameVsShardCount(assignRequest.collectionName, clusterState, nodeList);
// if nodelist was empty, this map will be empty too. (passing null above however gets a full map)
ArrayList<Assign.ReplicaCount> sortedNodeList = new ArrayList<>(nodeNameVsShardCount.values());
sortedNodeList.sort(Comparator.comparingInt(Assign.ReplicaCount::weight));
nodeList = sortedNodeList.stream().map(replicaCount -> replicaCount.nodeName).collect(Collectors.toList());
@ -573,28 +561,18 @@ public class Assign {
int i = 0;
List<ReplicaPosition> result = new ArrayList<>();
for (String aShard : assignRequest.shardNames) {
for (Map.Entry<Replica.Type, Integer> e : countsPerReplicaType(assignRequest).entrySet()) {
for (String aShard : assignRequest.shardNames)
for (Map.Entry<Replica.Type, Integer> e : ImmutableMap.of(Replica.Type.NRT, assignRequest.numNrtReplicas,
Replica.Type.TLOG, assignRequest.numTlogReplicas,
Replica.Type.PULL, assignRequest.numPullReplicas
).entrySet()) {
for (int j = 0; j < e.getValue(); j++) {
// TODO: not sure if we can receive an non-null, empty assingRequest.nodes,
// but if we do this appears to result in a div/0. Also not sure if empty should be
// same as null or an error (with better message) here.
result.add(new ReplicaPosition(aShard, j, e.getKey(), nodeList.get(i % nodeList.size())));
i++;
}
}
}
return result;
}
// keeps this big ugly construction block out of otherwise legible code
private ImmutableMap<Replica.Type, Integer> countsPerReplicaType(AssignRequest assignRequest) {
return ImmutableMap.of(
Replica.Type.NRT, assignRequest.numNrtReplicas,
Replica.Type.TLOG, assignRequest.numTlogReplicas,
Replica.Type.PULL, assignRequest.numPullReplicas
);
}
}
public static class RulesBasedAssignStrategy implements AssignStrategy {