SOLR-18381: remove ClusterState.getReplicaNamesPerCollectionOnNode - #4768
Conversation
…n-deprecate createFromCollectionMap getReplicaNamesPerCollectionOnNode had 0 remaining callers after migrating ZkController/ZkControllerTest to collectionStream() + getReplicasOnNode. createFromCollectionMap is un-deprecated instead of removed, contrary to the ticket's title: it's still called from createFromJson (deprecated, tracked separately in the still-open SOLR-18149) plus two production call sites. AI-assisted (Claude Sonnet 5)
Same shape as apache#4763/apache#4761 -- a narrow, single-purpose ClusterState helper, no observable behavior change.
|
I wonder if we wait till the fix for SOLR-18149 lands before merging this? |
|
also, for purely internal type changes with no change log, you can just add the label |
createFromJson's own deprecation is being resolved separately (SOLR-18149) -- this comment shouldn't assume its status either way.
|
I don't have permissions to add labels as an outside contributor -- could you add |
argh, that's annonying. also, feel free to just slap in a comment to that effect and I can do the bookeeping. |
|
Opened #4777 -- un-deprecates |
…te-createfromcollectionmap
| } | ||
|
|
||
| @Deprecated | ||
| /** Still used by {@link #createFromJson}. */ |
There was a problem hiding this comment.
createFromJson has been removed. We can remove this one. Ideally should have folded both together into the same change to tackle ClusterState API IMO.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
…te-createfromcollectionmap Resolves a conflict from SOLR-18149's createFromJson removal (main) colliding with this branch's un-deprecation of createFromCollectionMap: drops the now-dead createFromJson method/comment, keeps createFromCollectionMap un-deprecated. Also addresses dsmiley's review: simplifies ZkControllerTest's replicasOnNode lookup from a Map built over every collection to a direct getCollection() call for the one collection the test actually checks.
|
Both addressed. Rebased on main ( |
dsmiley
changed the title
…te-createfromcollectionmap
…omcollectionmap' into SOLR-18381-un-deprecate-createfromcollectionmap
3 participants
https://issues.apache.org/jira/browse/SOLR-18381
Removes
ClusterState.getReplicaNamesPerCollectionOnNode(0 callers left after migratingZkController/ZkControllerTesttocollectionStream()+getReplicasOnNode), and un-deprecatescreateFromCollectionMapinstead of removing it as the title suggests -- it's still called fromcreateFromJson(deprecated, tracked separately in the still-open SOLR-18149) plus two production call sites.14 tests, 0 failures.
SOLR-18370 (local, not yet a PR) also touches ZkController.java, different region -- re-check once it opens a PR.
AI-assisted (Claude Sonnet 5)