You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2021/11/09 13:11:07 UTC

[GitHub] [solr] noblepaul opened a new pull request #403: SOLR-15694 Concept of node roles and non-data nodes

noblepaul opened a new pull request #403:
URL: https://github.com/apache/solr/pull/403


   WIP


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] murblanc commented on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-971454944


   > In this PR, only a single role can be assigned to a node, and I'm fixing that. Changes to come soon, hence this is not yet ready for reviewing. Sorry for the confusion. Also, merging this PR can't happen now, until the SIP-15 has been voted for.
   
   While you're at it Ishan, please fix the formatting of the many `if(...` that lack a space after the if and change them into:
   `if (...`.
   Thanks.


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r777332670



##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,198 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.
+
+== Definitions
+
+=== Node role
+
+A role is a designation of a node that indicates that the node may perform a certain functionality that is governed by the role.
+
+=== Mode
+Every role has a list of modes under which a node can be. It can be simple (e.g. `["on", "off"]`) or more granular (e.g. `["allowed", "preferred", "disallowed"]`).
+
+== Roles
+
+In order to specify role(s) for a node, one needs to start a Solr node with the following parameter.
+
+.Startup Parameter
+[cols="1,2,1,1"] 
+|===
+|Parameter |Value |Required? | Default
+
+|solr.node.roles
+|Comma separated list of roles (in the format: `<role>:<mode>`) for this node.
+Examples: `-Dsolr.node.roles=data:on,overseer:allowed` or `-Dsolr.node.roles=overseer:preferred`
+|No
+|`data:on,overseer:allowed`
+|===
+
+[TIP]
+====
+If a node has been started with no `solr.node.roles` parameter, it will be assumed to have the data role turned on and overseer role allowed on it. If you've never used roles before, you likely won't need to change anything in your startup parameters to accomodate the functionality associated with these roles.
+====
+
+.Supported roles
+[cols="1,1"] 
+|===
+|Role |Modes
+
+|`data`
+|on, off
+
+|`overseer`
+|allowed, preferred, disallowed
+|===
+
+=== `data` role
+A node with this role (in mode "on") can host shards and replicas for collections.
+
+=== `overseer` role
+A node with this role can perform duties of an overseer node (unless mode is `disallowed`). When one or more nodes have the overseer role in `preferred` mode, the overseer leader will be elected from one of these nodes. In case no node is designated as a preferred overseer or no such node is live, the overseer leader will be elected from one of the nodes that have the overseer role in `allowed` mode. If all nodes that are designated with overseer role (allowed or preferred) are down, the cluster will be left without an overseer.
+
+== Example usage
+
+Sometimes, when the nodes in a cluster are under heavy querying or indexing load, the overseer leader node might be unable to perform collection management duties efficiently. It might be reasonable to have dedicated nodes to act as the overseer. Such an effect can be achieved as follows:
+
+* Most nodes (data nodes) in the cluster start with `-Dsolr.node.roles=data:on,overseer:allowed` (or with no parameter, since the default value for `solr.node.roles` is the same).

Review comment:
       Here's the link to the quoted mail: https://lists.apache.org/thread/xn5z79hyb2sggwsqs41165krcj96hvdl




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] noblepaul commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r773652976



##########
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##########
@@ -944,6 +946,14 @@ public void load() {
       });
 
       clusterSingletons.setReady();
+      if (NodeRoles.PREFERRED.equals(nodeRoles.getRoleMode(NodeRoles.Role.OVERSEER))) {
+        try {
+          log.info("This node is started as a preferred overseer");

Review comment:
       @janhoy 
   
   I agree with @murblanc . It's hard to have a generic interface to perform the logic of a role. The place in which this code is executed is very important. it may also alter the behavior of existing classes. However, it may be possible to have a generic interface for roles for certain roles that will come up in the future. The current design doesn't stop us from doing so




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r779829825



##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +64,194 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    Map<String, Object> result = new LinkedHashMap<>();
+
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result = null;
+    try {
+      List<String> children = zk.listData(path);
+      if (children != null && !children.isEmpty()) {
+        result = new HashMap<>();
+      } else {
+        return depth >= 1 ? Collections.emptyList() : null;
+      }
+      for (String child : children) {
+        Object c = readRecursive(path + "/" + child, zk, depth - 1);
+        result.put(child, c);
+      }
+    } catch (Exception e) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+    }
+    if (depth == 1) {
+      return result.keySet();
+    } else {
+      return result;
+    }
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}",
+          permission = COLL_READ_PERM)
+  public void nodesWithRole(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String role = req.getPathTemplateValues().get("role");
+    rsp.add("node-roles", Map.of(role,
+            readRecursive(ZkStateReader.NODE_ROLES + "/"+ role,
+                    collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 2)));
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/node/{node}",
+          permission = COLL_READ_PERM)
+  @SuppressWarnings("unchecked")
+  public void rolesForNode(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String node = req.getPathTemplateValues().get("node");
+    Map<String, String> ret = new HashMap<String, String>();
+    Map<String, Map<String, Set<String>>> roles = (Map<String, Map<String, Set<String>>>) readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3);
+    for (String role: roles.keySet()) {
+      for (String mode : roles.get(role).keySet()) {
+        if (roles.get(role).get(mode) instanceof List && ((List) roles.get(role).get(mode)).size() == 0) {
+          continue;
+        }
+        Set<String> nodes = roles.get(role).get(mode);
+        if (nodes.contains(node)) ret.put(role, mode);
+      }
+    }
+    for (String role: ret.keySet()) {
+      rsp.add(role, ret.get(role));
+    }
+  }
+
   @EndPoint(method = GET,
-      path = "/cluster/aliases",
-      permission = COLL_READ_PERM)
+          path = "/cluster/node-roles/supported",
+          permission = COLL_READ_PERM)
+  public void supportedRoles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+     Map<String, Object> roleModesSupportedMap = new HashMap<>();
+    for (NodeRoles.Role role: NodeRoles.Role.values()) {
+      roleModesSupportedMap.put(role.toString(), Map.of("modes", role.supportedModes(), "defaultIfAbsent", role.defaultIfAbsent()));

Review comment:
       Removed this "defaultIfAbsent" from this API response. Renamed it to "modeWhenRoleIsAbsent" internally. For the user, there is no role-specific "default" to consider, everything is to be explicitly specified.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r777332293



##########
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##########
@@ -944,6 +946,14 @@ public void load() {
       });
 
       clusterSingletons.setReady();
