Skip to content

SOLR-18381: remove ClusterState.getReplicaNamesPerCollectionOnNode #4768

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
epugh merged 9 commits into from
Sep 5, 2026
+15 −25
Merged

SOLR-18381: remove ClusterState.getReplicaNamesPerCollectionOnNode #4768

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
fac3dd4
SOLR-18381: remove ClusterState.getReplicaNamesPerCollectionOnNode, u…
serhiy-bzhezytskyy Aug 19, 2026
f2147ca
SOLR-18381: drop the changelog entry
serhiy-bzhezytskyy Aug 21, 2026
4fa2816
SOLR-18381: drop the stale 'deprecated but not yet removed' clause
serhiy-bzhezytskyy Aug 21, 2026
0916306
Merge remote-tracking branch 'origin/main' into SOLR-18381-un-depreca…
serhiy-bzhezytskyy Aug 24, 2026
c5aa3a1
Merge remote-tracking branch 'origin/main' into SOLR-18381-un-depreca…
serhiy-bzhezytskyy Aug 25, 2026
ab817a3
tidy
epugh Aug 25, 2026
b79e94f
Merge remote-tracking branch 'origin/main' into SOLR-18381-un-depreca…
serhiy-bzhezytskyy Aug 26, 2026
45531ad
Merge remote-tracking branch 'myfork/SOLR-18381-un-deprecate-createfr…
serhiy-bzhezytskyy Aug 26, 2026
a358173
Merge remote-tracking branch 'origin/main' into SOLR-18381-un-depreca…
serhiy-bzhezytskyy Aug 28, 2026
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
View file
Open in desktop
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
15 changes: 12 additions & 3 deletions solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java
View file
Open in desktop

@dsmiley dsmiley Aug 24, 2026

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.

@serhiy-bzhezytskyy serhiy-bzhezytskyy Aug 26, 2026

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 @@ -27,6 +27,7 @@
import java.nio.file.Path;
import java.time.Duration;
import java.time.Instant;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -389,9 +390,17 @@ 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);
Map<String, List<Replica>> replicasOnNode = new HashMap<>();
clusterState
.collectionStream()
.forEach(
col -> {
List<Replica> replicasOfCollection = col.getReplicasOnNode(nodeName);
if (!replicasOfCollection.isEmpty()) {
replicasOnNode.put(col.getName(), replicasOfCollection);
}
});
assertFalse("There should be replicas on the existing node", replicasOnNode.isEmpty());
List<Replica> replicas = replicasOnNode.get(collectionName);
assertNotNull("There should be replicas for the collection on the existing node", replicas);
assertEquals(
Expand Down
19 changes: 1 addition & 18 deletions solr/solrj/src/java/org/apache/solr/common/cloud/ClusterState.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,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 @@ -167,22 +166,6 @@ public Set<String> getLiveNodes() {
return liveNodes;
}

@Deprecated
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 Down Expand Up @@ -225,7 +208,7 @@ public static ClusterState createFromJson(
return createFromCollectionMap(version, stateMap, liveNodes, creationTime, prsSupplier);
}

@Deprecated
/** Still used by {@link #createFromJson}. */

@dsmiley dsmiley Aug 24, 2026

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.

createFromJson has been removed. We can remove this one. Ideally should have folded both together into the same change to tackle ClusterState API IMO.

@serhiy-bzhezytskyy serhiy-bzhezytskyy Aug 25, 2026

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.

Checked — it has its own independent callers (BackupManager, DistributedClusterStateUpdater, ZkStateReader in production, plus 4 test files), unrelated to createFromJson. Can't remove it; createFromJson was just a thin wrapper over it.

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