madrob commented on a change in pull request #644: URL: https://github.com/apache/solr/pull/644#discussion_r825174871
########## File path: solr/core/src/java/org/apache/solr/cloud/Overseer.java ########## @@ -444,9 +449,18 @@ private void checkIfIamStillLeader() { } } } + class CommandWithClusterState extends ArrayList<ZkWriteCommand> { Review comment: This feels like bad design. We probably want to have composition over inheritance here, it feels really strange to be extending ArrayList just to give it an extra data field for what we're returning. ########## File path: solr/core/src/java/org/apache/solr/cloud/Overseer.java ########## @@ -444,9 +449,18 @@ private void checkIfIamStillLeader() { } } } + class CommandWithClusterState extends ArrayList<ZkWriteCommand> { + final ClusterState clusterState; + + CommandWithClusterState(ZkWriteCommand cmd, ClusterState state) { + super(1); + super.add(cmd); + this.clusterState = state; + } + } private List<ZkWriteCommand> processMessage(ClusterState clusterState, Review comment: We might want to refactor this away from returning a list and instead return a ZkWriteCommand directly. Most of the operations are singleton commands so that is easy to handle. Then we could create a MultiCommand to handle the cases where that is not enough (looks like just down node, I think?). And the use case you're trying to address would be handled either by double-dispatch pattern or a similar cast to what we have in your PR now. ########## File path: solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java ########## @@ -160,6 +164,38 @@ public ZkWriteCommand modifyCollection(final ClusterState clusterState, ZkNodePr } } + /** + * Update the state/leader of replicas in state.json from PRS data + */ + public static DocCollection updateReplicasFromPrs(DocCollection coll, PerReplicaStates prs) { + //we are disabling PRS. Update the replica states + Map<String, Slice> modifiedSlices = new LinkedHashMap<>(); + coll.forEachReplica((s, replica) -> { + PerReplicaStates.State prsState = prs.states.get(replica.getName()); + if (prsState != null) { + if (prsState.state != replica.getState()) { Review comment: Yea, I don't think I have any better ways. -- 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