You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by is...@apache.org on 2022/01/06 19:47:59 UTC
[solr] 27/34: addressed review comments
This is an automated email from the ASF dual-hosted git repository.
ishan pushed a commit to branch jira/SOLR15694
in repository https://gitbox.apache.org/repos/asf/solr.git
commit 0017f0b54e6012e69b8dd737930e3342722649ad
Author: Noble Paul <no...@gmail.com>
AuthorDate: Wed Dec 22 23:19:52 2021 +1100
addressed review comments
---
.../apache/solr/cloud/api/collections/Assign.java | 32 ++++++++--------------
.../src/java/org/apache/solr/cluster/Cluster.java | 1 +
.../placement/impl/PlacementRequestImpl.java | 2 +-
.../impl/SimpleClusterAbstractionsImpl.java | 15 +++++++++-
.../src/java/org/apache/solr/core/NodeRoles.java | 17 +++++-------
.../placement/ClusterAbstractionsForTest.java | 5 ++++
6 files changed, 39 insertions(+), 33 deletions(-)
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
index 7a4394f..88271a1 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
@@ -21,20 +21,7 @@ import static org.apache.solr.common.cloud.ZkStateReader.CORE_NAME_PROP;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Random;
-import java.util.Set;
+import java.util.*;
import java.util.stream.Collectors;
import org.apache.solr.client.solrj.cloud.DistribStateManager;
@@ -248,22 +235,25 @@ public class Assign {
Collections.shuffle(nodeList, random);
}
} else {
- nodeList = new ArrayList<>(liveNodes);
- filterNonDataNodes(zk, nodeList);
+ nodeList = new ArrayList<>(filterNonDataNodes(zk, liveNodes));
Collections.shuffle(nodeList, random);
}
return nodeList;
}
- public static void filterNonDataNodes(DistribStateManager zk, List<String> liveNodes) {
+ public static Collection<String> filterNonDataNodes(DistribStateManager zk, Collection<String> liveNodes) {
try {
List<String> noData = ClusterAPI.getNodesByRole(NodeRoles.Role.DATA, NodeRoles.Mode.OFF, zk);
- if (!noData.isEmpty()) {
- liveNodes.removeAll(noData);
- }
+ if (noData.isEmpty()) {
+ return liveNodes;
+ } else {
+ liveNodes = new HashSet<>(liveNodes);
+ liveNodes.removeAll(noData);
+ return liveNodes;
+ }
} catch (Exception e) {
- throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Problem fetching roles from Zookeeper", e);
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Error fetching roles from Zookeeper", e);
}
}
diff --git a/solr/core/src/java/org/apache/solr/cluster/Cluster.java b/solr/core/src/java/org/apache/solr/cluster/Cluster.java
index 3b7bdd4..4cd32a4 100644
--- a/solr/core/src/java/org/apache/solr/cluster/Cluster.java
+++ b/solr/core/src/java/org/apache/solr/cluster/Cluster.java
@@ -31,6 +31,7 @@ public interface Cluster {
*/
Set<Node> getLiveNodes();
+ Set<Node> getLiveNodes(boolean filterNonDataNodes) ;
/**
* Returns info about the given collection if one exists.
*
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementRequestImpl.java b/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementRequestImpl.java
index ff3f090..3814eb6 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementRequestImpl.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementRequestImpl.java
@@ -86,7 +86,7 @@ public class PlacementRequestImpl implements PlacementRequest {
throw new Assign.AssignmentException("Bad assign request: empty list of nodes for collection " + solrCollection.getName());
}
} else {
- nodes = cluster.getLiveNodes();
+ nodes = cluster.getLiveNodes(true);
if (nodes.isEmpty()) {
throw new Assign.AssignmentException("Impossible assign request: no live nodes for collection " + solrCollection.getName());
}
diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java b/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java
index 48f3f50..64c0b64 100644
--- a/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java
+++ b/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java
@@ -23,6 +23,7 @@ import java.util.stream.Collectors;
import com.google.common.collect.Maps;
import org.apache.solr.client.solrj.cloud.SolrCloudManager;
+import org.apache.solr.cloud.api.collections.Assign;
import org.apache.solr.cluster.*;
import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.cloud.DocCollection;
@@ -53,10 +54,16 @@ class SimpleClusterAbstractionsImpl {
static class ClusterImpl implements Cluster {
private final Set<Node> liveNodes;
+ private final Set<Node> liveNodesWithData;
private final ClusterState clusterState;
ClusterImpl(SolrCloudManager solrCloudManager) throws IOException {
- liveNodes = NodeImpl.getNodes(solrCloudManager.getClusterStateProvider().getLiveNodes());
+ Set<String> liveNodes = solrCloudManager.getClusterStateProvider().getLiveNodes();
+ Collection<String> liveNodesWithData = Assign.filterNonDataNodes( solrCloudManager.getDistribStateManager(), liveNodes);
+ this.liveNodes = NodeImpl.getNodes(liveNodes);
+ this.liveNodesWithData = liveNodesWithData.size() == liveNodes.size() ?
+ this.liveNodes :
+ NodeImpl.getNodes(liveNodesWithData);
clusterState = solrCloudManager.getClusterStateProvider().getClusterState();
}
@@ -65,6 +72,12 @@ class SimpleClusterAbstractionsImpl {
return liveNodes;
}
+ public Set<Node> getLiveNodes(boolean filterNonDataNodes) {
+ return filterNonDataNodes ?
+ liveNodesWithData :
+ liveNodes;
+ }
+
@Override
public SolrCollection getCollection(String collectionName) {
return SolrCollectionImpl.createCollectionFacade(clusterState, collectionName);
diff --git a/solr/core/src/java/org/apache/solr/core/NodeRoles.java b/solr/core/src/java/org/apache/solr/core/NodeRoles.java
index 714d8b1..317f771 100644
--- a/solr/core/src/java/org/apache/solr/core/NodeRoles.java
+++ b/solr/core/src/java/org/apache/solr/core/NodeRoles.java
@@ -18,7 +18,6 @@ package org.apache.solr.core;
import java.lang.invoke.MethodHandles;
import java.util.*;
-import org.apache.solr.common.MapWriter;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.StringUtils;
import org.apache.solr.common.cloud.ZkStateReader;
@@ -61,7 +60,7 @@ public class NodeRoles {
}
for(Role r: Role.values()) {
if (!roles.containsKey(r)) {
- roles.put(r, r.defaultIfAbsent());
+ roles.put(r, r.defaultMode());
}
}
nodeRoles = Collections.unmodifiableMap(roles);
@@ -100,7 +99,7 @@ public class NodeRoles {
return Set.of(Mode.ON, Mode.OFF);
}
@Override
- public Mode defaultIfAbsent() {
+ public Mode defaultMode() {
return Mode.OFF;
}
},
@@ -110,7 +109,7 @@ public class NodeRoles {
return Set.of(Mode.ALLOWED, Mode.PREFERRED, Mode.DISALLOWED);
}
@Override
- public Mode defaultIfAbsent() {
+ public Mode defaultMode() {
return Mode.DISALLOWED;
}
};
@@ -122,12 +121,10 @@ public class NodeRoles {
}
public static Role getRole(String value) {
- try {
- Role role = Role.valueOf(value.toUpperCase(Locale.ROOT));
- return role;
- } catch (IllegalArgumentException ex) {
- throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role: " + value);
+ for (Role role: Role.values()) {
+ if (value.equals(role.roleName)) return role;
}
+ throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role: " + value);
}
public abstract Set<Mode> supportedModes();
@@ -135,7 +132,7 @@ public class NodeRoles {
/**
* Default mode for a role in nodes where this role is not specified.
*/
- public abstract Mode defaultIfAbsent();
+ public abstract Mode defaultMode();
@Override
public String toString() {
diff --git a/solr/core/src/test/org/apache/solr/cluster/placement/ClusterAbstractionsForTest.java b/solr/core/src/test/org/apache/solr/cluster/placement/ClusterAbstractionsForTest.java
index 1260a10..bb85bd3 100644
--- a/solr/core/src/test/org/apache/solr/cluster/placement/ClusterAbstractionsForTest.java
+++ b/solr/core/src/test/org/apache/solr/cluster/placement/ClusterAbstractionsForTest.java
@@ -43,6 +43,11 @@ class ClusterAbstractionsForTest {
}
@Override
+ public Set<Node> getLiveNodes(boolean filterNonDataNodes) {
+ return liveNodes;
+ }
+
+ @Override
public SolrCollection getCollection(String collectionName) {
return collections.get(collectionName);
}