Address comments

Signed-off-by: Lakshya Taragi <lakshya.taragi@gmail.com>
ltaragi · Apr 24, 2024 · 9eddae3 · 9eddae3
1 parent 630796f
commit 9eddae3
Showing 1 changed file with 19 additions and 19 deletions.
diff --git a/.../opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java b/.../opensearch/cluster/routing/allocation/decider/RemoteStoreMigrationAllocationDecider.java
@@ -96,12 +96,12 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
         }
 
         IndexMetadata indexMetadata = allocation.metadata().getIndexSafe(shardRouting.index());
-        boolean remoteStoreBackedIndex = IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.get(indexMetadata.getSettings());
+        boolean remoteSettingsBackedIndex = IndexMetadata.INDEX_REMOTE_STORE_ENABLED_SETTING.get(indexMetadata.getSettings());
 
         if (migrationDirection.equals(Direction.NONE)) {
-            boolean isNoDecision = (remoteStoreBackedIndex && targetNode.isRemoteStoreNode() == false)
-                || (remoteStoreBackedIndex == false && targetNode.isRemoteStoreNode());
-            String reason = String.format(Locale.ROOT, " for %sremote store backed index", remoteStoreBackedIndex ? "" : "non ");
+            // remote backed indices on docrep nodes and non remote backed indices on remote nodes are not allowed
+            boolean isNoDecision = remoteSettingsBackedIndex ^ targetNode.isRemoteStoreNode();
+            String reason = String.format(Locale.ROOT, " for %sremote store backed index", remoteSettingsBackedIndex ? "" : "non ");
             return allocation.decision(
                 isNoDecision ? Decision.NO : Decision.YES,
                 NAME,
@@ -110,23 +110,23 @@ public Decision canAllocate(ShardRouting shardRouting, RoutingNode node, Routing
         } else if (migrationDirection.equals(Direction.DOCREP)) {
             // docrep migration direction is currently not supported
             return allocation.decision(Decision.YES, NAME, getDecisionDetails(true, shardRouting, targetNode, " for DOCREP direction"));
-        }
-
-        // check for remote store backed indices
-        if (remoteStoreBackedIndex && targetNode.isRemoteStoreNode() == false) {
-            // allocations and relocations must be to a remote node
-            String reason = String.format(
-                Locale.ROOT,
-                " because a remote store backed index's shard copy can only be %s to a remote node",
-                ((shardRouting.assignedToNode() == false) ? "allocated" : "relocated")
-            );
-            return allocation.decision(Decision.NO, NAME, getDecisionDetails(false, shardRouting, targetNode, reason));
-        }
+        } else {
+            // check for remote store backed indices
+            if (remoteSettingsBackedIndex && targetNode.isRemoteStoreNode() == false) {
+                // allocations and relocations must be to a remote node
+                String reason = String.format(
+                    Locale.ROOT,
+                    " because a remote store backed index's shard copy can only be %s to a remote node",
+                    ((shardRouting.assignedToNode() == false) ? "allocated" : "relocated")
+                );
+                return allocation.decision(Decision.NO, NAME, getDecisionDetails(false, shardRouting, targetNode, reason));
+            }
 
-        if (shardRouting.primary()) {
-            return primaryShardDecision(shardRouting, targetNode, allocation);
+            if (shardRouting.primary()) {
+                return primaryShardDecision(shardRouting, targetNode, allocation);
+            }
+            return replicaShardDecision(shardRouting, targetNode, allocation);
         }
-        return replicaShardDecision(shardRouting, targetNode, allocation);
     }
 
     // handle scenarios for allocation of a new shard's primary copy