Introduce a new search node role to hold search only shards (opensearch-project#17620)

* Introduce a new search node role

Signed-off-by: Vinay Krishna Pudyodu <[email protected]>

* Add changelog

Signed-off-by: Vinay Krishna Pudyodu <[email protected]>

* fixed PR comments

Signed-off-by: Vinay Krishna Pudyodu <[email protected]>

* rename search to warm in FsProbe test

Signed-off-by: Vinay Krishna Pudyodu <[email protected]>

* fixed ClusterStatsIT tests

Signed-off-by: Vinay Krishna Pudyodu <[email protected]>

* added ClusterStatsIT test for search node role

Signed-off-by: Vinay Krishna Pudyodu <[email protected]>

---------

Signed-off-by: Vinay Krishna Pudyodu <[email protected]>

Author: vinaykpud

CHANGELOG.md

@@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 ### Added
 - Change priority for scheduling reroute during timeout([#16445](https://github.com/opensearch-project/OpenSearch/pull/16445))
 - Renaming the node role search to warm ([#17573](https://github.com/opensearch-project/OpenSearch/pull/17573))
+- Introduce a new search node role to hold search only shards ([#17620](https://github.com/opensearch-project/OpenSearch/pull/17620))
 - Search Replica Allocation and Recovery ([#17457](https://github.com/opensearch-project/OpenSearch/pull/17457))
 
 ### Dependencies

server/src/internalClusterTest/java/org/opensearch/action/admin/cluster/stats/ClusterStatsIT.java

@@ -90,7 +90,7 @@ private void waitForNodes(int numNodes) {
     public void testNodeCounts() {
         int total = 1;
         internalCluster().startNode();
-        Map<String, Integer> expectedCounts = getExpectedCounts(1, 1, 1, 1, 1, 0, 0);
+        Map<String, Integer> expectedCounts = getExpectedCounts(1, 1, 1, 1, 1, 0, 0, 0);
         int numNodes = randomIntBetween(1, 5);
 
         ClusterStatsResponse response = client().admin()
@@ -159,7 +159,7 @@ public void testNodeCountsWithDeprecatedMasterRole() throws ExecutionException,
         internalCluster().startNode(settings);
         waitForNodes(total);
 
-        Map<String, Integer> expectedCounts = getExpectedCounts(0, 1, 1, 0, 0, 0, 0);
+        Map<String, Integer> expectedCounts = getExpectedCounts(0, 1, 1, 0, 0, 0, 0, 0);
 
         Client client = client();
         ClusterStatsResponse response = client.admin()
@@ -484,7 +484,7 @@ public void testNodeRolesWithMasterLegacySettings() throws ExecutionException, I
         internalCluster().startNodes(legacyMasterSettings);
         waitForNodes(total);
 
-        Map<String, Integer> expectedCounts = getExpectedCounts(0, 1, 1, 0, 1, 0, 0);
+        Map<String, Integer> expectedCounts = getExpectedCounts(0, 1, 1, 0, 1, 0, 0, 0);
 
         Client client = client();
         ClusterStatsResponse clusterStatsResponse = client.admin()
@@ -518,7 +518,7 @@ public void testNodeRolesWithClusterManagerRole() throws ExecutionException, Int
         internalCluster().startNodes(clusterManagerNodeRoleSettings);
         waitForNodes(total);
 
-        Map<String, Integer> expectedCounts = getExpectedCounts(0, 1, 1, 0, 1, 0, 0);
+        Map<String, Integer> expectedCounts = getExpectedCounts(0, 1, 1, 0, 1, 0, 0, 0);
 
         Client client = client();
         ClusterStatsResponse clusterStatsResponse = client.admin()
@@ -546,7 +546,7 @@ public void testNodeRolesWithSeedDataNodeLegacySettings() throws ExecutionExcept
         internalCluster().startNodes(legacySeedDataNodeSettings);
         waitForNodes(total);
 
-        Map<String, Integer> expectedRoleCounts = getExpectedCounts(1, 1, 1, 0, 1, 0, 0);
+        Map<String, Integer> expectedRoleCounts = getExpectedCounts(1, 1, 1, 0, 1, 0, 0, 0);
 
         Client client = client();
         ClusterStatsResponse clusterStatsResponse = client.admin()
@@ -577,7 +577,7 @@ public void testNodeRolesWithDataNodeLegacySettings() throws ExecutionException,
         internalCluster().startNodes(legacyDataNodeSettings);
         waitForNodes(total);
 
-        Map<String, Integer> expectedRoleCounts = getExpectedCounts(1, 1, 1, 0, 1, 0, 0);
+        Map<String, Integer> expectedRoleCounts = getExpectedCounts(1, 1, 1, 0, 1, 0, 0, 0);
 
         Client client = client();
         ClusterStatsResponse clusterStatsResponse = client.admin()
@@ -594,6 +594,29 @@ public void testNodeRolesWithDataNodeLegacySettings() throws ExecutionException,
         assertEquals(expectedNodesRoles, Set.of(getNodeRoles(client, 0), getNodeRoles(client, 1)));
     }
 
+    public void testNodeRolesWithSearchNode() throws ExecutionException, InterruptedException {
+        int total = 2;
+        internalCluster().startClusterManagerOnlyNodes(1);
+        internalCluster().startSearchOnlyNode();
+        waitForNodes(total);
+
+        Map<String, Integer> expectedRoleCounts = getExpectedCounts(0, 1, 1, 0, 0, 0, 1, 0);
+
+        Client client = client();
+        ClusterStatsResponse clusterStatsResponse = client.admin()
+            .cluster()
+            .prepareClusterStats()
+            .useAggregatedNodeLevelResponses(randomBoolean())
+            .get();
+        assertCounts(clusterStatsResponse.getNodesStats().getCounts(), total, expectedRoleCounts);
+
+        Set<Set<String>> expectedNodesRoles = Set.of(
+            Set.of(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName()),
+            Set.of(DiscoveryNodeRole.SEARCH_ROLE.roleName())
+        );
+        assertEquals(expectedNodesRoles, Set.of(getNodeRoles(client, 0), getNodeRoles(client, 1)));
+    }
+
     public void testClusterStatsWithNodeMetricsFilter() {
         internalCluster().startNode();
         ensureGreen();
@@ -887,6 +910,7 @@ private Map<String, Integer> getExpectedCounts(
         int clusterManagerRoleCount,
         int ingestRoleCount,
         int remoteClusterClientRoleCount,
+        int warmRoleCount,
         int searchRoleCount,
         int coordinatingOnlyCount
     ) {
@@ -896,7 +920,8 @@ private Map<String, Integer> getExpectedCounts(
         expectedCounts.put(DiscoveryNodeRole.CLUSTER_MANAGER_ROLE.roleName(), clusterManagerRoleCount);
         expectedCounts.put(DiscoveryNodeRole.INGEST_ROLE.roleName(), ingestRoleCount);
         expectedCounts.put(DiscoveryNodeRole.REMOTE_CLUSTER_CLIENT_ROLE.roleName(), remoteClusterClientRoleCount);
-        expectedCounts.put(DiscoveryNodeRole.WARM_ROLE.roleName(), searchRoleCount);
+        expectedCounts.put(DiscoveryNodeRole.WARM_ROLE.roleName(), warmRoleCount);
+        expectedCounts.put(DiscoveryNodeRole.SEARCH_ROLE.roleName(), searchRoleCount);
         expectedCounts.put(ClusterStatsNodes.Counts.COORDINATING_ONLY, coordinatingOnlyCount);
         return expectedCounts;
     }

server/src/main/java/org/opensearch/cluster/node/DiscoveryNode.java

@@ -490,6 +490,15 @@ public boolean isWarmNode() {
         return roles.contains(DiscoveryNodeRole.WARM_ROLE);
     }
 
+    /**
+     * Returns whether the node is dedicated to host search replicas.
+     *
+     * @return true if the node contains a search role, false otherwise
+     */
+    public boolean isSearchNode() {
+        return roles.contains(DiscoveryNodeRole.SEARCH_ROLE);
+    }
+
     /**
      * Returns whether the node is a remote store node.
      *

server/src/main/java/org/opensearch/cluster/node/DiscoveryNodeRole.java

@@ -310,11 +310,39 @@ public Setting<Boolean> legacySetting() {
 
     };
 
+    /**
+     * Represents the role for a search node, which is dedicated to host search replicas.
+     */
+    public static final DiscoveryNodeRole SEARCH_ROLE = new DiscoveryNodeRole("search", "s", true) {
+
+        @Override
+        public Setting<Boolean> legacySetting() {
+            // search role is added in 2.4 so doesn't need to configure legacy setting
+            return null;
+        }
+
+        @Override
+        public void validateRole(List<DiscoveryNodeRole> roles) {
+            for (DiscoveryNodeRole role : roles) {
+                if (role.equals(DiscoveryNodeRole.SEARCH_ROLE) == false) {
+                    throw new IllegalArgumentException(
+                        String.format(
+                            Locale.ROOT,
+                            "%s role cannot be combined with any other role on a node.",
+                            DiscoveryNodeRole.SEARCH_ROLE.roleName()
+                        )
+                    );
+                }
+            }
+        }
+
+    };
+
     /**
      * The built-in node roles.
      */
     public static SortedSet<DiscoveryNodeRole> BUILT_IN_ROLES = Collections.unmodifiableSortedSet(
-        new TreeSet<>(Arrays.asList(DATA_ROLE, INGEST_ROLE, CLUSTER_MANAGER_ROLE, REMOTE_CLUSTER_CLIENT_ROLE, WARM_ROLE))
+        new TreeSet<>(Arrays.asList(DATA_ROLE, INGEST_ROLE, CLUSTER_MANAGER_ROLE, REMOTE_CLUSTER_CLIENT_ROLE, WARM_ROLE, SEARCH_ROLE))
     );
 
     /**

server/src/test/java/org/opensearch/cluster/node/DiscoveryNodeTests.java

@@ -242,6 +242,14 @@ public void testDiscoveryNodeIsWarmUnset() {
         runTestDiscoveryNodeIsWarm(nonWarmNode(), false);
     }
 
+    public void testDiscoveryNodeIsSearchSet() {
+        runTestDiscoveryNodeIsSearch(NodeRoles.searchOnlyNode(), true);
+    }
+
+    public void testDiscoveryNodeIsSearchUnset() {
+        runTestDiscoveryNodeIsSearch(NodeRoles.nonSearchNode(), false);
+    }
+
     // Added in 2.0 temporarily, validate the MASTER_ROLE is in the list of known roles.
     // MASTER_ROLE was removed from BUILT_IN_ROLES and is imported by setDeprecatedMasterRole(),
     // as a workaround for making the new CLUSTER_MANAGER_ROLE has got the same abbreviation 'm'.
@@ -271,6 +279,16 @@ private void runTestDiscoveryNodeIsWarm(final Settings settings, final boolean e
         }
     }
 
+    private void runTestDiscoveryNodeIsSearch(final Settings settings, final boolean expected) {
+        final DiscoveryNode node = DiscoveryNode.createLocal(settings, new TransportAddress(TransportAddress.META_ADDRESS, 9200), "node");
+        assertThat(node.isSearchNode(), equalTo(expected));
+        if (expected) {
+            assertThat(node.getRoles(), hasItem(DiscoveryNodeRole.SEARCH_ROLE));
+        } else {
+            assertThat(node.getRoles(), not(hasItem(DiscoveryNodeRole.SEARCH_ROLE)));
+        }
+    }
+
     public void testGetRoleFromRoleNameIsCaseInsensitive() {
         String dataRoleName = "DATA";
         DiscoveryNodeRole dataNodeRole = DiscoveryNode.getRoleFromRoleName(dataRoleName);

server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java

@@ -126,7 +126,7 @@ public void testFsInfo() throws IOException {
     }
 
     public void testFsCacheInfo() throws IOException {
-        Settings settings = Settings.builder().put("node.roles", "search").build();
+        Settings settings = Settings.builder().put("node.roles", "warm").build();
         try (NodeEnvironment env = newNodeEnvironment(settings)) {
             ByteSizeValue gbByteSizeValue = new ByteSizeValue(1, ByteSizeUnit.GB);
             env.fileCacheNodePath().fileCacheReservedSize = gbByteSizeValue;
@@ -164,7 +164,7 @@ public void testFsCacheInfo() throws IOException {
     }
 
     public void testFsInfoWhenFileCacheOccupied() throws IOException {
-        Settings settings = Settings.builder().putList("node.roles", "search", "data").build();
+        Settings settings = Settings.builder().putList("node.roles", "warm", "data").build();
         try (NodeEnvironment env = newNodeEnvironment(settings)) {
             // Use the total space as reserved space to simulate the situation where the cache space is occupied
             final long totalSpace = adjustForHugeFilesystems(env.fileCacheNodePath().fileStore.getTotalSpace());

server/src/test/java/org/opensearch/node/NodeRoleSettingsTests.java

@@ -44,6 +44,15 @@ public void testClusterManagerAndDataNodeRoles() {
         );
     }
 
+    /**
+     * Validate search role cannot coexist with any other role on a node.
+     */
+    public void testSearchRoleCannotCoExistWithAnyOtherRole() {
+        Settings roleSettings = Settings.builder().put(NodeRoleSettings.NODE_ROLES_SETTING.getKey(), "search, test_role").build();
+        Exception exception = expectThrows(IllegalArgumentException.class, () -> NodeRoleSettings.NODE_ROLES_SETTING.get(roleSettings));
+        assertThat(exception.getMessage(), containsString("search role cannot be combined with any other role on a node."));
+    }
+
     /**
      * Validate setting master role will result a deprecation message.
      * Remove the test after removing MASTER_ROLE.

test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java

@@ -166,6 +166,7 @@
 import static org.opensearch.test.NodeRoles.onlyRole;
 import static org.opensearch.test.NodeRoles.onlyRoles;
 import static org.opensearch.test.NodeRoles.removeRoles;
+import static org.opensearch.test.NodeRoles.searchOnlyNode;
 import static org.opensearch.test.OpenSearchTestCase.assertBusy;
 import static org.opensearch.test.OpenSearchTestCase.randomBoolean;
 import static org.opensearch.test.OpenSearchTestCase.randomFrom;
@@ -2318,6 +2319,22 @@ public List<String> startDataAndWarmNodes(int numNodes, Settings settings) {
         return startNodes(numNodes, Settings.builder().put(onlyRoles(settings, warmAndDataRoles)).build());
     }
 
+    public List<String> startSearchOnlyNodes(int numNodes) {
+        return startSearchOnlyNodes(numNodes, Settings.EMPTY);
+    }
+
+    public List<String> startSearchOnlyNodes(int numNodes, Settings settings) {
+        return startNodes(numNodes, Settings.builder().put(searchOnlyNode(settings)).build());
+    }
+
+    public String startSearchOnlyNode() {
+        return startSearchOnlyNode(Settings.EMPTY);
+    }
+
+    public String startSearchOnlyNode(Settings settings) {
+        return startNode(Settings.builder().put(settings).put(searchOnlyNode(settings)).build());
+    }
+
     public List<String> startDataOnlyNodes(int numNodes) {
         return startDataOnlyNodes(numNodes, Settings.EMPTY);
     }

test/framework/src/main/java/org/opensearch/test/NodeRoles.java

@@ -224,4 +224,19 @@ public static Settings nonWarmNode(final Settings settings) {
         return removeRoles(settings, Collections.singleton(DiscoveryNodeRole.WARM_ROLE));
     }
 
+    public static Settings searchOnlyNode() {
+        return searchOnlyNode(Settings.EMPTY);
+    }
+
+    public static Settings searchOnlyNode(final Settings settings) {
+        return onlyRole(settings, DiscoveryNodeRole.SEARCH_ROLE);
+    }
+
+    public static Settings nonSearchNode() {
+        return nonSearchNode(Settings.EMPTY);
+    }
+
+    public static Settings nonSearchNode(final Settings settings) {
+        return removeRoles(settings, Collections.singleton(DiscoveryNodeRole.SEARCH_ROLE));
+    }
 }