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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
