dsmiley commented on code in PR #3305:
URL: https://github.com/apache/solr/pull/3305#discussion_r2025963993
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java:
##########
@@ -865,6 +871,76 @@ public void removeLiveNodesListener(LiveNodesListener
listener) {
liveNodesListeners.remove(listener);
}
+ /**
+ * Returns the lowest Solr version among all live nodes in the cluster.
+ *
+ * @return the lowest Solr version as a String, or null if no version is
found
+ * @throws KeeperException if a ZooKeeper error occurs
+ * @throws InterruptedException if the thread is interrupted
+ */
+ public String getLowestSolrVersion() throws KeeperException,
InterruptedException {
Review Comment:
I personally hate it when a "get" method does network access; it's so subtle
-- the caller would have no clue except maybe the exceptions that can be thrown
as seen here. Using a different verb like "fetch" would be suitable. But
actually what I expected/hoped was that we'd cache the live node metadata in
ZkStateReader from live node watching. I could imagine this might be more
scope than you had in mind; maybe this here is fine for now.
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java:
##########
@@ -865,6 +871,76 @@ public void removeLiveNodesListener(LiveNodesListener
listener) {
liveNodesListeners.remove(listener);
}
+ /**
+ * Returns the lowest Solr version among all live nodes in the cluster.
+ *
+ * @return the lowest Solr version as a String, or null if no version is
found
+ * @throws KeeperException if a ZooKeeper error occurs
+ * @throws InterruptedException if the thread is interrupted
+ */
+ public String getLowestSolrVersion() throws KeeperException,
InterruptedException {
+ List<String> liveNodeNames = zkClient.getChildren(LIVE_NODES_ZKNODE, null,
true);
+ String lowestVersion = null;
+
+ for (String nodeName : liveNodeNames) {
+ String path = LIVE_NODES_ZKNODE + "/" + nodeName;
+ byte[] data = zkClient.getData(path, null, null, true);
+ if (data == null || data.length == 0) {
+ continue;
Review Comment:
if there's no data, this means it's a Solr 9.8 node (or earlier). This is
important!
##########
solr/core/src/java/org/apache/solr/cloud/ZkController.java:
##########
@@ -1183,6 +1191,18 @@ private void createEphemeralLiveNode() throws
KeeperException, InterruptedExcept
zkClient.multi(ops);
}
+ private byte[] buildLiveNodeData() {
Review Comment:
wouldn't this belong in ZkStateReader, which is primarily responsible for
reading/writing ZK?
--
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
