justinrsweeney commented on code in PR #1258:
URL: https://github.com/apache/solr/pull/1258#discussion_r1060209357


##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java:
##########
@@ -679,7 +679,13 @@ private void constructState(Set<String> 
changedCollections) {
       result.putIfAbsent(entry.getKey(), entry.getValue());
     }
 
-    this.clusterState = new ClusterState(result, liveNodes);
+    this.clusterState = new ClusterState(result, zkLiveNodes.getLiveNodes());
+    // Listen for future live nodes changes to update ClusterState
+    zkLiveNodes.addLiveNodesListener(
+        (o, n) -> {
+          clusterState.setLiveNodes(n);
+          return false;
+        });

Review Comment:
   This is new, but none of this is really tested yet.
   
   The idea I'm thinking here is to create the ClusterState once and then have 
classes for each data type in ZK handling updates and updating necessary places 
via listeners like here: 
https://github.com/apache/solr/pull/1258/files/144d380843439672b2b7e76dae6fc6c94f839924#diff-4710a20b3d893bec0637610d3cce7dc383ca328c68a6fb0d4c7266e2041b9d0bR90,
 instead of having a bunch of methods in ZkStateReader that update ClusterState 
for all sort of data types.



-- 
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