Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions solr/core/src/java/org/apache/solr/cloud/ZkController.java
Original file line number Diff line number Diff line change
Expand Up @@ -2975,8 +2975,17 @@ public Collection<String> publishNodeAsDown(String nodeName) {
log.info("Publish node={} as DOWN", nodeName);

ClusterState clusterState = getClusterState();
Map<String, List<Replica>> replicasPerCollectionOnNode =
clusterState.getReplicaNamesPerCollectionOnNode(nodeName);
Map<String, List<Replica>> replicasPerCollectionOnNode = new HashMap<>();
clusterState
.collectionStream()
.forEach(
col -> {
List<Replica> replicas = col.getReplicasOnNode(nodeName);
if (!replicas.isEmpty()) {
replicasPerCollectionOnNode.put(col.getName(), replicas);
}
});

if (distributedClusterStateUpdater.isDistributedStateUpdate()) {
// Note that with the current implementation, when distributed cluster state updates are
// enabled, we mark the node down synchronously from this thread, whereas the Overseer cluster
Expand Down

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking at this, it's apparent the logic should be simplified to only get the list of replicas on this node for the collection the test cares about. No need for a Map; only a List. Could build in a single Stream.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Already resolved -- main picked this up via #4760 (unrelated SOLR-18382 cleanup), and I just merged main into this branch. Current code is exactly what you described: no Map, just clusterState.getCollection(collectionName).getReplicasOnNode(nodeName).

Original file line number Diff line number Diff line change
Expand Up @@ -388,11 +388,10 @@ public List<CoreDescriptor> getCoreDescriptors() {
zkController.getZkStateReader().forciblyRefreshAllClusterStateSlow();
ClusterState clusterState = zkController.getClusterState();

Map<String, List<Replica>> replicasOnNode =
clusterState.getReplicaNamesPerCollectionOnNode(nodeName);
assertNotNull("There should be replicas on the existing node", replicasOnNode);
List<Replica> replicas = replicasOnNode.get(collectionName);
assertNotNull("There should be replicas for the collection on the existing node", replicas);
List<Replica> replicas =
clusterState.getCollection(collectionName).getReplicasOnNode(nodeName);
assertFalse(
"There should be replicas for the collection on the existing node", replicas.isEmpty());
assertEquals(
"Wrong number of replicas for the collection on the existing node", 1, replicas.size());
for (Replica replica : replicas) {
Expand Down
18 changes: 0 additions & 18 deletions solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
Expand Down Expand Up @@ -160,22 +159,6 @@ public Set<String> getLiveNodes() {
return liveNodes;
}

@Deprecated(since = "10.0")
public Map<String, List<Replica>> getReplicaNamesPerCollectionOnNode(final String nodeName) {
Map<String, List<Replica>> replicaNamesPerCollectionOnNode = new HashMap<>();
collectionStates.values().stream()
.map(CollectionRef::get)
.filter(Objects::nonNull)
.forEach(
col -> {
List<Replica> replicas = col.getReplicasOnNode(nodeName);
if (!replicas.isEmpty()) {
replicaNamesPerCollectionOnNode.put(col.getName(), replicas);
}
});
return replicaNamesPerCollectionOnNode;
}

/** Check if node is alive. */
public boolean liveNodesContain(String name) {
return liveNodes.contains(name);
Expand All @@ -190,7 +173,6 @@ public String toString() {
return sb.toString();
}

@Deprecated
public static ClusterState createFromCollectionMap(
int version,
Map<String, Object> stateMap,
Expand Down