+      if (NodeRoles.PREFERRED.equals(nodeRoles.getRoleMode(NodeRoles.Role.OVERSEER))) {
+        try {
+          log.info("This node is started as a preferred overseer");

Review comment:
       If we are at a point where we want pluggable roles, we can easily refactor the code to achieve that. I think it is still early enough to know all usecases that roles can be used for in order to arrive at the right abstractions. All of this is internal code as of now, so no backcompat will break when we introduce a refactoring.
   




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] madrob commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r778333682



##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,196 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.
+
+== Definitions
+
+=== Node role
+
+A role is a designation of a node that indicates that the node may perform a certain functionality that is governed by the role.

Review comment:
       ```suggestion
   A role is a designation for a node that indicates that the node may perform a certain function.
   ```

##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,196 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.

Review comment:
       Also, can simplify this language.
   
   ```suggestion
   A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying, collection management tasks, etc. To set up a cluster where these functions are isolated to certain dedicated nodes, we can use the concept of node roles.
   ```

##########
File path: solr/core/src/java/org/apache/solr/cluster/Cluster.java
##########
@@ -31,6 +31,7 @@
    */
   Set<Node> getLiveNodes();
 
+  Set<Node> getLiveNodes(boolean filterNonDataNodes) ;

Review comment:
       This feels like an overspecialization. Would it be better to have as an entirely separate method? `getLiveDataNodes()`?

##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
##########
@@ -103,6 +103,16 @@
   public static final String STATE_TIMESTAMP_PROP = "stateTimestamp";
   public static final String COLLECTIONS_ZKNODE = "/collections";
   public static final String LIVE_NODES_ZKNODE = "/live_nodes";
+
+  /**
+   * The following, node_roles and roles.json are for assigning roles to
+   * nodes. The node_roles is the preferred way (using -Dsolr.node.roles param),
+   * and roles.json is used by legacy ADDROLE API command.
+   * TODO: Deprecate and remove support for roles.json in an upcoming release.

Review comment:
       This TODO shouldn't be part of the javadoc, it should be a separate line comment on `ROLES`

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +65,191 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
   @EndPoint(method = GET,
-      path = "/cluster/aliases",
-      permission = COLL_READ_PERM)
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result;
+    try {
+      List<String> children = zk.listData(path);
+      if (children != null && !children.isEmpty()) {
+        result = new HashMap<>();
+      } else {
+        return depth >= 1 ? Collections.emptySet(): null;

Review comment:
       Isn't this always true? We already checked for `depth == 0`.

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +65,191 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
   @EndPoint(method = GET,
-      path = "/cluster/aliases",
-      permission = COLL_READ_PERM)
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {

Review comment:
       This is pretty opaque how sometimes we are returning a Set and sometimes a Map. I'd prefer a more consistent data structure rather than attempting to optimize for a few bytes of memory.

##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,196 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.
+
+== Definitions
+
+=== Node role
+
+A role is a designation of a node that indicates that the node may perform a certain functionality that is governed by the role.
+
+=== Mode
+Every role has a list of modes under which a node can be. It can be simple (e.g. `["on", "off"]`) or more granular (e.g. `["allowed", "preferred", "disallowed"]`).
+
+== Roles
+
+In order to specify role(s) for a node, one needs to start a Solr node with the following parameter.
+
+.Startup Parameter
+[cols="1,2,1,1"] 
+|===
+|Parameter |Value |Required? | Default
+
+|solr.node.roles
+|Comma separated list of roles (in the format: `<role>:<mode>`) for this node.
+Examples: `-Dsolr.node.roles=data:on,overseer:allowed` or `-Dsolr.node.roles=overseer:preferred`
+|No
+|`data:on,overseer:allowed`

Review comment:
       I don't understand what is wrong with this line?

##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
##########
@@ -21,20 +21,7 @@
 
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Random;
-import java.util.Set;
+import java.util.*;

Review comment:
       nit: we don't do wildcard imports.

##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
##########
@@ -244,13 +235,28 @@ private static boolean existCoreName(String coreName, Slice slice) {
         Collections.shuffle(nodeList, random);
       }
     } else {
-      nodeList = new ArrayList<>(liveNodes);
+      nodeList = new ArrayList<>(filterNonDataNodes(zk, liveNodes));
       Collections.shuffle(nodeList, random);
     }
 
     return nodeList;
   }
 
+  public static Collection<String> filterNonDataNodes(DistribStateManager zk, Collection<String> liveNodes) {

Review comment:
       Should this be a generic filter method that takes a Role parameter?

##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##########
@@ -913,7 +924,9 @@ private void init() {
         ElectionContext context = new OverseerElectionContext(zkClient,
             overseer, getNodeName());
         overseerElector.setup(context);
-        overseerElector.joinElection(context, false);
+        if (cc.nodeRoles.isOverseerAllowedOrPreferred()) {

Review comment:
       Can we expand this conditional to avoid getting the context above?

##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerRoleCmd.java
##########
@@ -64,6 +64,10 @@ public void call(ClusterState state, ZkNodeProps message, NamedList<Object> resu
     SolrZkClient zkClient = zkStateReader.getZkClient();
     Map<String, List<String>> roles = null;
     String node = message.getStr("node");
+    if ("false".equals(message.getStr("persist"))) {// no need to persist to roles.json

Review comment:
       Is this a new feature? I don't see doc changes on it.

##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##########
@@ -2211,6 +2225,15 @@ public void checkOverseerDesignate() {
     }
   }
 
+  public void setPreferredOverseer() throws KeeperException, InterruptedException {
+    ZkNodeProps props = new ZkNodeProps(Overseer.QUEUE_OPERATION, ADDROLE.toString().toLowerCase(Locale.ROOT),
+        "node", getNodeName(),
+        "role", "overseer",
+        "persist", "false");
+    log.info("Going to add role {} ", props);

Review comment:
       Should we warn that this is deprecated and point people to the new way to set roles?

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles {
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  /**
+   * Roles to be assumed on nodes that don't have roles specified for them at startup
+   */
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, Mode> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, Mode> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      Mode m = Mode.valueOf(roleMode.get(1).toUpperCase(Locale.ROOT));
+      if (r.supportedModes().contains(m)) {
+        roles.put(r, m);
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+                "Unknown role mode '" + roleMode.get(1) + "' for role '" + r + "'");
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.modeWhenRoleIsAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, Mode> getRoles() {
+    return nodeRoles;
+  }
+
+  public Mode getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  public boolean isOverseerAllowedOrPreferred() {
+    Mode roleMode = nodeRoles.get(Role.OVERSEER);
+    return Mode.ALLOWED.equals(roleMode) || Mode.PREFERRED.equals(roleMode);
+  }
+
+  public enum Mode {
+    ON, OFF, ALLOWED, PREFERRED, DISALLOWED;

Review comment:
       I thought each role was going to define its own modes? This feels overly restrictive. Packages that add roles ought to be able to define their own modes, maybe they have tiered priorities or something like that.

##########
File path: solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.cloud;
+
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+
+import java.util.Map;
+
+import org.apache.solr.client.solrj.embedded.JettySolrRunner;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.V2Request;
+import org.apache.solr.client.solrj.response.V2Response;
+import org.apache.solr.core.NodeRoles;
+import org.junit.After;
+import org.junit.Before;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRolesTest extends SolrCloudTestCase {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @Before
+  public void setupCluster() throws Exception {
+    configureCluster(1)
+            .addConfig("conf", configset("cloud-minimal"))
+            .configure();
+  }
+
+  @After
+  public void tearDownCluster() throws Exception {
+    shutdownCluster();
+  }
+
+  @SuppressWarnings("rawtypes")
+  public void testRoleIntegration() throws Exception {
+    JettySolrRunner j0 = cluster.getJettySolrRunner(0);
+    testSupportedRolesAPI();
+
+    // Start a dedicated overseer node
+    JettySolrRunner j1 = startNodeWithRoles("overseer:preferred,data:off");
+    validateNodeRoles(j1.getNodeName(), "node-roles/overseer/preferred", j1.getNodeName(), "node-roles/data/off");
+
+    V2Response rsp;
+    OverseerRolesTest.waitForNewOverseer(20, j1.getNodeName(), true);
+
+    // Start another node that is allowed or preferred overseer but has data
+    String overseerModeOnDataNode = random().nextBoolean() ? "preferred" : "allowed";
+    JettySolrRunner j2 = startNodeWithRoles("overseer:" + overseerModeOnDataNode + ",data:on");
+    validateNodeRoles(j2.getNodeName(), "node-roles/overseer/" + overseerModeOnDataNode, j2.getNodeName(), "node-roles/data/on");
+
+    // validate the preferred overseers
+    validateNodeRoles(j2.getNodeName(), "node-roles/overseer/" + overseerModeOnDataNode, j1.getNodeName(), "node-roles/overseer/preferred");
+
+    String COLLECTION_NAME = "TEST_ROLES";
+    CollectionAdminRequest
+            .createCollection(COLLECTION_NAME, "conf", 3, 1)
+            .process(cluster.getSolrClient());
+    cluster.waitForActiveCollection(COLLECTION_NAME, 3, 3);
+
+    // Assert that no replica was placed on the dedicated overseer node
+    String dedicatedOverseer = j1.getNodeName();
+    cluster.getSolrClient().getClusterStateProvider().getCollection(COLLECTION_NAME)
+            .forEachReplica((s, replica) -> assertNotEquals(replica.node, dedicatedOverseer));
+
+    // Shutdown the dedicated overseer, make sure that node disappears from the roles output
+    j1.stop();
+
+    // Wait and make sure that another node picks up overseer responsibilities
+    OverseerRolesTest.waitForNewOverseer(20, it -> !dedicatedOverseer.equals(it), false);
+
+    // Make sure the stopped node no longer has the role assigned
+    rsp = new V2Request.Builder("/cluster/node-roles/role/overseer/" + overseerModeOnDataNode).GET().build().process(cluster.getSolrClient());
+    assertFalse(((Collection) rsp._get("node-roles/overseer/" + overseerModeOnDataNode, null)).contains(j1.getNodeName()));
+  }
+
+  @SuppressWarnings("rawtypes")
+  private void validateNodeRoles(String... nodenamePaths) throws org.apache.solr.client.solrj.SolrServerException, java.io.IOException {
+    V2Response rsp = new V2Request.Builder("/cluster/node-roles").GET().build().process(cluster.getSolrClient());
+    for (int i = 0; i < nodenamePaths.length; i += 2) {
+      String nodename = nodenamePaths[i];
+      String path = nodenamePaths[i + 1];
+      assertTrue("Didn't find " + nodename + " at " + path + ". Full response: " + rsp.jsonStr(),
+              ((Collection) rsp._get(path, Collections.emptyList())).contains(nodename));
+    }
+  }
+
+  @SuppressWarnings("unchecked")
+  private void testSupportedRolesAPI() throws Exception {
+    V2Response rsp = new V2Request.Builder("/cluster/node-roles/supported").GET().build().process(cluster.getSolrClient());
+    Map<String, Object> l = (Map<String, Object>) rsp._get("supported-roles", Collections.emptyMap());
+    assertTrue(l.containsKey("data"));
+    assertTrue(l.containsKey("overseer"));
+  }
+
+  private JettySolrRunner startNodeWithRoles(String roles) throws Exception {

Review comment:
       I'm worried that we won't be able to run concurrent tests when we are reading system properties this way. Hadn't considered this when we were talking about sys prop vs other approaches. I think we might need another way to be able to set roles.

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +65,191 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
   @EndPoint(method = GET,
-      path = "/cluster/aliases",
-      permission = COLL_READ_PERM)
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));

Review comment:
       Can we retain a reference to the state manager? Or is it likely to change over time. These repeated long call chains are hard to read.

##########
File path: solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.cloud;
+
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+
+import java.util.Map;
+
+import org.apache.solr.client.solrj.embedded.JettySolrRunner;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.V2Request;
+import org.apache.solr.client.solrj.response.V2Response;
+import org.apache.solr.core.NodeRoles;
+import org.junit.After;
+import org.junit.Before;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRolesTest extends SolrCloudTestCase {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @Before
+  public void setupCluster() throws Exception {
+    configureCluster(1)
+            .addConfig("conf", configset("cloud-minimal"))
+            .configure();
+  }
+
+  @After
+  public void tearDownCluster() throws Exception {
+    shutdownCluster();
+  }
+
+  @SuppressWarnings("rawtypes")
+  public void testRoleIntegration() throws Exception {
+    JettySolrRunner j0 = cluster.getJettySolrRunner(0);
+    testSupportedRolesAPI();
+
+    // Start a dedicated overseer node
+    JettySolrRunner j1 = startNodeWithRoles("overseer:preferred,data:off");
+    validateNodeRoles(j1.getNodeName(), "node-roles/overseer/preferred", j1.getNodeName(), "node-roles/data/off");
+
+    V2Response rsp;
+    OverseerRolesTest.waitForNewOverseer(20, j1.getNodeName(), true);
+
+    // Start another node that is allowed or preferred overseer but has data
+    String overseerModeOnDataNode = random().nextBoolean() ? "preferred" : "allowed";
+    JettySolrRunner j2 = startNodeWithRoles("overseer:" + overseerModeOnDataNode + ",data:on");
+    validateNodeRoles(j2.getNodeName(), "node-roles/overseer/" + overseerModeOnDataNode, j2.getNodeName(), "node-roles/data/on");
+
+    // validate the preferred overseers
+    validateNodeRoles(j2.getNodeName(), "node-roles/overseer/" + overseerModeOnDataNode, j1.getNodeName(), "node-roles/overseer/preferred");
+
+    String COLLECTION_NAME = "TEST_ROLES";
+    CollectionAdminRequest
+            .createCollection(COLLECTION_NAME, "conf", 3, 1)
+            .process(cluster.getSolrClient());
+    cluster.waitForActiveCollection(COLLECTION_NAME, 3, 3);
+
+    // Assert that no replica was placed on the dedicated overseer node
+    String dedicatedOverseer = j1.getNodeName();
+    cluster.getSolrClient().getClusterStateProvider().getCollection(COLLECTION_NAME)
+            .forEachReplica((s, replica) -> assertNotEquals(replica.node, dedicatedOverseer));
+
+    // Shutdown the dedicated overseer, make sure that node disappears from the roles output
+    j1.stop();
+
+    // Wait and make sure that another node picks up overseer responsibilities
+    OverseerRolesTest.waitForNewOverseer(20, it -> !dedicatedOverseer.equals(it), false);
+
+    // Make sure the stopped node no longer has the role assigned
+    rsp = new V2Request.Builder("/cluster/node-roles/role/overseer/" + overseerModeOnDataNode).GET().build().process(cluster.getSolrClient());
+    assertFalse(((Collection) rsp._get("node-roles/overseer/" + overseerModeOnDataNode, null)).contains(j1.getNodeName()));
+  }
+
+  @SuppressWarnings("rawtypes")

Review comment:
       please use generic types.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] murblanc commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
murblanc commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r772758746



##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {
+    String roleMode = nodeRoles.get(Role.OVERSEER);
+    return ALLOWED.equals(roleMode) || PREFERRED.equals(roleMode);
+  }
+
+  public enum Role {
+    DATA("data"),
+    OVERSEER("overseer") {

Review comment:
       I actually think I get your point a bit better @janhoy. Do you mean a package introducing a role used by that package (and obviously not by the rest of Solr), and that role being managed like any other role?
   
   Wouldn't a package be able to achieve similar behavior (with more flexibility and not necessarily increased code complexity) by using a specific system property for its configuration? (or is that not an option? I'm not familiar with packages)




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] murblanc commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
murblanc commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r772766560



##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {
+    String roleMode = nodeRoles.get(Role.OVERSEER);
+    return ALLOWED.equals(roleMode) || PREFERRED.equals(roleMode);
+  }
+
+  public enum Role {
+    DATA("data"),
+    OVERSEER("overseer") {

Review comment:
       If we consider a package can change the roles of a node when it starts, then indeed the CDCR role could reset everything else, but configuring a cluster would soon become a nightmare.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r773547692



##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {
+    String roleMode = nodeRoles.get(Role.OVERSEER);
+    return ALLOWED.equals(roleMode) || PREFERRED.equals(roleMode);
+  }
+
+  public enum Role {
+    DATA("data"),
+    OVERSEER("overseer") {
+      @Override
+      public Set<String> supportedModes() {
+        return OVERSEER_MODES;
+      }
+      @Override
+      public String defaultIfAbsent() {
+        return DISALLOWED;
+      }
+    };
+
+    public final String roleName;
+
+    Role(String name) {
+      this.roleName = name;
+    }
+
+    public static Role getRole(String value) {
+      for (Role role: Role.values()) {

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {
+    String roleMode = nodeRoles.get(Role.OVERSEER);
+    return ALLOWED.equals(roleMode) || PREFERRED.equals(roleMode);
+  }
+
+  public enum Role {
+    DATA("data"),
+    OVERSEER("overseer") {
+      @Override
+      public Set<String> supportedModes() {
+        return OVERSEER_MODES;

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {

Review comment:
       There seems no need. Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +64,194 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    Map<String, Object> result = new LinkedHashMap<>();
+
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result = null;
+    try {
+      List<String> children = zk.listData(path);
+      if (children != null && !children.isEmpty()) {
+        result = new HashMap<>();
+      } else {
+        return depth >= 1 ? Collections.emptyList() : null;
+      }
+      for (String child : children) {
+        Object c = readRecursive(path + "/" + child, zk, depth - 1);
+        result.put(child, c);
+      }
+    } catch (Exception e) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+    }
+    if (depth == 1) {
+      return result.keySet();
+    } else {
+      return result;
+    }
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}",
+          permission = COLL_READ_PERM)
+  public void nodesWithRole(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String role = req.getPathTemplateValues().get("role");
+    rsp.add("node-roles", Map.of(role,
+            readRecursive(ZkStateReader.NODE_ROLES + "/"+ role,
+                    collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 2)));
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/node/{node}",
+          permission = COLL_READ_PERM)
+  @SuppressWarnings("unchecked")
+  public void rolesForNode(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String node = req.getPathTemplateValues().get("node");
+    Map<String, String> ret = new HashMap<String, String>();
+    Map<String, Map<String, Set<String>>> roles = (Map<String, Map<String, Set<String>>>) readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3);
+    for (String role: roles.keySet()) {
+      for (String mode : roles.get(role).keySet()) {
+        if (roles.get(role).get(mode) instanceof List && ((List) roles.get(role).get(mode)).size() == 0) {
+          continue;
+        }
+        Set<String> nodes = roles.get(role).get(mode);
+        if (nodes.contains(node)) ret.put(role, mode);
+      }
+    }
+    for (String role: ret.keySet()) {
+      rsp.add(role, ret.get(role));
+    }
+  }
+
   @EndPoint(method = GET,
-      path = "/cluster/aliases",
-      permission = COLL_READ_PERM)
+          path = "/cluster/node-roles/supported",
+          permission = COLL_READ_PERM)
+  public void supportedRoles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+     Map<String, Object> roleModesSupportedMap = new HashMap<>();
+    for (NodeRoles.Role role: NodeRoles.Role.values()) {
+      roleModesSupportedMap.put(role.toString(), Map.of("modes", role.supportedModes(), "defaultIfAbsent", role.defaultIfAbsent()));
+    }
+    rsp.add("supported-roles", roleModesSupportedMap);
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}/{role-val}",

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +64,194 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    Map<String, Object> result = new LinkedHashMap<>();
+
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result = null;

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +64,194 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    Map<String, Object> result = new LinkedHashMap<>();

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +64,194 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    Map<String, Object> result = new LinkedHashMap<>();
+
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result = null;
+    try {
+      List<String> children = zk.listData(path);
+      if (children != null && !children.isEmpty()) {
+        result = new HashMap<>();
+      } else {
+        return depth >= 1 ? Collections.emptyList() : null;
+      }
+      for (String child : children) {
+        Object c = readRecursive(path + "/" + child, zk, depth - 1);
+        result.put(child, c);
+      }
+    } catch (Exception e) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+    }
+    if (depth == 1) {
+      return result.keySet();
+    } else {
+      return result;
+    }
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}",
+          permission = COLL_READ_PERM)
+  public void nodesWithRole(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String role = req.getPathTemplateValues().get("role");
+    rsp.add("node-roles", Map.of(role,
+            readRecursive(ZkStateReader.NODE_ROLES + "/"+ role,
+                    collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 2)));
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/node/{node}",
+          permission = COLL_READ_PERM)
+  @SuppressWarnings("unchecked")
+  public void rolesForNode(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String node = req.getPathTemplateValues().get("node");
+    Map<String, String> ret = new HashMap<String, String>();
+    Map<String, Map<String, Set<String>>> roles = (Map<String, Map<String, Set<String>>>) readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3);
+    for (String role: roles.keySet()) {
+      for (String mode : roles.get(role).keySet()) {
+        if (roles.get(role).get(mode) instanceof List && ((List) roles.get(role).get(mode)).size() == 0) {

Review comment:
       The recursive method was giving out an emptyList instead of emptySet. Fixed it there. Thanks.

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +64,194 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    Map<String, Object> result = new LinkedHashMap<>();
+
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result = null;
+    try {
+      List<String> children = zk.listData(path);
+      if (children != null && !children.isEmpty()) {
+        result = new HashMap<>();
+      } else {
+        return depth >= 1 ? Collections.emptyList() : null;
+      }
+      for (String child : children) {
+        Object c = readRecursive(path + "/" + child, zk, depth - 1);
+        result.put(child, c);
+      }
+    } catch (Exception e) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+    }
+    if (depth == 1) {
+      return result.keySet();
+    } else {
+      return result;
+    }
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}",
+          permission = COLL_READ_PERM)
+  public void nodesWithRole(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String role = req.getPathTemplateValues().get("role");
+    rsp.add("node-roles", Map.of(role,
+            readRecursive(ZkStateReader.NODE_ROLES + "/"+ role,
+                    collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 2)));
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/node/{node}",
+          permission = COLL_READ_PERM)
+  @SuppressWarnings("unchecked")
+  public void rolesForNode(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String node = req.getPathTemplateValues().get("node");
+    Map<String, String> ret = new HashMap<String, String>();
+    Map<String, Map<String, Set<String>>> roles = (Map<String, Map<String, Set<String>>>) readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3);
+    for (String role: roles.keySet()) {
+      for (String mode : roles.get(role).keySet()) {
+        if (roles.get(role).get(mode) instanceof List && ((List) roles.get(role).get(mode)).size() == 0) {
+          continue;
+        }
+        Set<String> nodes = roles.get(role).get(mode);
+        if (nodes.contains(node)) ret.put(role, mode);
+      }
+    }
+    for (String role: ret.keySet()) {
+      rsp.add(role, ret.get(role));
+    }
+  }
+
   @EndPoint(method = GET,
-      path = "/cluster/aliases",
-      permission = COLL_READ_PERM)
+          path = "/cluster/node-roles/supported",
+          permission = COLL_READ_PERM)
+  public void supportedRoles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+     Map<String, Object> roleModesSupportedMap = new HashMap<>();
+    for (NodeRoles.Role role: NodeRoles.Role.values()) {
+      roleModesSupportedMap.put(role.toString(), Map.of("modes", role.supportedModes(), "defaultIfAbsent", role.defaultIfAbsent()));
+    }
+    rsp.add("supported-roles", roleModesSupportedMap);
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}/{role-val}",
+          permission = COLL_READ_PERM)
+  public void nodesWithRoleVal(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String role = req.getPathTemplateValues().get("role");
+    String roleVal = req.getPathTemplateValues().get("role-val");
+    List<String> nodes =  collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager()
+            .getDistribStateManager().listData(ZkStateReader.NODE_ROLES + "/"+ role+"/"+roleVal);

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/cloud/OverseerNodePrioritizer.java
##########
@@ -59,11 +63,18 @@ public OverseerNodePrioritizer(ZkStateReader zkStateReader, Overseer overseer, S
 
   public synchronized void prioritizeOverseerNodes(String overseerId) throws Exception {
     SolrZkClient zk = zkStateReader.getZkClient();
-    if(!zk.exists(ZkStateReader.ROLES,true))return;
-    Map<?,?> m = (Map<?,?>) Utils.fromJSON(zk.getData(ZkStateReader.ROLES, null, new Stat(), true));
+    List<String> overseerDesignates = new ArrayList<>();
+    if (zk.exists(ZkStateReader.ROLES,true)) {
+      Map<?,?> m = (Map<?,?>) Utils.fromJSON(zk.getData(ZkStateReader.ROLES, null, new Stat(), true));
+      @SuppressWarnings("unchecked")
+      List<String> l = (List<String>) m.get("overseer");
+      if (l != null) {
+        overseerDesignates.addAll(l);
+      }
+    }
 
-    List<?> overseerDesignates = (List<?>) m.get("overseer");
-    if(overseerDesignates==null || overseerDesignates.isEmpty()) return;
+    overseerDesignates.addAll(ClusterAPI.getNodesByRole(NodeRoles.Role.OVERSEER, NodeRoles.PREFERRED, new ZkDistribStateManager(zkStateReader.getZkClient())));

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##########
@@ -913,7 +924,9 @@ private void init() {
         ElectionContext context = new OverseerElectionContext(zkClient,
             overseer, getNodeName());
         overseerElector.setup(context);
-        overseerElector.joinElection(context, false);
+        if(cc.nodeRoles.isOverseerAllowed()) {

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##########
@@ -357,7 +358,9 @@ public void command() throws SessionExpiredException {
 
                 overseerElector.setup(context);
 
-                overseerElector.joinElection(context, true);
+                if(cc.nodeRoles.isOverseerAllowed()) {

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##########
@@ -854,6 +857,14 @@ public static void createClusterZkNodes(SolrZkClient zkClient)
       throws KeeperException, InterruptedException, IOException {
     ZkCmdExecutor cmdExecutor = new ZkCmdExecutor(zkClient.getZkClientTimeout());
     cmdExecutor.ensureExists(ZkStateReader.LIVE_NODES_ZKNODE, zkClient);
+    cmdExecutor.ensureExists(ZkStateReader.NODE_ROLES, zkClient);
+    for (NodeRoles.Role role : NodeRoles.Role.values()) {
+      cmdExecutor.ensureExists(ZkStateReader.NODE_ROLES + "/" + role.roleName, zkClient);

Review comment:
       Fixed all occurrences here to use a method in NodeRoles class that took Role and Mode. However, left out the ClusterAPI references to use that method since I didn't want to constrain the API calls to be able to resolve strictly typed Node, Role values for backcompat reasons. Maybe, we can add another version of those methods that only took strings, not Role/Mode values?

##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##########
@@ -1086,6 +1099,11 @@ private void createEphemeralLiveNode() throws KeeperException,
     log.info("Register node as live in ZooKeeper:{}", nodePath);
     List<Op> ops = new ArrayList<>(2);
     ops.add(Op.create(nodePath, null, zkClient.getZkACLProvider().getACLsToAdd(nodePath), CreateMode.EPHEMERAL));
+
+    // Create the roles node as well
+   cc.nodeRoles.getRoles().forEach((role, val) -> ops.add(Op.create(ZkStateReader.NODE_ROLES + "/" + role.roleName+ "/"+val +"/"+ nodeName,

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##########
@@ -944,6 +946,14 @@ public void load() {
       });
 
       clusterSingletons.setReady();
+      if (NodeRoles.PREFERRED.equals(nodeRoles.getRoleMode(NodeRoles.Role.OVERSEER))) {
+        try {
+          log.info("This node is started as a preferred overseer");
+          zkSys.getZkController().setPreferredOverseer();
+        } catch (KeeperException | InterruptedException e) {
+          throw new SolrException(ErrorCode.SERVER_ERROR, e);
+        }
+      }
       if (!distributedCollectionCommandRunner.isPresent()) {

Review comment:
       Couldn't understand the comment. @noblepaul please take a look.

##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
##########
@@ -103,6 +103,7 @@
   public static final String STATE_TIMESTAMP_PROP = "stateTimestamp";
   public static final String COLLECTIONS_ZKNODE = "/collections";
   public static final String LIVE_NODES_ZKNODE = "/live_nodes";
+  public static final String NODE_ROLES = "/node_roles";

Review comment:
       Fixed, thanks.

##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,198 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.
+
+== Definitions
+
+=== Node role
+
+A role is a designation of a node that indicates that the node may perform a certain functionality that is governed by the role.
+
+=== Mode
+Every role has a list of modes under which a node can be. It can be simple (e.g. `["on", "off"]`) or more granular (e.g. `["allowed", "preferred", "disallowed"]`).
+
+== Roles
+
+In order to specify role(s) for a node, one needs to start a Solr node with the following parameter.
+
+.Startup Parameter
+[cols="1,2,1,1"] 
+|===
+|Parameter |Value |Required? | Default
+
+|solr.node.roles
+|Comma separated list of roles (in the format: `<role>:<mode>`) for this node.
+Examples: `-Dsolr.node.roles=data:on,overseer:allowed` or `-Dsolr.node.roles=overseer:preferred`
+|No
+|`data:on,overseer:allowed`
+|===
+
+[TIP]
+====
+If a node has been started with no `solr.node.roles` parameter, it will be assumed to have the data role turned on and overseer role allowed on it. If you've never used roles before, you likely won't need to change anything in your startup parameters to accomodate the functionality associated with these roles.

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.cloud;
+
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+
+import java.util.List;
+import java.util.Map;
+
+import org.apache.solr.client.solrj.embedded.JettySolrRunner;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.V2Request;
+import org.apache.solr.client.solrj.response.V2Response;
+import org.apache.solr.core.NodeRoles;
+import org.junit.After;
+import org.junit.Before;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRolesTest extends SolrCloudTestCase {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @Before
+  public void setupCluster() throws Exception {
+    configureCluster(1)
+            .addConfig("conf", configset("cloud-minimal"))
+            .configure();
+  }
+
+  @After
+  public void tearDownCluster() throws Exception {
+    shutdownCluster();
+  }
+
+  @SuppressWarnings("unchecked")
+  public void testRoleIntegration() throws Exception {
+    JettySolrRunner j0 = cluster.getJettySolrRunner(0);
+    JettySolrRunner j1 = null, j2 = null;

Review comment:
       Fixed, thanks.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-999221193


   Addressed some comments from Ilan. TODO: Address some others from Ilan and Jan.


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-971449610


   In this PR, only a single role can be assigned to a node, and I'm fixing that. Changes to come soon, hence this is not yet ready for reviewing. Sorry for the confusion.


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-971621437


   > Making sure we agree on the fact that nodes without defined roles are assumed to have all roles, not only data role.
   
   As discussed on the SIP thread (mainly during a back and forth between Ishan & Gus), we don't want to have "all roles enabled by default", since that means any roles added in future will also get enabled by default and that might not be desireable. Rather, we only want the "data" role turned on by default due to backcompat reasons (i.e. those who are not using roles feature are already using their nodes for data).


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] HoustonPutman commented on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-974476570


   > Rather, we only want the "data" role turned on by default due to backcompat reasons
   
   It seemed pretty clear to me in the dev list thread that this was not a consensus. There are many roles that we want enabled by default, like the "query/compute/whatever its new name is" role. That would also be a backcompat concern.


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] noblepaul commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r773839765



##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {
+    String roleMode = nodeRoles.get(Role.OVERSEER);
+    return ALLOWED.equals(roleMode) || PREFERRED.equals(roleMode);
+  }
+
+  public enum Role {
+    DATA("data"),
+    OVERSEER("overseer") {
+      @Override
+      public Set<String> supportedModes() {
+        return OVERSEER_MODES;
+      }
+      @Override
+      public String defaultIfAbsent() {
+        return DISALLOWED;
+      }
+    };
+
+    public final String roleName;
+
+    Role(String name) {
+      this.roleName = name;
+    }
+
+    public static Role getRole(String value) {
+      for (Role role: Role.values()) {

Review comment:
       toUpperCase() creates an extra string always and it is unnecesary




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r779825974



##########
File path: solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.cloud;
+
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+
+import java.util.Map;
+
+import org.apache.solr.client.solrj.embedded.JettySolrRunner;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.V2Request;
+import org.apache.solr.client.solrj.response.V2Response;
+import org.apache.solr.core.NodeRoles;
+import org.junit.After;
+import org.junit.Before;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRolesTest extends SolrCloudTestCase {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @Before
+  public void setupCluster() throws Exception {
+    configureCluster(1)
+            .addConfig("conf", configset("cloud-minimal"))
+            .configure();
+  }
+
+  @After
+  public void tearDownCluster() throws Exception {
+    shutdownCluster();
+  }
+
+  @SuppressWarnings("rawtypes")
+  public void testRoleIntegration() throws Exception {
+    JettySolrRunner j0 = cluster.getJettySolrRunner(0);
+    testSupportedRolesAPI();
+
+    // Start a dedicated overseer node
+    JettySolrRunner j1 = startNodeWithRoles("overseer:preferred,data:off");
+    validateNodeRoles(j1.getNodeName(), "node-roles/overseer/preferred", j1.getNodeName(), "node-roles/data/off");
+
+    V2Response rsp;
+    OverseerRolesTest.waitForNewOverseer(20, j1.getNodeName(), true);
+
+    // Start another node that is allowed or preferred overseer but has data
+    String overseerModeOnDataNode = random().nextBoolean() ? "preferred" : "allowed";
+    JettySolrRunner j2 = startNodeWithRoles("overseer:" + overseerModeOnDataNode + ",data:on");
+    validateNodeRoles(j2.getNodeName(), "node-roles/overseer/" + overseerModeOnDataNode, j2.getNodeName(), "node-roles/data/on");
+
+    // validate the preferred overseers
+    validateNodeRoles(j2.getNodeName(), "node-roles/overseer/" + overseerModeOnDataNode, j1.getNodeName(), "node-roles/overseer/preferred");
+
+    String COLLECTION_NAME = "TEST_ROLES";
+    CollectionAdminRequest
+            .createCollection(COLLECTION_NAME, "conf", 3, 1)
+            .process(cluster.getSolrClient());
+    cluster.waitForActiveCollection(COLLECTION_NAME, 3, 3);
+
+    // Assert that no replica was placed on the dedicated overseer node
+    String dedicatedOverseer = j1.getNodeName();
+    cluster.getSolrClient().getClusterStateProvider().getCollection(COLLECTION_NAME)
+            .forEachReplica((s, replica) -> assertNotEquals(replica.node, dedicatedOverseer));
+
+    // Shutdown the dedicated overseer, make sure that node disappears from the roles output
+    j1.stop();
+
+    // Wait and make sure that another node picks up overseer responsibilities
+    OverseerRolesTest.waitForNewOverseer(20, it -> !dedicatedOverseer.equals(it), false);
+
+    // Make sure the stopped node no longer has the role assigned
+    rsp = new V2Request.Builder("/cluster/node-roles/role/overseer/" + overseerModeOnDataNode).GET().build().process(cluster.getSolrClient());
+    assertFalse(((Collection) rsp._get("node-roles/overseer/" + overseerModeOnDataNode, null)).contains(j1.getNodeName()));
+  }
+
+  @SuppressWarnings("rawtypes")
+  private void validateNodeRoles(String... nodenamePaths) throws org.apache.solr.client.solrj.SolrServerException, java.io.IOException {
+    V2Response rsp = new V2Request.Builder("/cluster/node-roles").GET().build().process(cluster.getSolrClient());
+    for (int i = 0; i < nodenamePaths.length; i += 2) {
+      String nodename = nodenamePaths[i];
+      String path = nodenamePaths[i + 1];
+      assertTrue("Didn't find " + nodename + " at " + path + ". Full response: " + rsp.jsonStr(),
+              ((Collection) rsp._get(path, Collections.emptyList())).contains(nodename));
+    }
+  }
+
+  @SuppressWarnings("unchecked")
+  private void testSupportedRolesAPI() throws Exception {
+    V2Response rsp = new V2Request.Builder("/cluster/node-roles/supported").GET().build().process(cluster.getSolrClient());
+    Map<String, Object> l = (Map<String, Object>) rsp._get("supported-roles", Collections.emptyMap());
+    assertTrue(l.containsKey("data"));
+    assertTrue(l.containsKey("overseer"));
+  }
+
+  private JettySolrRunner startNodeWithRoles(String roles) throws Exception {

Review comment:
       Each instance of this test runs on a separate JVM, so it shouldn't be a problem running such tests in parallel.

##########
File path: solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java
##########
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.cloud;
+
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+
+import java.util.Map;
+
+import org.apache.solr.client.solrj.embedded.JettySolrRunner;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.V2Request;
+import org.apache.solr.client.solrj.response.V2Response;
+import org.apache.solr.core.NodeRoles;
+import org.junit.After;
+import org.junit.Before;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRolesTest extends SolrCloudTestCase {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @Before
+  public void setupCluster() throws Exception {
+    configureCluster(1)
+            .addConfig("conf", configset("cloud-minimal"))
+            .configure();
+  }
+
+  @After
+  public void tearDownCluster() throws Exception {
+    shutdownCluster();
+  }
+
+  @SuppressWarnings("rawtypes")
+  public void testRoleIntegration() throws Exception {
+    JettySolrRunner j0 = cluster.getJettySolrRunner(0);
+    testSupportedRolesAPI();
+
+    // Start a dedicated overseer node
+    JettySolrRunner j1 = startNodeWithRoles("overseer:preferred,data:off");
+    validateNodeRoles(j1.getNodeName(), "node-roles/overseer/preferred", j1.getNodeName(), "node-roles/data/off");
+
+    V2Response rsp;
+    OverseerRolesTest.waitForNewOverseer(20, j1.getNodeName(), true);
+
+    // Start another node that is allowed or preferred overseer but has data
+    String overseerModeOnDataNode = random().nextBoolean() ? "preferred" : "allowed";
+    JettySolrRunner j2 = startNodeWithRoles("overseer:" + overseerModeOnDataNode + ",data:on");
+    validateNodeRoles(j2.getNodeName(), "node-roles/overseer/" + overseerModeOnDataNode, j2.getNodeName(), "node-roles/data/on");
+
+    // validate the preferred overseers
+    validateNodeRoles(j2.getNodeName(), "node-roles/overseer/" + overseerModeOnDataNode, j1.getNodeName(), "node-roles/overseer/preferred");
+
+    String COLLECTION_NAME = "TEST_ROLES";
+    CollectionAdminRequest
+            .createCollection(COLLECTION_NAME, "conf", 3, 1)
+            .process(cluster.getSolrClient());
+    cluster.waitForActiveCollection(COLLECTION_NAME, 3, 3);
+
+    // Assert that no replica was placed on the dedicated overseer node
+    String dedicatedOverseer = j1.getNodeName();
+    cluster.getSolrClient().getClusterStateProvider().getCollection(COLLECTION_NAME)
+            .forEachReplica((s, replica) -> assertNotEquals(replica.node, dedicatedOverseer));
+
+    // Shutdown the dedicated overseer, make sure that node disappears from the roles output
+    j1.stop();
+
+    // Wait and make sure that another node picks up overseer responsibilities
+    OverseerRolesTest.waitForNewOverseer(20, it -> !dedicatedOverseer.equals(it), false);
+
+    // Make sure the stopped node no longer has the role assigned
+    rsp = new V2Request.Builder("/cluster/node-roles/role/overseer/" + overseerModeOnDataNode).GET().build().process(cluster.getSolrClient());
+    assertFalse(((Collection) rsp._get("node-roles/overseer/" + overseerModeOnDataNode, null)).contains(j1.getNodeName()));
+  }
+
+  @SuppressWarnings("rawtypes")

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +65,191 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
   @EndPoint(method = GET,
-      path = "/cluster/aliases",
-      permission = COLL_READ_PERM)
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result;
+    try {
+      List<String> children = zk.listData(path);
+      if (children != null && !children.isEmpty()) {
+        result = new HashMap<>();
+      } else {
+        return depth >= 1 ? Collections.emptySet(): null;

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles {
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  /**
+   * Roles to be assumed on nodes that don't have roles specified for them at startup
+   */
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, Mode> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, Mode> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      Mode m = Mode.valueOf(roleMode.get(1).toUpperCase(Locale.ROOT));
+      if (r.supportedModes().contains(m)) {
+        roles.put(r, m);
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
+                "Unknown role mode '" + roleMode.get(1) + "' for role '" + r + "'");
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.modeWhenRoleIsAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, Mode> getRoles() {
+    return nodeRoles;
+  }
+
+  public Mode getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  public boolean isOverseerAllowedOrPreferred() {
+    Mode roleMode = nodeRoles.get(Role.OVERSEER);
+    return Mode.ALLOWED.equals(roleMode) || Mode.PREFERRED.equals(roleMode);
+  }
+
+  public enum Mode {
+    ON, OFF, ALLOWED, PREFERRED, DISALLOWED;

Review comment:
       Fixed, thanks. Changed from using strings to enum based on Ilan's suggestion, but moved back to using strings now.

##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerRoleCmd.java
##########
@@ -64,6 +64,10 @@ public void call(ClusterState state, ZkNodeProps message, NamedList<Object> resu
     SolrZkClient zkClient = zkStateReader.getZkClient();
     Map<String, List<String>> roles = null;
     String node = message.getStr("node");
+    if ("false".equals(message.getStr("persist"))) {// no need to persist to roles.json

Review comment:
       This isn't a new feature, just a parameter added to the ADDROLE.

##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##########
@@ -2211,6 +2225,15 @@ public void checkOverseerDesignate() {
     }
   }
 
+  public void setPreferredOverseer() throws KeeperException, InterruptedException {
+    ZkNodeProps props = new ZkNodeProps(Overseer.QUEUE_OPERATION, ADDROLE.toString().toLowerCase(Locale.ROOT),
+        "node", getNodeName(),
+        "role", "overseer",
+        "persist", "false");
+    log.info("Going to add role {} ", props);

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
##########
@@ -244,13 +235,28 @@ private static boolean existCoreName(String coreName, Slice slice) {
         Collections.shuffle(nodeList, random);
       }
     } else {
-      nodeList = new ArrayList<>(liveNodes);
+      nodeList = new ArrayList<>(filterNonDataNodes(zk, liveNodes));
       Collections.shuffle(nodeList, random);
     }
 
     return nodeList;
   }
 
+  public static Collection<String> filterNonDataNodes(DistribStateManager zk, Collection<String> liveNodes) {

Review comment:
       I don't see any other use here for filtering on a generic role. In case we need later, we can add so.

##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
##########
@@ -21,20 +21,7 @@
 
 import java.io.IOException;
 import java.lang.invoke.MethodHandles;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Locale;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Random;
-import java.util.Set;
+import java.util.*;

Review comment:
       Fixed, thanks.

##########
File path: solr/core/src/java/org/apache/solr/cluster/Cluster.java
##########
@@ -31,6 +31,7 @@
    */
   Set<Node> getLiveNodes();
 
+  Set<Node> getLiveNodes(boolean filterNonDataNodes) ;

Review comment:
       Fixed, thanks.

##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
##########
@@ -103,6 +103,16 @@
   public static final String STATE_TIMESTAMP_PROP = "stateTimestamp";
   public static final String COLLECTIONS_ZKNODE = "/collections";
   public static final String LIVE_NODES_ZKNODE = "/live_nodes";
+
+  /**
+   * The following, node_roles and roles.json are for assigning roles to
+   * nodes. The node_roles is the preferred way (using -Dsolr.node.roles param),
+   * and roles.json is used by legacy ADDROLE API command.
+   * TODO: Deprecate and remove support for roles.json in an upcoming release.

Review comment:
       Fixed, thanks.

##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,196 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.
+
+== Definitions
+
+=== Node role
+
+A role is a designation of a node that indicates that the node may perform a certain functionality that is governed by the role.

Review comment:
       Fixed, thanks.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman edited a comment on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman edited a comment on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-971449610


   In this PR, only a single role can be assigned to a node, and I'm fixing that. Changes to come soon, hence this is not yet ready for reviewing. Sorry for the confusion. Also, merging this PR can't happen now, until the SIP-15 has been voted for.


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] noblepaul commented on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-1007849998


   Thanks everyone for participating 


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] noblepaul closed pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
noblepaul closed pull request #403:
URL: https://github.com/apache/solr/pull/403


   


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r779914569



##########
File path: solr/core/src/java/org/apache/solr/cluster/Cluster.java
##########
@@ -31,6 +31,11 @@
    */
   Set<Node> getLiveNodes();
 
+  /**
+   * @return current set of live nodes that are supposed to host data.

Review comment:
       *MissingSummary:*  A summary fragment is required; consider using the value of the @return block as a summary fragment instead. [(details)](https://google.github.io/styleguide/javaguide.html#s7.2-summary-fragment)
   (at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] noblepaul commented on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
noblepaul commented on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-1007849998


   Thanks everyone for participating 


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r777331393



##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,198 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.
+
+== Definitions
+
+=== Node role
+
+A role is a designation of a node that indicates that the node may perform a certain functionality that is governed by the role.
+
+=== Mode
+Every role has a list of modes under which a node can be. It can be simple (e.g. `["on", "off"]`) or more granular (e.g. `["allowed", "preferred", "disallowed"]`).
+
+== Roles
+
+In order to specify role(s) for a node, one needs to start a Solr node with the following parameter.
+
+.Startup Parameter
+[cols="1,2,1,1"] 
+|===
+|Parameter |Value |Required? | Default
+
+|solr.node.roles
+|Comma separated list of roles (in the format: `<role>:<mode>`) for this node.
+Examples: `-Dsolr.node.roles=data:on,overseer:allowed` or `-Dsolr.node.roles=overseer:preferred`
+|No
+|`data:on,overseer:allowed`
+|===
+
+[TIP]
+====
+If a node has been started with no `solr.node.roles` parameter, it will be assumed to have the data role turned on and overseer role allowed on it. If you've never used roles before, you likely won't need to change anything in your startup parameters to accomodate the functionality associated with these roles.
+====
+
+.Supported roles
+[cols="1,1"] 
+|===
+|Role |Modes
+
+|`data`
+|on, off
+
+|`overseer`
+|allowed, preferred, disallowed
+|===
+
+=== `data` role
+A node with this role (in mode "on") can host shards and replicas for collections.
+
+=== `overseer` role
+A node with this role can perform duties of an overseer node (unless mode is `disallowed`). When one or more nodes have the overseer role in `preferred` mode, the overseer leader will be elected from one of these nodes. In case no node is designated as a preferred overseer or no such node is live, the overseer leader will be elected from one of the nodes that have the overseer role in `allowed` mode. If all nodes that are designated with overseer role (allowed or preferred) are down, the cluster will be left without an overseer.
+
+== Example usage
+
+Sometimes, when the nodes in a cluster are under heavy querying or indexing load, the overseer leader node might be unable to perform collection management duties efficiently. It might be reasonable to have dedicated nodes to act as the overseer. Such an effect can be achieved as follows:
+
+* Most nodes (data nodes) in the cluster start with `-Dsolr.node.roles=data:on,overseer:allowed` (or with no parameter, since the default value for `solr.node.roles` is the same).

Review comment:
       I don't think per role default is a sensible idea, I'd rather stick with per node default string. I had explained this in the SIP thread as follows:
   
   ```
      * User upgrades from Solr 9.0 to 9.1, where "ui" role has been introduced. Developers of "ui" role want it to be available for most users.
           - In Ilan's proposal, the developer chooses this in 9.1: ui: {modes: [on, off], default1: on, default2: on}. Now, user upgrading will see that UI is running on his two [dedicated] overseer nodes, and he's confused (because he explicitly specified what he wants)
           - In my proposal, the developer chooses ui: {modes: [on, off]}; default roles for those users who don't specify roles: "data:on, overseer:allowed, ui:on". Now, there are no surprises of implicit default. Users who don't use roles at all will get this functionality turned on, just as the developer wanted. Users who use roles will have to explicitly append "ui:on" to their roles string on their nodes during the upgrade (this tip will come from the upgrade notes).
   ```
   
   In short, a user who specified "-Dsolr.node.roles=overseer:preferred" wants the node to only do overseer duties and nothing else, even if a Solr upgrade has a new role added that is supposed to be turned on by default on nodes where no role has been specified.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-971457263


   > While you're at it Ishan, please fix the formatting of the many if(... that lack a space after the if and change them into:
   
   Sure, Ilan. :-)


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r779831163



##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,198 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.
+
+== Definitions
+
+=== Node role
+
+A role is a designation of a node that indicates that the node may perform a certain functionality that is governed by the role.
+
+=== Mode
+Every role has a list of modes under which a node can be. It can be simple (e.g. `["on", "off"]`) or more granular (e.g. `["allowed", "preferred", "disallowed"]`).
+
+== Roles
+
+In order to specify role(s) for a node, one needs to start a Solr node with the following parameter.
+
+.Startup Parameter
+[cols="1,2,1,1"] 
+|===
+|Parameter |Value |Required? | Default
+
+|solr.node.roles
+|Comma separated list of roles (in the format: `<role>:<mode>`) for this node.
+Examples: `-Dsolr.node.roles=data:on,overseer:allowed` or `-Dsolr.node.roles=overseer:preferred`
+|No
+|`data:on,overseer:allowed`
+|===
+
+[TIP]
+====
+If a node has been started with no `solr.node.roles` parameter, it will be assumed to have the data role turned on and overseer role allowed on it. If you've never used roles before, you likely won't need to change anything in your startup parameters to accomodate the functionality associated with these roles.
+====
+
+.Supported roles

Review comment:
       There is no concept of a default mode for a role.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r779830910



##########
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##########
@@ -944,6 +946,14 @@ public void load() {
       });
 
       clusterSingletons.setReady();
+      if (NodeRoles.PREFERRED.equals(nodeRoles.getRoleMode(NodeRoles.Role.OVERSEER))) {
+        try {
+          log.info("This node is started as a preferred overseer");
+          zkSys.getZkController().setPreferredOverseer();
+        } catch (KeeperException | InterruptedException e) {
+          throw new SolrException(ErrorCode.SERVER_ERROR, e);
+        }
+      }
       if (!distributedCollectionCommandRunner.isPresent()) {

Review comment:
       Fixed, thanks.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] janhoy commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r772699089



##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {
+    String roleMode = nodeRoles.get(Role.OVERSEER);
+    return ALLOWED.equals(roleMode) || PREFERRED.equals(roleMode);
+  }
+
+  public enum Role {
+    DATA("data"),
+    OVERSEER("overseer") {

Review comment:
       With the choice of Role being a hardcoded enum, will it not ever be possible for plugins / packages to introduce a new role?




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] murblanc edited a comment on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
murblanc edited a comment on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-996930678


   > This is now ready for review. FYI, @murblanc.
   
   will try to  have a look this week-end, otherwise early next week. [Edit: done]


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-1006908991


   > `PlacementRequestImpl.toPlacementRequest` needs to be updated (when nodes are passed in and when not) to filter out nodes that can't take data.
   
   Fixed, thanks.


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] murblanc commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
murblanc commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r777504751



##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,198 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.
+
+== Definitions
+
+=== Node role
+
+A role is a designation of a node that indicates that the node may perform a certain functionality that is governed by the role.
+
+=== Mode
+Every role has a list of modes under which a node can be. It can be simple (e.g. `["on", "off"]`) or more granular (e.g. `["allowed", "preferred", "disallowed"]`).
+
+== Roles
+
+In order to specify role(s) for a node, one needs to start a Solr node with the following parameter.
+
+.Startup Parameter
+[cols="1,2,1,1"] 
+|===
+|Parameter |Value |Required? | Default
+
+|solr.node.roles
+|Comma separated list of roles (in the format: `<role>:<mode>`) for this node.
+Examples: `-Dsolr.node.roles=data:on,overseer:allowed` or `-Dsolr.node.roles=overseer:preferred`
+|No
+|`data:on,overseer:allowed`
+|===
+
+[TIP]
+====
+If a node has been started with no `solr.node.roles` parameter, it will be assumed to have the data role turned on and overseer role allowed on it. If you've never used roles before, you likely won't need to change anything in your startup parameters to accomodate the functionality associated with these roles.
+====
+
+.Supported roles
+[cols="1,1"] 
+|===
+|Role |Modes
+
+|`data`
+|on, off
+
+|`overseer`
+|allowed, preferred, disallowed
+|===
+
+=== `data` role
+A node with this role (in mode "on") can host shards and replicas for collections.
+
+=== `overseer` role
+A node with this role can perform duties of an overseer node (unless mode is `disallowed`). When one or more nodes have the overseer role in `preferred` mode, the overseer leader will be elected from one of these nodes. In case no node is designated as a preferred overseer or no such node is live, the overseer leader will be elected from one of the nodes that have the overseer role in `allowed` mode. If all nodes that are designated with overseer role (allowed or preferred) are down, the cluster will be left without an overseer.
+
+== Example usage
+
+Sometimes, when the nodes in a cluster are under heavy querying or indexing load, the overseer leader node might be unable to perform collection management duties efficiently. It might be reasonable to have dedicated nodes to act as the overseer. Such an effect can be achieved as follows:
+
+* Most nodes (data nodes) in the cluster start with `-Dsolr.node.roles=data:on,overseer:allowed` (or with no parameter, since the default value for `solr.node.roles` is the same).

Review comment:
       Having a per role default (`defaultIfAbsent`) as well as a default for all roles when no role is specified (default value for `solr.node.roles`) is obviously more complex than only per role default (understanding how a Solr node behaves requires looking at two places to understand default behavior).
   
   One can assume that a Solr deployment where `solr.node.roles` is overridden on each node (an advanced or "expert" cluster config) would be careful on upgrades and will not fall into the "ui" issue you mention.
   
   And in the general case, with roles independent of each other, picking a node to be (or not to be) overseer for example will have a side effect impact on all other roles (switching them from default value in `solr.node.roles` to per role default in `defaultIfAbsent`).
   So a user not caring about roles in general but only wanting to exclude a given node from being Overseer would have to go look at the source code and do so on each code update and copy all defaults into its node's `solr.node.roles`, rather than just setting overseer to disallowed there.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r777409802



##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +65,193 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result;
+    try {
+      List<String> children = zk.listData(path);
+      if (children != null && !children.isEmpty()) {
+        result = new HashMap<>();
+      } else {
+        return depth >= 1 ? Collections.emptySet(): null;
+      }
+      for (String child: children) {
+        Object c = readRecursive(path + "/" + child, zk, depth - 1);
+        result.put(child, c);
+      }
+    } catch (Exception e) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+    }
+    if (depth == 1) {
+      return result.keySet();
+    } else {
+      return result;
+    }
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}",
+          permission = COLL_READ_PERM)
+  public void nodesWithRole(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String role = req.getPathTemplateValues().get("role");
+    rsp.add("node-roles", Map.of(role,
+            readRecursive(ZkStateReader.NODE_ROLES + "/" + role,
+                    collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 2)));
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/node/{node}",
+          permission = COLL_READ_PERM)
+  @SuppressWarnings("unchecked")
+  public void rolesForNode(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String node = req.getPathTemplateValues().get("node");
+    Map<String, String> ret = new HashMap<String, String>();
+    Map<String, Map<String, Set<String>>> roles = (Map<String, Map<String, Set<String>>>) readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3);
+    for (String role: roles.keySet()) {
+      for (String mode: roles.get(role).keySet()) {
+        if (roles.get(role).get(mode).isEmpty()) continue;
+        Set<String> nodes = roles.get(role).get(mode);
+        if (nodes.contains(node)) ret.put(role, mode);
+      }
+    }
+    for (String role: ret.keySet()) {
+      rsp.add(role, ret.get(role));
+    }
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/supported",
+          permission = COLL_READ_PERM)
+  public void supportedRoles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+     Map<String, Object> roleModesSupportedMap = new HashMap<>();
+    for (NodeRoles.Role role: NodeRoles.Role.values()) {
+      roleModesSupportedMap.put(role.toString(),
+              Map.of("modes", (role.supportedModes()).stream().map(NodeRoles.Mode::toString).collect(Collectors.toList())));

Review comment:
       *UnnecessaryParentheses:*  These grouping parentheses are unnecessary; it is unlikely the code will be misinterpreted without them [(details)](https://errorprone.info/bugpattern/UnnecessaryParentheses)
   (at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

Review comment:
       *UnusedVariable:*  The field 'log' is never read. [(details)](https://errorprone.info/bugpattern/UnusedVariable)
   (at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +65,193 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

Review comment:
       *UnusedVariable:*  The field 'log' is never read. [(details)](https://errorprone.info/bugpattern/UnusedVariable)
   (at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +65,193 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result;
+    try {
+      List<String> children = zk.listData(path);
+      if (children != null && !children.isEmpty()) {
+        result = new HashMap<>();
+      } else {
+        return depth >= 1 ? Collections.emptySet(): null;
+      }
+      for (String child: children) {
+        Object c = readRecursive(path + "/" + child, zk, depth - 1);
+        result.put(child, c);
+      }
+    } catch (Exception e) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+    }
+    if (depth == 1) {
+      return result.keySet();
+    } else {
+      return result;
+    }
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}",
+          permission = COLL_READ_PERM)
+  public void nodesWithRole(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String role = req.getPathTemplateValues().get("role");
+    rsp.add("node-roles", Map.of(role,
+            readRecursive(ZkStateReader.NODE_ROLES + "/" + role,
+                    collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 2)));
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/node/{node}",
+          permission = COLL_READ_PERM)
+  @SuppressWarnings("unchecked")
+  public void rolesForNode(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String node = req.getPathTemplateValues().get("node");
+    Map<String, String> ret = new HashMap<String, String>();
+    Map<String, Map<String, Set<String>>> roles = (Map<String, Map<String, Set<String>>>) readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3);
+    for (String role: roles.keySet()) {
+      for (String mode: roles.get(role).keySet()) {
+        if (roles.get(role).get(mode).isEmpty()) continue;
+        Set<String> nodes = roles.get(role).get(mode);

Review comment:
       *INEFFICIENT_KEYSET_ITERATOR:*  Accessing a value using a key that was retrieved from a `keySet` iterator. It is more efficient to use an iterator on the `entrySet` of the map, avoiding the extra `HashMap.get(key)` lookup.
   (at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] murblanc commented on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-996930678


   > This is now ready for review. FYI, @murblanc.
   
   will try to  have a look this week-end, otherwise early next week.


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] murblanc commented on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
murblanc commented on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-971617906


   > * the /api/cluster/roles should also list nodes that didn't have -Dsolr.node.roles (they are assumed to have data role)
   
   Making sure we agree on the fact that nodes without defined roles are assumed to have **all roles**, not only data role.


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] murblanc commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
murblanc commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r772756813



##########
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##########
@@ -944,6 +946,14 @@ public void load() {
       });
 
       clusterSingletons.setReady();
+      if (NodeRoles.PREFERRED.equals(nodeRoles.getRoleMode(NodeRoles.Role.OVERSEER))) {
+        try {
+          log.info("This node is started as a preferred overseer");

Review comment:
       Given the goal of a role is to influence behavior (here related to Overseer, elsewhere to placing or not placing data, in the future other aspects), I don't see how to make this pluggable. Sure the definition of the role can be pluggable, but to make the role be able to actually change the behavior of Solr, existing classes have to be patched.
   
   What you suggest @janhoy is possible (discovering/initializing all roles and letting them do that work), but the missing bit is then for roles that want to change some specific behavior aspects of SolrCloud or interact with it in a different way than tweaking its configuration.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-971607650


   TODO items:
   * deprecate ADDROLE and REMOVEROLE commands
   * throw exception when a node containing cores is started without a data role
   * the /api/cluster/roles should also list nodes that didn't have -Dsolr.node.roles (they are assumed to have data role)
   
   I think except for these items, this PR is ready for reviewing. FYI @noblepaul @murblanc @hiteshk25. It should only be merged once the SIP-15 voting has concluded.


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] noblepaul closed pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
noblepaul closed pull request #403:
URL: https://github.com/apache/solr/pull/403


   


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] janhoy commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r772698361



##########
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##########
@@ -944,6 +946,14 @@ public void load() {
       });
 
       clusterSingletons.setReady();
+      if (NodeRoles.PREFERRED.equals(nodeRoles.getRoleMode(NodeRoles.Role.OVERSEER))) {
+        try {
+          log.info("This node is started as a preferred overseer");

Review comment:
       Do we want role-specific if-then-else code in CoreContainer? I think there were some questions in the SIP discussion regarding where role logic should reside, and this does not seem very pluggable nor scalable.
   
   What if NodeRole was an interface and each role was an implementation. Then CoreContainer could call some init method on each configured role when the node starts up, and that class would then do whatever it wants wrt reading its config, logging, talking to zk etc?




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] janhoy commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r772762468



##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {
+    String roleMode = nodeRoles.get(Role.OVERSEER);
+    return ALLOWED.equals(roleMode) || PREFERRED.equals(roleMode);
+  }
+
+  public enum Role {
+    DATA("data"),
+    OVERSEER("overseer") {

Review comment:
       You could imagine CDCR re-appearing as a package, and that it needs dedicated nodes to sync with other clusters. Or a new Tika package which wants to manage a set of nodes exposing (managed) TikaServer for parsing PDFs etc. Of course packages could re-invent their own role concept, and tag some nodes with `NODE_ROLES=[]` to tell solr-core to not assign anything else to those nodes. But I was more thinking that a role is just a namespace, and solr-core has some roles ootb, but other new roles could be registered without editing a global enum? Note, I have not sat down to try to design this - feels like a topic like whether roles should be pluggable should be discussed on SIP level.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] janhoy commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r772762468



##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {
+    String roleMode = nodeRoles.get(Role.OVERSEER);
+    return ALLOWED.equals(roleMode) || PREFERRED.equals(roleMode);
+  }
+
+  public enum Role {
+    DATA("data"),
+    OVERSEER("overseer") {

Review comment:
       You could imagine CDCR re-appearing as a package, and that it needs dedicated nodes to sync with other clusters. Or a new Tika package which wants to manage a set of nodes exposing (managed) TikaServer for parsing PDFs etc. Of course packages could re-invent their own role concept, and tag some nodes with `NODE_ROLES=[]` to tell solr-core to not assign anything else to those nodes. But I was more thinking that a role is just a namespace, and solr-core has some roles ootb, but other new roles could be registered without editing a global enum? Note, I have not sat down to try to design this - feels like a topic like whether roles should be pluggable should be discussed on POC level.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r779833151



##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,198 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.
+
+== Definitions
+
+=== Node role
+
+A role is a designation of a node that indicates that the node may perform a certain functionality that is governed by the role.
+
+=== Mode
+Every role has a list of modes under which a node can be. It can be simple (e.g. `["on", "off"]`) or more granular (e.g. `["allowed", "preferred", "disallowed"]`).
+
+== Roles
+
+In order to specify role(s) for a node, one needs to start a Solr node with the following parameter.
+
+.Startup Parameter
+[cols="1,2,1,1"] 
+|===
+|Parameter |Value |Required? | Default
+
+|solr.node.roles
+|Comma separated list of roles (in the format: `<role>:<mode>`) for this node.
+Examples: `-Dsolr.node.roles=data:on,overseer:allowed` or `-Dsolr.node.roles=overseer:preferred`
+|No
+|`data:on,overseer:allowed`
+|===
+
+[TIP]
+====
+If a node has been started with no `solr.node.roles` parameter, it will be assumed to have the data role turned on and overseer role allowed on it. If you've never used roles before, you likely won't need to change anything in your startup parameters to accomodate the functionality associated with these roles.
+====
+
+.Supported roles
+[cols="1,1"] 
+|===
+|Role |Modes
+
+|`data`
+|on, off
+
+|`overseer`
+|allowed, preferred, disallowed
+|===
+
+=== `data` role
+A node with this role (in mode "on") can host shards and replicas for collections.
+
+=== `overseer` role
+A node with this role can perform duties of an overseer node (unless mode is `disallowed`). When one or more nodes have the overseer role in `preferred` mode, the overseer leader will be elected from one of these nodes. In case no node is designated as a preferred overseer or no such node is live, the overseer leader will be elected from one of the nodes that have the overseer role in `allowed` mode. If all nodes that are designated with overseer role (allowed or preferred) are down, the cluster will be left without an overseer.
+
+== Example usage
+
+Sometimes, when the nodes in a cluster are under heavy querying or indexing load, the overseer leader node might be unable to perform collection management duties efficiently. It might be reasonable to have dedicated nodes to act as the overseer. Such an effect can be achieved as follows:
+
+* Most nodes (data nodes) in the cluster start with `-Dsolr.node.roles=data:on,overseer:allowed` (or with no parameter, since the default value for `solr.node.roles` is the same).

Review comment:
       > Having a per role default (defaultIfAbsent) as well as a default for all roles when no role is specified (default value for solr.node.roles) is obviously more complex than only per role default (understanding how a Solr node behaves requires looking at two places to understand default behavior).
   
   
   There is no per role default. What used to be called "defaultIfAbsent" is now renamed as "modeWhenRoleIsAbsent", and is used for internal purposes anyway.
   
   > One can assume that a Solr deployment where solr.node.roles is overridden on each node (an advanced or "expert" cluster config) would be careful on upgrades and will not fall into the "ui" issue you mention.
   
   I don't think we should make such an assumption. An upgrade should come with no surprises to the user, unless he/she explicitly wants to opt into a role introduced in the new release.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on pull request #403:
URL: https://github.com/apache/solr/pull/403#issuecomment-996551502


   This is now ready for review. FYI, @murblanc.


-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] murblanc commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
murblanc commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r772475914



##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";

Review comment:
       The default should be the defaults of each role rather than a string with a list of roles and values. this string should be removed.
   
   The issue with such a string is that if a single role is defined on a node, the change of roles of that node might impact other roles as they'll change from having the value defined in this string to the default for the role.

##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
##########
@@ -243,12 +247,24 @@ private static boolean existCoreName(String coreName, Slice slice) {
       }
     } else {
       nodeList = new ArrayList<>(liveNodes);
+      filterNonDataNodes(zk, nodeList);
       Collections.shuffle(nodeList, random);
     }
 
     return nodeList;
   }
 
+  public static void filterNonDataNodes(DistribStateManager zk, List<String> liveNodes) {
+    try {
+     List<String> noData =  ClusterAPI.getNodesByRole(NodeRoles.Role.DATA, NodeRoles.OFF, zk);
+     if(!noData.isEmpty()){

Review comment:
       formatting: add space after `if`.
   But that `if` should be removed altogether, `removeAll()` will remove nothing if list is empty.

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {
+    String roleMode = nodeRoles.get(Role.OVERSEER);
+    return ALLOWED.equals(roleMode) || PREFERRED.equals(roleMode);
+  }
+
+  public enum Role {
+    DATA("data"),
+    OVERSEER("overseer") {
+      @Override
+      public Set<String> supportedModes() {
+        return OVERSEER_MODES;
+      }
+      @Override
+      public String defaultIfAbsent() {
+        return DISALLOWED;
+      }
+    };
+
+    public final String roleName;
+
+    Role(String name) {
+      this.roleName = name;
+    }
+
+    public static Role getRole(String value) {
+      for (Role role: Role.values()) {
+        if (value.equals(role.roleName)) return role;
+      }
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role: " + value);
+    }
+
+    public Set<String> supportedModes() {
+      return ON_OFF;
+    }
+
+    /**
+     * Default mode for a role in nodes where this role is not specified.
+     */
+    public String defaultIfAbsent() {
+      return OFF;
+    }
+
+    @Override
+    public String toString() {
+      return super.toString().toLowerCase(Locale.ROOT);

Review comment:
       Any reason for this specific override, related to `MapWriter` maybe? If so, please comment.
   But if we rely on `roleName` then just return it.
   
   If no specific reason, I suggest we let the default `toString()` implementation return the enum name, so just remove this override.

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;

Review comment:
       Here if no roles are passed, I'd use the `defaultIfAbsent` of all the roles (i.e. if `null`, set to empty string, the logic below will take care of the rest).

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +64,194 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    Map<String, Object> result = new LinkedHashMap<>();
+
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result = null;
+    try {
+      List<String> children = zk.listData(path);
+      if (children != null && !children.isEmpty()) {
+        result = new HashMap<>();
+      } else {
+        return depth >= 1 ? Collections.emptyList() : null;
+      }
+      for (String child : children) {
+        Object c = readRecursive(path + "/" + child, zk, depth - 1);
+        result.put(child, c);
+      }
+    } catch (Exception e) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+    }
+    if (depth == 1) {
+      return result.keySet();
+    } else {
+      return result;
+    }
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}",
+          permission = COLL_READ_PERM)
+  public void nodesWithRole(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String role = req.getPathTemplateValues().get("role");
+    rsp.add("node-roles", Map.of(role,
+            readRecursive(ZkStateReader.NODE_ROLES + "/"+ role,
+                    collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 2)));
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/node/{node}",
+          permission = COLL_READ_PERM)
+  @SuppressWarnings("unchecked")
+  public void rolesForNode(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String node = req.getPathTemplateValues().get("node");
+    Map<String, String> ret = new HashMap<String, String>();
+    Map<String, Map<String, Set<String>>> roles = (Map<String, Map<String, Set<String>>>) readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3);
+    for (String role: roles.keySet()) {
+      for (String mode : roles.get(role).keySet()) {
+        if (roles.get(role).get(mode) instanceof List && ((List) roles.get(role).get(mode)).size() == 0) {
+          continue;
+        }
+        Set<String> nodes = roles.get(role).get(mode);
+        if (nodes.contains(node)) ret.put(role, mode);
+      }
+    }
+    for (String role: ret.keySet()) {
+      rsp.add(role, ret.get(role));
+    }
+  }
+
   @EndPoint(method = GET,
-      path = "/cluster/aliases",
-      permission = COLL_READ_PERM)
+          path = "/cluster/node-roles/supported",
+          permission = COLL_READ_PERM)
+  public void supportedRoles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+     Map<String, Object> roleModesSupportedMap = new HashMap<>();
+    for (NodeRoles.Role role: NodeRoles.Role.values()) {
+      roleModesSupportedMap.put(role.toString(), Map.of("modes", role.supportedModes(), "defaultIfAbsent", role.defaultIfAbsent()));
+    }
+    rsp.add("supported-roles", roleModesSupportedMap);
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}/{role-val}",

Review comment:
       Do roles have "modes" or "values"? Need consistency. Here it's value, in `NodeRoles` it's modes.

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));

Review comment:
       Need to add in the message the role name for which an unknown mode is configured.

##########
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##########
@@ -944,6 +946,14 @@ public void load() {
       });
 
       clusterSingletons.setReady();
+      if (NodeRoles.PREFERRED.equals(nodeRoles.getRoleMode(NodeRoles.Role.OVERSEER))) {
+        try {
+          log.info("This node is started as a preferred overseer");
+          zkSys.getZkController().setPreferredOverseer();
+        } catch (KeeperException | InterruptedException e) {
+          throw new SolrException(ErrorCode.SERVER_ERROR, e);
+        }
+      }
       if (!distributedCollectionCommandRunner.isPresent()) {

Review comment:
       The added bloc above must be moved inside this if otherwise we will be sending a message over the overseer queue when no overseer is running to consume it.

##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##########
@@ -913,7 +924,9 @@ private void init() {
         ElectionContext context = new OverseerElectionContext(zkClient,
             overseer, getNodeName());
         overseerElector.setup(context);
-        overseerElector.joinElection(context, false);
+        if(cc.nodeRoles.isOverseerAllowed()) {

Review comment:
       formatting: please add space after `if`.

##########
File path: solr/core/src/java/org/apache/solr/cloud/OverseerNodePrioritizer.java
##########
@@ -59,11 +63,18 @@ public OverseerNodePrioritizer(ZkStateReader zkStateReader, Overseer overseer, S
 
   public synchronized void prioritizeOverseerNodes(String overseerId) throws Exception {
     SolrZkClient zk = zkStateReader.getZkClient();
-    if(!zk.exists(ZkStateReader.ROLES,true))return;
-    Map<?,?> m = (Map<?,?>) Utils.fromJSON(zk.getData(ZkStateReader.ROLES, null, new Stat(), true));
+    List<String> overseerDesignates = new ArrayList<>();
+    if (zk.exists(ZkStateReader.ROLES,true)) {
+      Map<?,?> m = (Map<?,?>) Utils.fromJSON(zk.getData(ZkStateReader.ROLES, null, new Stat(), true));
+      @SuppressWarnings("unchecked")
+      List<String> l = (List<String>) m.get("overseer");
+      if (l != null) {
+        overseerDesignates.addAll(l);
+      }
+    }
 
-    List<?> overseerDesignates = (List<?>) m.get("overseer");
-    if(overseerDesignates==null || overseerDesignates.isEmpty()) return;
+    overseerDesignates.addAll(ClusterAPI.getNodesByRole(NodeRoles.Role.OVERSEER, NodeRoles.PREFERRED, new ZkDistribStateManager(zkStateReader.getZkClient())));

Review comment:
       I'd log a warning if Overseer role configured in both `/roles.json` and in (new) node roles.

##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,198 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.
+
+== Definitions
+
+=== Node role
+
+A role is a designation of a node that indicates that the node may perform a certain functionality that is governed by the role.
+
+=== Mode
+Every role has a list of modes under which a node can be. It can be simple (e.g. `["on", "off"]`) or more granular (e.g. `["allowed", "preferred", "disallowed"]`).
+
+== Roles
+
+In order to specify role(s) for a node, one needs to start a Solr node with the following parameter.
+
+.Startup Parameter
+[cols="1,2,1,1"] 
+|===
+|Parameter |Value |Required? | Default
+
+|solr.node.roles
+|Comma separated list of roles (in the format: `<role>:<mode>`) for this node.
+Examples: `-Dsolr.node.roles=data:on,overseer:allowed` or `-Dsolr.node.roles=overseer:preferred`
+|No
+|`data:on,overseer:allowed`
+|===
+
+[TIP]
+====
+If a node has been started with no `solr.node.roles` parameter, it will be assumed to have the data role turned on and overseer role allowed on it. If you've never used roles before, you likely won't need to change anything in your startup parameters to accomodate the functionality associated with these roles.

Review comment:
       typo: accommodate (two m)

##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,198 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.
+
+== Definitions
+
+=== Node role
+
+A role is a designation of a node that indicates that the node may perform a certain functionality that is governed by the role.
+
+=== Mode
+Every role has a list of modes under which a node can be. It can be simple (e.g. `["on", "off"]`) or more granular (e.g. `["allowed", "preferred", "disallowed"]`).
+
+== Roles
+
+In order to specify role(s) for a node, one needs to start a Solr node with the following parameter.
+
+.Startup Parameter
+[cols="1,2,1,1"] 
+|===
+|Parameter |Value |Required? | Default
+
+|solr.node.roles
+|Comma separated list of roles (in the format: `<role>:<mode>`) for this node.
+Examples: `-Dsolr.node.roles=data:on,overseer:allowed` or `-Dsolr.node.roles=overseer:preferred`
+|No
+|`data:on,overseer:allowed`
+|===
+
+[TIP]
+====
+If a node has been started with no `solr.node.roles` parameter, it will be assumed to have the data role turned on and overseer role allowed on it. If you've never used roles before, you likely won't need to change anything in your startup parameters to accomodate the functionality associated with these roles.
+====
+
+.Supported roles
+[cols="1,1"] 
+|===
+|Role |Modes
+
+|`data`
+|on, off
+
+|`overseer`
+|allowed, preferred, disallowed
+|===
+
+=== `data` role
+A node with this role (in mode "on") can host shards and replicas for collections.
+
+=== `overseer` role
+A node with this role can perform duties of an overseer node (unless mode is `disallowed`). When one or more nodes have the overseer role in `preferred` mode, the overseer leader will be elected from one of these nodes. In case no node is designated as a preferred overseer or no such node is live, the overseer leader will be elected from one of the nodes that have the overseer role in `allowed` mode. If all nodes that are designated with overseer role (allowed or preferred) are down, the cluster will be left without an overseer.
+
+== Example usage
+
+Sometimes, when the nodes in a cluster are under heavy querying or indexing load, the overseer leader node might be unable to perform collection management duties efficiently. It might be reasonable to have dedicated nodes to act as the overseer. Such an effect can be achieved as follows:
+
+* Most nodes (data nodes) in the cluster start with `-Dsolr.node.roles=data:on,overseer:allowed` (or with no parameter, since the default value for `solr.node.roles` is the same).

Review comment:
       If you accept that the default should be per role (and I hope you do, it doesn't change cluster behavior with this PR but simplifies life for later), this sentence has to be rephrased (see comment line 41 of `NodeRoles`)

##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,198 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.
+
+== Definitions
+
+=== Node role
+
+A role is a designation of a node that indicates that the node may perform a certain functionality that is governed by the role.
+
+=== Mode
+Every role has a list of modes under which a node can be. It can be simple (e.g. `["on", "off"]`) or more granular (e.g. `["allowed", "preferred", "disallowed"]`).
+
+== Roles
+
+In order to specify role(s) for a node, one needs to start a Solr node with the following parameter.
+
+.Startup Parameter
+[cols="1,2,1,1"] 
+|===
+|Parameter |Value |Required? | Default
+
+|solr.node.roles
+|Comma separated list of roles (in the format: `<role>:<mode>`) for this node.
+Examples: `-Dsolr.node.roles=data:on,overseer:allowed` or `-Dsolr.node.roles=overseer:preferred`
+|No
+|`data:on,overseer:allowed`
+|===
+
+[TIP]
+====
+If a node has been started with no `solr.node.roles` parameter, it will be assumed to have the data role turned on and overseer role allowed on it. If you've never used roles before, you likely won't need to change anything in your startup parameters to accomodate the functionality associated with these roles.
+====
+
+.Supported roles

Review comment:
       Table should indicate default value for each role.

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {

Review comment:
       Why is `NodeRoles` a `MapWriter`?

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {
+    String roleMode = nodeRoles.get(Role.OVERSEER);
+    return ALLOWED.equals(roleMode) || PREFERRED.equals(roleMode);
+  }
+
+  public enum Role {
+    DATA("data"),
+    OVERSEER("overseer") {
+      @Override
+      public Set<String> supportedModes() {
+        return OVERSEER_MODES;
+      }
+      @Override
+      public String defaultIfAbsent() {
+        return DISALLOWED;
+      }
+    };
+
+    public final String roleName;
+
+    Role(String name) {
+      this.roleName = name;
+    }
+
+    public static Role getRole(String value) {
+      for (Role role: Role.values()) {
+        if (value.equals(role.roleName)) return role;
+      }
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role: " + value);
+    }
+
+    public Set<String> supportedModes() {
+      return ON_OFF;
+    }
+
+    /**
+     * Default mode for a role in nodes where this role is not specified.
+     */
+    public String defaultIfAbsent() {

Review comment:
       I would make `abstract` this method as well as `supportedModes()` above, so each `Role` has to define them in the enum. Less error prone and easier to understand the logic of each role.
   
   I'd also rename this one into `defaultMode()`. "If absent" is redundant since we use the default only if the role is absent (i.e. not specified).

##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
##########
@@ -243,12 +247,24 @@ private static boolean existCoreName(String coreName, Slice slice) {
       }
     } else {
       nodeList = new ArrayList<>(liveNodes);
+      filterNonDataNodes(zk, nodeList);
       Collections.shuffle(nodeList, random);
     }
 
     return nodeList;
   }
 
+  public static void filterNonDataNodes(DistribStateManager zk, List<String> liveNodes) {

Review comment:
       This method should be moved to class `NodeRoles` (or another one if we want to separate single node role checks and multi node role checks) and be implemented in a more generic way (passing the role and passing the role value to filter on).

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {
+    String roleMode = nodeRoles.get(Role.OVERSEER);
+    return ALLOWED.equals(roleMode) || PREFERRED.equals(roleMode);
+  }
+
+  public enum Role {
+    DATA("data"),
+    OVERSEER("overseer") {
+      @Override
+      public Set<String> supportedModes() {
+        return OVERSEER_MODES;

Review comment:
       There should be no constant `OVERSEER_MODES` and the only way to get to these values should be by doing `Role.OVERSEER.supportedModes()`.
   
   This line should therefore be: `return Set.of(ALLOWED, PREFERRED, DISALLOWED);`

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {
+    String roleMode = nodeRoles.get(Role.OVERSEER);
+    return ALLOWED.equals(roleMode) || PREFERRED.equals(roleMode);
+  }
+
+  public enum Role {
+    DATA("data"),
+    OVERSEER("overseer") {
+      @Override
+      public Set<String> supportedModes() {
+        return OVERSEER_MODES;
+      }
+      @Override
+      public String defaultIfAbsent() {
+        return DISALLOWED;
+      }
+    };
+
+    public final String roleName;
+
+    Role(String name) {
+      this.roleName = name;
+    }
+
+    public static Role getRole(String value) {
+      for (Role role: Role.values()) {

Review comment:
       I'd simplify and make the enum name be used as the role name, by using Role.valueOf(value.toUpperCase()).

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +64,194 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    Map<String, Object> result = new LinkedHashMap<>();
+
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result = null;

Review comment:
       initialization not needed here.

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +64,194 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    Map<String, Object> result = new LinkedHashMap<>();
+
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result = null;
+    try {
+      List<String> children = zk.listData(path);
+      if (children != null && !children.isEmpty()) {
+        result = new HashMap<>();
+      } else {
+        return depth >= 1 ? Collections.emptyList() : null;
+      }
+      for (String child : children) {
+        Object c = readRecursive(path + "/" + child, zk, depth - 1);
+        result.put(child, c);
+      }
+    } catch (Exception e) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+    }
+    if (depth == 1) {
+      return result.keySet();
+    } else {
+      return result;
+    }
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}",
+          permission = COLL_READ_PERM)
+  public void nodesWithRole(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String role = req.getPathTemplateValues().get("role");
+    rsp.add("node-roles", Map.of(role,
+            readRecursive(ZkStateReader.NODE_ROLES + "/"+ role,
+                    collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 2)));
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/node/{node}",
+          permission = COLL_READ_PERM)
+  @SuppressWarnings("unchecked")
+  public void rolesForNode(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String node = req.getPathTemplateValues().get("node");
+    Map<String, String> ret = new HashMap<String, String>();
+    Map<String, Map<String, Set<String>>> roles = (Map<String, Map<String, Set<String>>>) readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3);
+    for (String role: roles.keySet()) {
+      for (String mode : roles.get(role).keySet()) {
+        if (roles.get(role).get(mode) instanceof List && ((List) roles.get(role).get(mode)).size() == 0) {

Review comment:
       How can this ever be a `List`?

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +64,194 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    Map<String, Object> result = new LinkedHashMap<>();

Review comment:
       unused variable

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +64,194 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    Map<String, Object> result = new LinkedHashMap<>();
+
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result = null;
+    try {
+      List<String> children = zk.listData(path);
+      if (children != null && !children.isEmpty()) {
+        result = new HashMap<>();
+      } else {
+        return depth >= 1 ? Collections.emptyList() : null;
+      }
+      for (String child : children) {
+        Object c = readRecursive(path + "/" + child, zk, depth - 1);
+        result.put(child, c);
+      }
+    } catch (Exception e) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+    }
+    if (depth == 1) {
+      return result.keySet();
+    } else {
+      return result;
+    }
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}",
+          permission = COLL_READ_PERM)
+  public void nodesWithRole(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String role = req.getPathTemplateValues().get("role");
+    rsp.add("node-roles", Map.of(role,
+            readRecursive(ZkStateReader.NODE_ROLES + "/"+ role,
+                    collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 2)));
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/node/{node}",
+          permission = COLL_READ_PERM)
+  @SuppressWarnings("unchecked")
+  public void rolesForNode(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String node = req.getPathTemplateValues().get("node");
+    Map<String, String> ret = new HashMap<String, String>();
+    Map<String, Map<String, Set<String>>> roles = (Map<String, Map<String, Set<String>>>) readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3);
+    for (String role: roles.keySet()) {

Review comment:
       Seems a bit wasteful to read all the info from ZK to then throw away most of it and only retain the data pertaining to a single node. We could add some basic filtering logic in `readRecursive` to limit to a given node.

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";

Review comment:
       Code would be more structured if node roles are defined in a subclass of `NodeRoles`.
   So a role is for example `NodeRoles.Role.OVERSEER` and a mode is `NodeRoles.Mode.PREFERRED`.

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +64,194 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    Map<String, Object> result = new LinkedHashMap<>();
+
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result = null;
+    try {
+      List<String> children = zk.listData(path);
+      if (children != null && !children.isEmpty()) {
+        result = new HashMap<>();
+      } else {
+        return depth >= 1 ? Collections.emptyList() : null;
+      }
+      for (String child : children) {
+        Object c = readRecursive(path + "/" + child, zk, depth - 1);
+        result.put(child, c);
+      }
+    } catch (Exception e) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+    }
+    if (depth == 1) {
+      return result.keySet();
+    } else {
+      return result;
+    }
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}",
+          permission = COLL_READ_PERM)
+  public void nodesWithRole(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String role = req.getPathTemplateValues().get("role");
+    rsp.add("node-roles", Map.of(role,
+            readRecursive(ZkStateReader.NODE_ROLES + "/"+ role,
+                    collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 2)));
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/node/{node}",
+          permission = COLL_READ_PERM)
+  @SuppressWarnings("unchecked")
+  public void rolesForNode(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String node = req.getPathTemplateValues().get("node");
+    Map<String, String> ret = new HashMap<String, String>();
+    Map<String, Map<String, Set<String>>> roles = (Map<String, Map<String, Set<String>>>) readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3);
+    for (String role: roles.keySet()) {
+      for (String mode : roles.get(role).keySet()) {
+        if (roles.get(role).get(mode) instanceof List && ((List) roles.get(role).get(mode)).size() == 0) {
+          continue;
+        }
+        Set<String> nodes = roles.get(role).get(mode);
+        if (nodes.contains(node)) ret.put(role, mode);
+      }
+    }
+    for (String role: ret.keySet()) {
+      rsp.add(role, ret.get(role));
+    }
+  }
+
   @EndPoint(method = GET,
-      path = "/cluster/aliases",
-      permission = COLL_READ_PERM)
+          path = "/cluster/node-roles/supported",
+          permission = COLL_READ_PERM)
+  public void supportedRoles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+     Map<String, Object> roleModesSupportedMap = new HashMap<>();
+    for (NodeRoles.Role role: NodeRoles.Role.values()) {
+      roleModesSupportedMap.put(role.toString(), Map.of("modes", role.supportedModes(), "defaultIfAbsent", role.defaultIfAbsent()));
+    }
+    rsp.add("supported-roles", roleModesSupportedMap);
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}/{role-val}",
+          permission = COLL_READ_PERM)
+  public void nodesWithRoleVal(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String role = req.getPathTemplateValues().get("role");
+    String roleVal = req.getPathTemplateValues().get("role-val");
+    List<String> nodes =  collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager()
+            .getDistribStateManager().listData(ZkStateReader.NODE_ROLES + "/"+ role+"/"+roleVal);

Review comment:
       Formatting: please leave a space on each side of the `+` sign.

##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {

Review comment:
       Name is misleading given `ALLOWED` is a mode. Method should rather be called `isOverseerDisallowed()` for example (and callers changed to add negation), or `isOverseerAllowedOrPreferred()` etc.

##########
File path: solr/core/src/java/org/apache/solr/handler/ClusterAPI.java
##########
@@ -57,92 +64,194 @@
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_EDIT_PERM;
 import static org.apache.solr.security.PermissionNameProvider.Name.CONFIG_READ_PERM;
 
-/** All V2 APIs that have  a prefix of /api/cluster/
- *
+/**
+ * All V2 APIs that have  a prefix of /api/cluster/
  */
 public class ClusterAPI {
   private final CollectionsHandler collectionsHandler;
   private final ConfigSetsHandler configSetsHandler;
 
-  public  final Commands commands = new Commands();
-  public  final ConfigSetCommands configSetCommands = new ConfigSetCommands();
+  public final Commands commands = new Commands();
+  public final ConfigSetCommands configSetCommands = new ConfigSetCommands();
 
   public ClusterAPI(CollectionsHandler ch, ConfigSetsHandler configSetsHandler) {
     this.collectionsHandler = ch;
     this.configSetsHandler = configSetsHandler;
   }
 
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles",
+          permission = COLL_READ_PERM)
+  public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    Map<String, Object> result = new LinkedHashMap<>();
+
+    rsp.add("node-roles", readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3));
+  }
+
+  Object readRecursive(String path, DistribStateManager zk, int depth) {
+    if (depth == 0) return null;
+    Map<String, Object> result = null;
+    try {
+      List<String> children = zk.listData(path);
+      if (children != null && !children.isEmpty()) {
+        result = new HashMap<>();
+      } else {
+        return depth >= 1 ? Collections.emptyList() : null;
+      }
+      for (String child : children) {
+        Object c = readRecursive(path + "/" + child, zk, depth - 1);
+        result.put(child, c);
+      }
+    } catch (Exception e) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
+    }
+    if (depth == 1) {
+      return result.keySet();
+    } else {
+      return result;
+    }
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/role/{role}",
+          permission = COLL_READ_PERM)
+  public void nodesWithRole(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String role = req.getPathTemplateValues().get("role");
+    rsp.add("node-roles", Map.of(role,
+            readRecursive(ZkStateReader.NODE_ROLES + "/"+ role,
+                    collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 2)));
+  }
+
+  @EndPoint(method = GET,
+          path = "/cluster/node-roles/node/{node}",
+          permission = COLL_READ_PERM)
+  @SuppressWarnings("unchecked")
+  public void rolesForNode(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+    String node = req.getPathTemplateValues().get("node");
+    Map<String, String> ret = new HashMap<String, String>();
+    Map<String, Map<String, Set<String>>> roles = (Map<String, Map<String, Set<String>>>) readRecursive(ZkStateReader.NODE_ROLES,
+            collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(), 3);
+    for (String role: roles.keySet()) {
+      for (String mode : roles.get(role).keySet()) {
+        if (roles.get(role).get(mode) instanceof List && ((List) roles.get(role).get(mode)).size() == 0) {
+          continue;
+        }
+        Set<String> nodes = roles.get(role).get(mode);
+        if (nodes.contains(node)) ret.put(role, mode);
+      }
+    }
+    for (String role: ret.keySet()) {
+      rsp.add(role, ret.get(role));
+    }
+  }
+
   @EndPoint(method = GET,
-      path = "/cluster/aliases",
-      permission = COLL_READ_PERM)
+          path = "/cluster/node-roles/supported",
+          permission = COLL_READ_PERM)
+  public void supportedRoles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception {
+     Map<String, Object> roleModesSupportedMap = new HashMap<>();
+    for (NodeRoles.Role role: NodeRoles.Role.values()) {
+      roleModesSupportedMap.put(role.toString(), Map.of("modes", role.supportedModes(), "defaultIfAbsent", role.defaultIfAbsent()));

Review comment:
       This endpoint is another reason to remove `NodeRoles.DEFAULT_ROLES_STRING`. It might be confusing to return these values from an API call yet the default ends up being different when no roles are defined.

##########
File path: solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
##########
@@ -103,6 +103,7 @@
   public static final String STATE_TIMESTAMP_PROP = "stateTimestamp";
   public static final String COLLECTIONS_ZKNODE = "/collections";
   public static final String LIVE_NODES_ZKNODE = "/live_nodes";
+  public static final String NODE_ROLES = "/node_roles";

Review comment:
       A comment here should explain the difference between `/node_roles` and `/roles.json` defined further down.

##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##########
@@ -357,7 +358,9 @@ public void command() throws SessionExpiredException {
 
                 overseerElector.setup(context);
 
-                overseerElector.joinElection(context, true);
+                if(cc.nodeRoles.isOverseerAllowed()) {

Review comment:
       formatting: please add space after `if`.

##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##########
@@ -1086,6 +1099,11 @@ private void createEphemeralLiveNode() throws KeeperException,
     log.info("Register node as live in ZooKeeper:{}", nodePath);
     List<Op> ops = new ArrayList<>(2);
     ops.add(Op.create(nodePath, null, zkClient.getZkACLProvider().getACLsToAdd(nodePath), CreateMode.EPHEMERAL));
+
+    // Create the roles node as well
+   cc.nodeRoles.getRoles().forEach((role, val) -> ops.add(Op.create(ZkStateReader.NODE_ROLES + "/" + role.roleName+ "/"+val +"/"+ nodeName,

Review comment:
       Another example of ZK path assembly that would benefit from a method building that path (see comment line 862)

##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##########
@@ -854,6 +857,14 @@ public static void createClusterZkNodes(SolrZkClient zkClient)
       throws KeeperException, InterruptedException, IOException {
     ZkCmdExecutor cmdExecutor = new ZkCmdExecutor(zkClient.getZkClientTimeout());
     cmdExecutor.ensureExists(ZkStateReader.LIVE_NODES_ZKNODE, zkClient);
+    cmdExecutor.ensureExists(ZkStateReader.NODE_ROLES, zkClient);
+    for (NodeRoles.Role role : NodeRoles.Role.values()) {
+      cmdExecutor.ensureExists(ZkStateReader.NODE_ROLES + "/" + role.roleName, zkClient);

Review comment:
       The various paths `ZkStateReader.NODE_ROLES + "/" + role.roleName` and `ZkStateReader.NODE_ROLES + "/" + role.roleName + "/"+v` are used here and in `ClusterAPI`. It would be better to define a method building them so the actual string assembly has to be defined in only once.

##########
File path: solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java
##########
@@ -0,0 +1,112 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.solr.cloud;
+
+import java.lang.invoke.MethodHandles;
+import java.util.Collection;
+import java.util.Collections;
+
+import java.util.List;
+import java.util.Map;
+
+import org.apache.solr.client.solrj.embedded.JettySolrRunner;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.V2Request;
+import org.apache.solr.client.solrj.response.V2Response;
+import org.apache.solr.core.NodeRoles;
+import org.junit.After;
+import org.junit.Before;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRolesTest extends SolrCloudTestCase {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  @Before
+  public void setupCluster() throws Exception {
+    configureCluster(1)
+            .addConfig("conf", configset("cloud-minimal"))
+            .configure();
+  }
+
+  @After
+  public void tearDownCluster() throws Exception {
+    shutdownCluster();
+  }
+
+  @SuppressWarnings("unchecked")
+  public void testRoleIntegration() throws Exception {
+    JettySolrRunner j0 = cluster.getJettySolrRunner(0);
+    JettySolrRunner j1 = null, j2 = null;

Review comment:
       setting to `null` is not needed.

##########
File path: solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java
##########
@@ -243,12 +247,24 @@ private static boolean existCoreName(String coreName, Slice slice) {
       }
     } else {
       nodeList = new ArrayList<>(liveNodes);
+      filterNonDataNodes(zk, nodeList);

Review comment:
       In the if branch above (when a node set is passed), the set should likely be filtered as well, and warnings issued if some requested nodes have data role configured to off.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] murblanc commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
murblanc commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r773779415



##########
File path: solr/core/src/java/org/apache/solr/core/CoreContainer.java
##########
@@ -944,6 +946,14 @@ public void load() {
       });
 
       clusterSingletons.setReady();
+      if (NodeRoles.PREFERRED.equals(nodeRoles.getRoleMode(NodeRoles.Role.OVERSEER))) {
+        try {
+          log.info("This node is started as a preferred overseer");
+          zkSys.getZkController().setPreferredOverseer();
+        } catch (KeeperException | InterruptedException e) {
+          throw new SolrException(ErrorCode.SERVER_ERROR, e);
+        }
+      }
       if (!distributedCollectionCommandRunner.isPresent()) {

Review comment:
       You can't do Overseer related actions if Overseer is not used in the cluster that is configured for distributed cluster state change and collection API.
   
   So the code should instead look like:
   ```
   if (!distributedCollectionCommandRunner.isPresent()) {
        if (NodeRoles.PREFERRED.equals(nodeRoles.getRoleMode(NodeRoles.Role.OVERSEER))) {
           try {
              log.info("This node is started as a preferred overseer");
              zkSys.getZkController().setPreferredOverseer();
           } catch (KeeperException | InterruptedException e) {
              throw new SolrException(ErrorCode.SERVER_ERROR, e);
           }
        }
        zkSys.getZkController().checkOverseerDesignate();
   }
   ```




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] noblepaul commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
noblepaul commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r773839765



##########
File path: solr/core/src/java/org/apache/solr/core/NodeRoles.java
##########
@@ -0,0 +1,129 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.core;
+
+import java.lang.invoke.MethodHandles;
+import java.util.*;
+import org.apache.solr.common.MapWriter;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.StringUtils;
+import org.apache.solr.common.util.StrUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class NodeRoles implements MapWriter {
+  private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+  public static final String NODE_ROLES_PROP = "solr.node.roles";
+
+  public static final String ON =  "on";
+  public static final String OFF =  "off";
+  public static final String ALLOWED =  "allowed";
+  public static final String DISALLOWED =  "disallowed";
+  public static final String PREFERRED =  "preferred";
+  public static final Set<String> OVERSEER_MODES = Set.of(ALLOWED, PREFERRED, DISALLOWED);
+  public static final Set<String> ON_OFF = Set.of(ON,OFF);
+
+  public static final String DEFAULT_ROLES_STRING = "data:on,overseer:allowed";
+
+  // Map of roles to mode that are applicable for this node.
+  private Map<Role, String> nodeRoles;
+
+  public NodeRoles(String rolesString) {
+    Map<Role, String> roles = new EnumMap<>(Role.class);
+    if (StringUtils.isEmpty(rolesString)) {
+     rolesString = DEFAULT_ROLES_STRING;
+    }
+    List<String> rolesList = StrUtils.splitSmart(rolesString, ',');
+    for (String s: rolesList) {
+      List<String> roleMode =  StrUtils.splitSmart(s,':');
+      Role r = Role.getRole(roleMode.get(0));
+      if (r.supportedModes().contains(roleMode.get(1))) {
+        roles.put(r, roleMode.get(1));
+      } else {
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role mode: " + roleMode.get(0));
+      }
+    }
+    for(Role r: Role.values()) {
+      if (!roles.containsKey(r)) {
+        roles.put(r, r.defaultIfAbsent());
+      }
+    }
+    nodeRoles = Collections.unmodifiableMap(roles);
+  }
+
+  public Map<Role, String> getRoles() {
+    return nodeRoles;
+  }
+
+  public String getRoleMode(Role role) {
+    return nodeRoles.get(role);
+  }
+
+  @Override
+  public void writeMap(EntryWriter ew) {
+    nodeRoles.forEach((role, s) -> ew.putNoEx(role.roleName, s));
+  }
+
+  public boolean isOverseerAllowed() {
+    String roleMode = nodeRoles.get(Role.OVERSEER);
+    return ALLOWED.equals(roleMode) || PREFERRED.equals(roleMode);
+  }
+
+  public enum Role {
+    DATA("data"),
+    OVERSEER("overseer") {
+      @Override
+      public Set<String> supportedModes() {
+        return OVERSEER_MODES;
+      }
+      @Override
+      public String defaultIfAbsent() {
+        return DISALLOWED;
+      }
+    };
+
+    public final String roleName;
+
+    Role(String name) {
+      this.roleName = name;
+    }
+
+    public static Role getRole(String value) {
+      for (Role role: Role.values()) {

Review comment:
       toUpperCase() creates an extra string always and it is unnecessary. Read operations should avoid object creation. Also people may start using arbitrary combination of cases such as ON, On, on etc. We should standardize on one string




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] janhoy commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
janhoy commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r777540508



##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,198 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.
+
+== Definitions
+
+=== Node role
+
+A role is a designation of a node that indicates that the node may perform a certain functionality that is governed by the role.
+
+=== Mode
+Every role has a list of modes under which a node can be. It can be simple (e.g. `["on", "off"]`) or more granular (e.g. `["allowed", "preferred", "disallowed"]`).
+
+== Roles
+
+In order to specify role(s) for a node, one needs to start a Solr node with the following parameter.
+
+.Startup Parameter
+[cols="1,2,1,1"] 
+|===
+|Parameter |Value |Required? | Default
+
+|solr.node.roles
+|Comma separated list of roles (in the format: `<role>:<mode>`) for this node.
+Examples: `-Dsolr.node.roles=data:on,overseer:allowed` or `-Dsolr.node.roles=overseer:preferred`
+|No
+|`data:on,overseer:allowed`
+|===
+
+[TIP]
+====
+If a node has been started with no `solr.node.roles` parameter, it will be assumed to have the data role turned on and overseer role allowed on it. If you've never used roles before, you likely won't need to change anything in your startup parameters to accomodate the functionality associated with these roles.
+====
+
+.Supported roles
+[cols="1,1"] 
+|===
+|Role |Modes
+
+|`data`
+|on, off
+
+|`overseer`
+|allowed, preferred, disallowed
+|===
+
+=== `data` role
+A node with this role (in mode "on") can host shards and replicas for collections.
+
+=== `overseer` role
+A node with this role can perform duties of an overseer node (unless mode is `disallowed`). When one or more nodes have the overseer role in `preferred` mode, the overseer leader will be elected from one of these nodes. In case no node is designated as a preferred overseer or no such node is live, the overseer leader will be elected from one of the nodes that have the overseer role in `allowed` mode. If all nodes that are designated with overseer role (allowed or preferred) are down, the cluster will be left without an overseer.
+
+== Example usage
+
+Sometimes, when the nodes in a cluster are under heavy querying or indexing load, the overseer leader node might be unable to perform collection management duties efficiently. It might be reasonable to have dedicated nodes to act as the overseer. Such an effect can be achieved as follows:
+
+* Most nodes (data nodes) in the cluster start with `-Dsolr.node.roles=data:on,overseer:allowed` (or with no parameter, since the default value for `solr.node.roles` is the same).

Review comment:
       Good points @murblanc. Especially I think this statement is true:
   
   > One can assume that a Solr deployment where solr.node.roles is overridden on each node (an advanced or "expert" cluster config) would be careful on upgrades and will not fall into the "ui" issue you mention.
   
   This seems to blow up in unnecessary configuration complexity, and optimizing for the needs of 5% of users, rather than 95%.
   
   So why not start with an explicit WYSIWYG and KISS approach, and then in 9.x refine based on real-life upgrade needs? All these config facets and various defaults is a recipe for future bugs and confusion.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r779826543



##########
File path: solr/solr-ref-guide/src/node-roles.adoc
##########
@@ -0,0 +1,196 @@
+= Node Roles
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+A node in Solr is usually capable of performing various types of operations, e.g. hosting replicas, performing indexing and querying on them, collection management tasks etc. However, if someone wants to setup a cluster where these functionalities are isolated to certain dedicated nodes, then this can be achieved leveraging the concept of node roles.

Review comment:
       Fixed, thanks.




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org


[GitHub] [solr] chatman commented on a change in pull request #403: SOLR-15694 Concept of node roles and non-data nodes

Posted by GitBox <gi...@apache.org>.
chatman commented on a change in pull request #403:
URL: https://github.com/apache/solr/pull/403#discussion_r779854562



##########
File path: solr/core/src/java/org/apache/solr/cloud/ZkController.java
##########
@@ -913,7 +924,9 @@ private void init() {
         ElectionContext context = new OverseerElectionContext(zkClient,
             overseer, getNodeName());
         overseerElector.setup(context);
-        overseerElector.joinElection(context, false);
+        if (cc.nodeRoles.isOverseerAllowedOrPreferred()) {

Review comment:
       Tried doing so, but ran into test failures. Maybe @noblepaul might know why this doesn't work?




-- 
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-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org