dsmiley commented on code in PR #2432:
URL: https://github.com/apache/solr/pull/2432#discussion_r1586719024
##########
solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java:
##########
@@ -181,29 +183,46 @@ public String getShardId(String nodeName, String
coreName) {
}
public String getShardId(String collectionName, String nodeName, String
coreName) {
Review Comment:
Nobody calls this; lets remove it
##########
solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java:
##########
@@ -181,29 +183,46 @@ public String getShardId(String nodeName, String
coreName) {
}
public String getShardId(String collectionName, String nodeName, String
coreName) {
- Collection<CollectionRef> states = collectionStates.values();
+ if (coreName == null || nodeName == null) {
+ return null;
+ }
+ Collection<CollectionRef> states = Collections.emptyList();
if (collectionName != null) {
CollectionRef c = collectionStates.get(collectionName);
if (c != null) states = Collections.singletonList(c);
+ } else {
+ states = collectionStates.values();
}
for (CollectionRef ref : states) {
DocCollection coll = ref.get();
Review Comment:
(I know already existed) looping all collections scares me where I work.
Maybe that's a LazyDocCollection and we call get() (default false to get
disallow cached state, i.e. we need to fetch from ZK).
##########
solr/core/src/java/org/apache/solr/cloud/ZkController.java:
##########
@@ -2849,9 +2834,14 @@ public boolean
checkIfCoreNodeNameAlreadyExists(CoreDescriptor dcore) {
* Best effort to set DOWN state for all replicas on node.
*
* @param nodeName to operate on
+ * @return the names of the collections that have replicas on the given node
*/
- public void publishNodeAsDown(String nodeName) {
+ public Collection<String> publishNodeAsDown(String nodeName) {
log.info("Publish node={} as DOWN", nodeName);
+
+ ClusterState clusterState = cc.getZkController().getClusterState();
+ Map<String, List<Replica>> replicasPerCollectionOnNode =
+ clusterState.getReplicaNamesPerCollectionOnNode(nodeName);
Review Comment:
It appears this is only called on the current node. If true, couldn't we
instead list CoreDescriptors to find cores on the current node? This scales
much better than looping all collections that exist in the entire cluster.
--
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
