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

Reply via email to