Address comments

Signed-off-by: Lakshya Taragi <[email protected]>
ltaragi · Apr 19, 2024 · b4305f6 · b4305f6
1 parent 82c2179
commit b4305f6
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 30 deletions.
diff --git a/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java b/server/src/main/java/org/opensearch/cluster/coordination/JoinTaskExecutor.java
@@ -31,6 +31,7 @@
 
 package org.opensearch.cluster.coordination;
 
+import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.opensearch.Version;
 import org.opensearch.cluster.ClusterState;
@@ -79,7 +80,7 @@ public class JoinTaskExecutor implements ClusterStateTaskExecutor<JoinTaskExecut
 
     private final AllocationService allocationService;
 
-    private final Logger logger;
+    private static Logger logger = LogManager.getLogger(JoinTaskExecutor.class);
     private final RerouteService rerouteService;
 
     private final RemoteStoreNodeService remoteStoreNodeService;
@@ -143,7 +144,7 @@ public JoinTaskExecutor(
         RemoteStoreNodeService remoteStoreNodeService
     ) {
         this.allocationService = allocationService;
-        this.logger = logger;
+        JoinTaskExecutor.logger = logger;
         this.rerouteService = rerouteService;
         this.remoteStoreNodeService = remoteStoreNodeService;
     }
@@ -521,24 +522,23 @@ private static void ensureRemoteStoreNodesCompatibility(DiscoveryNode joiningNod
                     );
                 }
             }
-        } else if (remoteStoreCompatibilityMode == CompatibilityMode.MIXED) {
-            Version joiningNodeVersion = joiningNode.getVersion();
-            if (joiningNodeVersion.after(currentNodes.getMaxNodeVersion()) || joiningNodeVersion.before(currentNodes.getMinNodeVersion())) {
-                boolean isAfter = joiningNodeVersion.after(currentNodes.getMaxNodeVersion());
-                String reason = String.format(
-                    Locale.ROOT,
-                    "mixed mode: a %s version [%s] node [%s] is not allowed to join cluster with %s version [%s]",
-                    isAfter ? "higher" : "lower",
-                    joiningNode.getVersion(),
-                    joiningNode,
-                    isAfter ? "maximum" : "minimum",
-                    isAfter ? currentNodes.getMaxNodeVersion() : currentNodes.getMinNodeVersion()
-                );
-                throw new IllegalStateException(reason);
-            }
-            if (joiningNode.isRemoteStoreNode()) {
-                Optional<DiscoveryNode> remoteDN = existingNodes.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst();
-                remoteDN.ifPresent(discoveryNode -> ensureRemoteStoreNodesCompatibility(joiningNode, discoveryNode));
+        } else {
+            if (remoteStoreCompatibilityMode == CompatibilityMode.MIXED) {
+                if (joiningNode.getVersion().after(currentNodes.getMaxNodeVersion())) {
+                    String reason = String.format(
+                        Locale.ROOT,
+                        "remote migration : a node [%s] of higher version [%s] is not allowed to join a cluster with maximum version [%s]",
+                        joiningNode,
+                        joiningNode.getVersion(),
+                        currentNodes.getMaxNodeVersion()
+                    );
+                    logger.warn(reason);
+                    throw new IllegalStateException(reason);
+                }
+                if (joiningNode.isRemoteStoreNode()) {
+                    Optional<DiscoveryNode> remoteDN = existingNodes.stream().filter(DiscoveryNode::isRemoteStoreNode).findFirst();
+                    remoteDN.ifPresent(discoveryNode -> ensureRemoteStoreNodesCompatibility(joiningNode, discoveryNode));
+                }
             }
         }
     }

diff --git a/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java b/server/src/test/java/org/opensearch/cluster/coordination/JoinTaskExecutorTests.java
@@ -891,8 +891,7 @@ public void testNodeJoinInMixedMode() {
         FeatureFlags.initializeFeatureFlags(nodeSettings);
 
         List<Version> versions = allOpenSearchVersions();
-        assert versions.size() >= 3 : "test requires at least three open search versions";
-        Version lowerVersion = versions.get(versions.size() - 3);
+        assert versions.size() >= 2 : "test requires at least two open search versions";
         Version baseVersion = versions.get(versions.size() - 2);
         Version higherVersion = versions.get(versions.size() - 1);
 
@@ -911,28 +910,24 @@ public void testNodeJoinInMixedMode() {
 
         Metadata metadata = Metadata.builder().persistentSettings(mixedModeCompatibilitySettings).build();
 
-        boolean joiningNodeIsHigher = randomBoolean();
-
-        // joining node of a different version (higher or lower) than the current nodes
+        // joining node of a higher version than the current nodes
         DiscoveryNode joiningNode1 = new DiscoveryNode(
             randomAlphaOfLength(10),
             randomAlphaOfLength(10),
             buildNewFakeTransportAddress(),
             remoteStoreNodeAttributes(SEGMENT_REPO, TRANSLOG_REPO),
             Collections.singleton(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE),
-            joiningNodeIsHigher ? higherVersion : lowerVersion
+            higherVersion
         );
         final IllegalStateException exception = expectThrows(
             IllegalStateException.class,
             () -> JoinTaskExecutor.ensureNodesCompatibility(joiningNode1, currentNodes, metadata)
         );
         String reason = String.format(
             Locale.ROOT,
-            "mixed mode: a %s version [%s] node [%s] is not allowed to join cluster with %s version [%s]",
-            joiningNodeIsHigher ? "higher" : "lower",
-            joiningNode1.getVersion(),
+            "remote migration : a node [%s] of higher version [%s] is not allowed to join a cluster with maximum version [%s]",
             joiningNode1,
-            joiningNodeIsHigher ? "maximum" : "minimum",
+            joiningNode1.getVersion(),
             currentNodes.getMaxNodeVersion()
         );
         assertEquals(reason, exception.getMessage());