HoustonPutman commented on code in PR #3510:
URL: https://github.com/apache/solr/pull/3510#discussion_r2323411502
##########
solr/core/src/java/org/apache/solr/cloud/ZkController.java:
##########
@@ -602,6 +602,49 @@ private void checkNoOldClusterstate(final SolrZkClient
zkClient) throws Interrup
}
}
+ /**
+ * Checks version compatibility with other nodes in the cluster. Refuses to
start if there's a
+ * major.minor version difference between our Solr version and other nodes
in the cluster. Note:
+ * uses live nodes.
+ */
+ private void checkClusterVersionCompatibility() throws InterruptedException,
KeeperException {
+ Optional<SolrVersion> lowestVersion =
zkStateReader.fetchLowestSolrVersion();
+ if (lowestVersion.isPresent()) {
+ SolrVersion ourVersion = SolrVersion.LATEST;
+ SolrVersion clusterVersion = lowestVersion.get();
+
+ if (ourVersion.lessThan(clusterVersion)) {
+ log.warn(
+ "Our Solr version {} is older than cluster version {}",
ourVersion, clusterVersion);
+
+ // Check major version compatibility
+ if (ourVersion.getMajorVersion() < clusterVersion.getMajorVersion()) {
+ String message =
+ String.format(
+ Locale.ROOT,
+ "Refusing to start Solr to avoid lowering the lowest major
version of nodes in the cluster. "
Review Comment:
```suggestion
"Refusing to start Solr, since our version is lower than
the lowest version currently running in the cluster. "
```
##########
solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java:
##########
@@ -872,38 +873,40 @@ public void removeLiveNodesListener(LiveNodesListener
listener) {
}
/**
- * Returns the lowest Solr version among all live nodes in the cluster. It's
not greater than
- * {@link SolrVersion#LATEST_STRING}. Will not return null. If older Solr
nodes have joined that
- * don't declare their version, the result won't be accurate, but it's at
least an upper bound on
- * the possible version it might be.
+ * Returns the lowest Solr version among all live nodes in the cluster. If
older Solr nodes have
+ * joined that don't declare their version, the result won't be accurate,
but it's at least an
+ * upper bound on the possible version it might be.
*
- * @return the lowest Solr version of the cluster; not null
+ * @return an Optional containing the lowest Solr version of nodes in the
cluster, or empty if no
+ * live nodes exist or all nodes return 9.9.0 for unspecified versions
*/
- public SolrVersion fetchLowestSolrVersion() throws KeeperException,
InterruptedException {
+ public Optional<SolrVersion> fetchLowestSolrVersion()
+ throws KeeperException, InterruptedException {
List<String> liveNodeNames = zkClient.getChildren(LIVE_NODES_ZKNODE, null,
true);
- SolrVersion lowest = SolrVersion.LATEST; // current software
+ SolrVersion lowest = null;
// the last version to not specify its version in live nodes
final SolrVersion UNSPECIFIED_VERSION = SolrVersion.valueOf("9.9.0");
+
for (String nodeName : liveNodeNames) {
String path = LIVE_NODES_ZKNODE + "/" + nodeName;
byte[] data = zkClient.getData(path, null, null, true);
if (data == null || data.length == 0) {
- return UNSPECIFIED_VERSION;
+ return Optional.of(UNSPECIFIED_VERSION);
Review Comment:
Since this is an optional now, why not just return empty instead of 9.9.0?
(Same for the similar thing below)
##########
solr/core/src/java/org/apache/solr/cloud/ZkController.java:
##########
@@ -602,6 +602,49 @@ private void checkNoOldClusterstate(final SolrZkClient
zkClient) throws Interrup
}
}
+ /**
+ * Checks version compatibility with other nodes in the cluster. Refuses to
start if there's a
+ * major.minor version difference between our Solr version and other nodes
in the cluster. Note:
+ * uses live nodes.
+ */
+ private void checkClusterVersionCompatibility() throws InterruptedException,
KeeperException {
+ Optional<SolrVersion> lowestVersion =
zkStateReader.fetchLowestSolrVersion();
+ if (lowestVersion.isPresent()) {
+ SolrVersion ourVersion = SolrVersion.LATEST;
+ SolrVersion clusterVersion = lowestVersion.get();
+
+ if (ourVersion.lessThan(clusterVersion)) {
+ log.warn(
+ "Our Solr version {} is older than cluster version {}",
ourVersion, clusterVersion);
+
+ // Check major version compatibility
+ if (ourVersion.getMajorVersion() < clusterVersion.getMajorVersion()) {
+ String message =
+ String.format(
+ Locale.ROOT,
+ "Refusing to start Solr to avoid lowering the lowest major
version of nodes in the cluster. "
+ + "Our version: %s, lowest version in cluster: %s.",
+ ourVersion,
+ clusterVersion);
+ throw new SolrException(ErrorCode.INVALID_STATE, message);
+ }
+
+ // Check minor version compatibility within the same major version
+ if (ourVersion.getMajorVersion() == clusterVersion.getMajorVersion()
+ && ourVersion.getMinorVersion() <
clusterVersion.getMinorVersion()) {
+ String message =
+ String.format(
+ Locale.ROOT,
+ "Refusing to start Solr to avoid lowering the lowest minor
version of nodes in the cluster. "
Review Comment:
```suggestion
"Refusing to start Solr, since our version is lower than
the lowest version currently running in the cluster. "
```
--
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]
