This is an automated email from the ASF dual-hosted git repository. ishan pushed a commit to branch jira/SOLR15694 in repository https://gitbox.apache.org/repos/asf/solr.git
commit 0ac0b3279ec9e9d9038e1a5dc3e68183071d93e6 Author: Ishan Chattopadhyaya <[email protected]> AuthorDate: Fri Jan 7 01:01:39 2022 +0530 Review feedback --- .../apache/solr/cloud/OverseerNodePrioritizer.java | 2 +- .../java/org/apache/solr/cloud/ZkController.java | 4 +- .../apache/solr/cloud/api/collections/Assign.java | 2 +- .../src/java/org/apache/solr/cluster/Cluster.java | 6 ++- .../placement/impl/PlacementRequestImpl.java | 2 +- .../impl/SimpleClusterAbstractionsImpl.java | 6 +-- .../java/org/apache/solr/core/CoreContainer.java | 2 +- .../src/java/org/apache/solr/core/NodeRoles.java | 53 ++++++++++------------ .../java/org/apache/solr/handler/ClusterAPI.java | 6 +-- .../test/org/apache/solr/cloud/NodeRolesTest.java | 1 - .../placement/ClusterAbstractionsForTest.java | 2 +- solr/solr-ref-guide/src/node-roles.adoc | 4 +- .../apache/solr/common/cloud/ZkStateReader.java | 2 +- 13 files changed, 43 insertions(+), 49 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerNodePrioritizer.java b/solr/core/src/java/org/apache/solr/cloud/OverseerNodePrioritizer.java index 5e6c7a7..2a02562 100644 --- a/solr/core/src/java/org/apache/solr/cloud/OverseerNodePrioritizer.java +++ b/solr/core/src/java/org/apache/solr/cloud/OverseerNodePrioritizer.java @@ -73,7 +73,7 @@ public class OverseerNodePrioritizer { } } - List<String> preferredOverseers = ClusterAPI.getNodesByRole(NodeRoles.Role.OVERSEER, NodeRoles.Mode.PREFERRED, + List<String> preferredOverseers = ClusterAPI.getNodesByRole(NodeRoles.Role.OVERSEER, NodeRoles.MODE_PREFERRED, new ZkDistribStateManager(zkStateReader.getZkClient())); for (String preferred: preferredOverseers) { if (overseerDesignates.contains(preferred)) { diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java index 8688bfd..197d136 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java @@ -860,7 +860,7 @@ public class ZkController implements Closeable { cmdExecutor.ensureExists(ZkStateReader.NODE_ROLES, zkClient); for (NodeRoles.Role role: NodeRoles.Role.values()) { cmdExecutor.ensureExists(NodeRoles.getZNodeForRole(role), zkClient); - for (NodeRoles.Mode mode: role.supportedModes()) { + for (String mode: role.supportedModes()) { cmdExecutor.ensureExists(NodeRoles.getZNodeForRoleMode(role, mode), zkClient); } } @@ -2230,7 +2230,7 @@ public class ZkController implements Closeable { "node", getNodeName(), "role", "overseer", "persist", "false"); - log.info("Going to add role {} ", props); + log.warn("Going to add role {}. It is deprecated to use ADDROLE and consider using Node Roles instead.", props); getOverseerCollectionQueue().offer(Utils.toJSON(props)); } diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java index 88271a1..e1d145d 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/Assign.java @@ -244,7 +244,7 @@ public class Assign { public static Collection<String> filterNonDataNodes(DistribStateManager zk, Collection<String> liveNodes) { try { - List<String> noData = ClusterAPI.getNodesByRole(NodeRoles.Role.DATA, NodeRoles.Mode.OFF, zk); + List<String> noData = ClusterAPI.getNodesByRole(NodeRoles.Role.DATA, NodeRoles.MODE_OFF, zk); if (noData.isEmpty()) { return liveNodes; } else { diff --git a/solr/core/src/java/org/apache/solr/cluster/Cluster.java b/solr/core/src/java/org/apache/solr/cluster/Cluster.java index 4cd32a4..b3b210f 100644 --- a/solr/core/src/java/org/apache/solr/cluster/Cluster.java +++ b/solr/core/src/java/org/apache/solr/cluster/Cluster.java @@ -31,7 +31,11 @@ public interface Cluster { */ Set<Node> getLiveNodes(); - Set<Node> getLiveNodes(boolean filterNonDataNodes) ; + /** + * @return current set of live nodes that are supposed to host data. + */ + Set<Node> getLiveDataNodes(); + /** * Returns info about the given collection if one exists. * diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementRequestImpl.java b/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementRequestImpl.java index 3814eb6..5a93720 100644 --- a/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementRequestImpl.java +++ b/solr/core/src/java/org/apache/solr/cluster/placement/impl/PlacementRequestImpl.java @@ -86,7 +86,7 @@ public class PlacementRequestImpl implements PlacementRequest { throw new Assign.AssignmentException("Bad assign request: empty list of nodes for collection " + solrCollection.getName()); } } else { - nodes = cluster.getLiveNodes(true); + nodes = cluster.getLiveDataNodes(); if (nodes.isEmpty()) { throw new Assign.AssignmentException("Impossible assign request: no live nodes for collection " + solrCollection.getName()); } diff --git a/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java b/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java index 64c0b64..e5d99b2 100644 --- a/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java +++ b/solr/core/src/java/org/apache/solr/cluster/placement/impl/SimpleClusterAbstractionsImpl.java @@ -72,10 +72,8 @@ class SimpleClusterAbstractionsImpl { return liveNodes; } - public Set<Node> getLiveNodes(boolean filterNonDataNodes) { - return filterNonDataNodes ? - liveNodesWithData : - liveNodes; + public Set<Node> getLiveDataNodes() { + return liveNodesWithData; } @Override diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java index e272767..b915330 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -947,7 +947,7 @@ public class CoreContainer { }); clusterSingletons.setReady(); - if (NodeRoles.Mode.PREFERRED.equals(nodeRoles.getRoleMode(NodeRoles.Role.OVERSEER))) { + if (NodeRoles.MODE_PREFERRED.equals(nodeRoles.getRoleMode(NodeRoles.Role.OVERSEER))) { try { log.info("This node has been started as a preferred overseer"); zkSys.getZkController().setPreferredOverseer(); diff --git a/solr/core/src/java/org/apache/solr/core/NodeRoles.java b/solr/core/src/java/org/apache/solr/core/NodeRoles.java index 16059db..bdaeb79 100644 --- a/solr/core/src/java/org/apache/solr/core/NodeRoles.java +++ b/solr/core/src/java/org/apache/solr/core/NodeRoles.java @@ -34,10 +34,10 @@ public class NodeRoles { 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; + private Map<Role, String> nodeRoles; public NodeRoles(String rolesString) { - Map<Role, Mode> roles = new EnumMap<>(Role.class); + Map<Role, String> roles = new EnumMap<>(Role.class); if (StringUtils.isEmpty(rolesString)) { rolesString = DEFAULT_ROLES_STRING; } @@ -45,7 +45,7 @@ public class NodeRoles { 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)); + String m = roleMode.get(1); if (r.supportedModes().contains(m)) { roles.put(r, m); } else { @@ -61,51 +61,44 @@ public class NodeRoles { nodeRoles = Collections.unmodifiableMap(roles); } - public Map<Role, Mode> getRoles() { + public Map<Role, String> getRoles() { return nodeRoles; } - public Mode getRoleMode(Role role) { + public String 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); + String roleMode = nodeRoles.get(Role.OVERSEER); + return MODE_ALLOWED.equals(roleMode) || MODE_PREFERRED.equals(roleMode); } - public enum Mode { - ON, OFF, ALLOWED, PREFERRED, DISALLOWED; - - /** - * Need this lowercasing so that the ZK references use the lowercase form, which is - * also the form documented in user facing documentation. - */ - @Override - public String toString() { - return name().toLowerCase(Locale.ROOT); - } - }; + public final static String MODE_ON = "on"; + public final static String MODE_OFF = "off"; + public final static String MODE_ALLOWED = "allowed"; + public final static String MODE_PREFERRED = "preferred"; + public final static String MODE_DISALLOWED = "disallowed"; public enum Role { DATA("data") { @Override - public Set<Mode> supportedModes() { - return Set.of(Mode.ON, Mode.OFF); + public Set<String> supportedModes() { + return Set.of(MODE_ON, MODE_OFF); } @Override - public Mode modeWhenRoleIsAbsent() { - return Mode.OFF; + public String modeWhenRoleIsAbsent() { + return MODE_OFF; } }, OVERSEER("overseer") { @Override - public Set<Mode> supportedModes() { - return Set.of(Mode.ALLOWED, Mode.PREFERRED, Mode.DISALLOWED); + public Set<String> supportedModes() { + return Set.of(MODE_ALLOWED, MODE_PREFERRED, MODE_DISALLOWED); } @Override - public Mode modeWhenRoleIsAbsent() { - return Mode.DISALLOWED; + public String modeWhenRoleIsAbsent() { + return MODE_DISALLOWED; } }; @@ -122,12 +115,12 @@ public class NodeRoles { throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown role: " + value); } - public abstract Set<Mode> supportedModes(); + public abstract Set<String> supportedModes(); /** * Default mode for a role in nodes where this role is not specified. */ - public abstract Mode modeWhenRoleIsAbsent(); + public abstract String modeWhenRoleIsAbsent(); @Override public String toString() { @@ -139,7 +132,7 @@ public class NodeRoles { return ZkStateReader.NODE_ROLES + "/" + role.roleName; } - public static String getZNodeForRoleMode(Role role, Mode mode) { + public static String getZNodeForRoleMode(Role role, String mode) { return ZkStateReader.NODE_ROLES + "/" + role.roleName + "/" + mode; } diff --git a/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java b/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java index 3e9f0b0..4b946aa 100644 --- a/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java +++ b/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java @@ -96,7 +96,7 @@ public class ClusterAPI { if (children != null && !children.isEmpty()) { result = new HashMap<>(); } else { - return depth >= 1 ? Collections.emptySet(): null; + return Collections.emptySet(); } for (String child: children) { Object c = readRecursive(path + "/" + child, zk, depth - 1); @@ -150,7 +150,7 @@ public class ClusterAPI { 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()))); + Map.of("modes", role.supportedModes())); } rsp.add("supported-roles", roleModesSupportedMap); } @@ -169,7 +169,7 @@ public class ClusterAPI { rsp.add( "node-roles", Map.of(roleStr, Collections.singletonMap(modeStr, nodes))); } - public static List<String> getNodesByRole(NodeRoles.Role role, NodeRoles.Mode mode, DistribStateManager zk) + public static List<String> getNodesByRole(NodeRoles.Role role, String mode, DistribStateManager zk) throws InterruptedException, IOException, KeeperException { try { return zk.listData(ZkStateReader.NODE_ROLES + "/" + role + "/" + mode); diff --git a/solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java b/solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java index fb05e34..1a573d6 100644 --- a/solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java @@ -48,7 +48,6 @@ public class NodeRolesTest extends SolrCloudTestCase { shutdownCluster(); } - @SuppressWarnings("rawtypes") public void testRoleIntegration() throws Exception { JettySolrRunner j0 = cluster.getJettySolrRunner(0); testSupportedRolesAPI(); diff --git a/solr/core/src/test/org/apache/solr/cluster/placement/ClusterAbstractionsForTest.java b/solr/core/src/test/org/apache/solr/cluster/placement/ClusterAbstractionsForTest.java index bb85bd3..adf4396 100644 --- a/solr/core/src/test/org/apache/solr/cluster/placement/ClusterAbstractionsForTest.java +++ b/solr/core/src/test/org/apache/solr/cluster/placement/ClusterAbstractionsForTest.java @@ -43,7 +43,7 @@ class ClusterAbstractionsForTest { } @Override - public Set<Node> getLiveNodes(boolean filterNonDataNodes) { + public Set<Node> getLiveDataNodes() { return liveNodes; } diff --git a/solr/solr-ref-guide/src/node-roles.adoc b/solr/solr-ref-guide/src/node-roles.adoc index aa36fe6e3..71d9c6c 100644 --- a/solr/solr-ref-guide/src/node-roles.adoc +++ b/solr/solr-ref-guide/src/node-roles.adoc @@ -16,13 +16,13 @@ // 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. +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. == 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. +A role is a designation for a node that indicates that the node may perform a certain function. === 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"]`). diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java index d35bd8f..848c69e 100644 --- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java +++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java @@ -104,11 +104,11 @@ public class ZkStateReader implements SolrCloseable { public static final String COLLECTIONS_ZKNODE = "/collections"; public static final String LIVE_NODES_ZKNODE = "/live_nodes"; + // TODO: Deprecate and remove support for roles.json in an upcoming release. /** * 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. */ public static final String NODE_ROLES = "/node_roles"; public static final String ROLES = "/roles.json";
