murblanc commented on a change in pull request #414:
URL: https://github.com/apache/solr/pull/414#discussion_r750523340
##########
File path:
solr/core/src/java/org/apache/solr/cloud/api/collections/ReplaceNodeCmd.java
##########
@@ -115,7 +119,7 @@ public void call(ClusterState state, ZkNodeProps message,
NamedList<Object> resu
.assignNrtReplicas(numNrtReplicas)
.assignTlogReplicas(numTlogReplicas)
.assignPullReplicas(numPullReplicas)
- .onNodes(new
ArrayList<>(ccc.getSolrCloudManager().getClusterStateProvider().getLiveNodes()))
+
.onNodes(ccc.getSolrCloudManager().getClusterStateProvider().getLiveNodes().stream().filter(node
-> !node.equals(source)).collect(Collectors.toList()))
Review comment:
Can you please remove line 185 and the empty finally below while you're
changing this file (unrelated to your changes and Github doesn't allow
commenting there)
##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
##########
@@ -356,6 +356,26 @@ public int weight() {
return createNodeList; // unmodified, but return for inline use
}
+ // throw an exception if all nodes in the supplied list are not live.
+ // Empty list will also fail.
+ // Returns the input
+ private static List<String> checkAnyLiveNodes(List<String> createNodeList,
ClusterState clusterState) {
+ Set<String> liveNodes = clusterState.getLiveNodes();
+ if (createNodeList == null) {
+ createNodeList = Collections.emptyList();
+ }
+ boolean anyLiveNodes = false;
+ for (String node : createNodeList) {
+ anyLiveNodes |= liveNodes.contains(node);
+ }
+ if (!anyLiveNodes) {
+ throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
+ "None of the node(s) specified " + createNodeList + " are not
currently active in "
Review comment:
Message should be reworded. "not" likely removed and `createNodeList`
output only once (second instance should be `liveNodes`).
Maybe "not currently active in" replaced by "active nodes"?
##########
File path: solr/core/src/test/org/apache/solr/cloud/ReplaceNodeTest.java
##########
@@ -158,12 +165,7 @@ public void test() throws Exception {
// make sure all newly created replicas on node are active
List<Replica> newReplicas = collection.getReplicas(node2bdecommissioned);
replicas.forEach(r -> {
Review comment:
Curly brackets are not needed:
`replicas.forEach(r -> newReplicas.removeIf(nr ->
nr.getName().equals(r.getName())));`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
