dsmiley commented on code in PR #3305:
URL: https://github.com/apache/solr/pull/3305#discussion_r2027736206


##########
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:
   I could see a refactoring/reorganization cleanup happening before/after this 
issue/pr, but wouldn't want to combine such a change with this one.



##########
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;
+      }
+
+      @SuppressWarnings("unchecked")
+      Map<String, Object> props = (Map<String, Object>) Utils.fromJSON(data);
+      String nodeVersion = (String) props.get(LIVE_NODE_SOLR_VERSION);
+      if (nodeVersion != null) {
+        if (lowestVersion == null || compareVersions(nodeVersion, 
lowestVersion) < 0) {
+          lowestVersion = nodeVersion;
+        }
+      }
+    }
+
+    return lowestVersion;
+  }
+
+  /**
+   * Compares two version strings that are dot-separated numbers.
+   *
+   * <p>For example, "9.8.1" is considered higher than "9.5.0".
+   *
+   * @param v1 the first version string
+   * @param v2 the second version string
+   * @return a negative integer if v1 is lower than v2, zero if they are 
equal, or a positive
+   *     integer if v1 is higher than v2.
+   */
+  private int compareVersions(String v1, String v2) {

Review Comment:
   ZkStateReader has tons of code and this method does *not* belong here.  
Wouldn't SolrVersion already do this stuff?



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