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 84982ef4308451a4ff8096b6f479aa1e62603f11 Author: Ishan Chattopadhyaya <[email protected]> AuthorDate: Wed Nov 17 19:14:34 2021 +0530 Multiple roles supported per node --- .../apache/solr/cloud/OverseerNodePrioritizer.java | 6 +- .../java/org/apache/solr/cloud/ZkController.java | 9 ++- .../apache/solr/cloud/api/collections/Assign.java | 9 ++- .../cloud/api/collections/OverseerRoleCmd.java | 2 +- .../java/org/apache/solr/core/CoreContainer.java | 4 +- .../src/java/org/apache/solr/core/NodeRole.java | 69 +++++++++++--------- .../java/org/apache/solr/handler/ClusterAPI.java | 17 +++-- .../test/org/apache/solr/cloud/NodeRolesTest.java | 73 +++++++++------------- .../client/solrj/cloud/DistribStateManager.java | 12 ++-- 9 files changed, 98 insertions(+), 103 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 9a423a2..f88c686 100644 --- a/solr/core/src/java/org/apache/solr/cloud/OverseerNodePrioritizer.java +++ b/solr/core/src/java/org/apache/solr/cloud/OverseerNodePrioritizer.java @@ -63,17 +63,17 @@ public class OverseerNodePrioritizer { public synchronized void prioritizeOverseerNodes(String overseerId) throws Exception { SolrZkClient zk = zkStateReader.getZkClient(); List<String> overseerDesignates = new ArrayList<>(); - if(zk.exists(ZkStateReader.ROLES,true)) { + 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) { + if (l != null) { overseerDesignates.addAll(l); } } overseerDesignates.addAll(ClusterAPI.getNodesByRole("overseer", new ZkDistribStateManager(zkStateReader.getZkClient()))); - if(overseerDesignates.isEmpty()) return; + if (overseerDesignates.isEmpty()) return; String ldr = OverseerTaskProcessor.getLeaderNode(zk); if(overseerDesignates.contains(ldr)) return; log.info("prioritizing overseer nodes at {} overseer designates are {}", overseerId, overseerDesignates); 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 c955497..05c6c5b 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java +++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java @@ -79,7 +79,6 @@ import org.apache.solr.core.CoreContainer; import org.apache.solr.core.CoreDescriptor; import org.apache.solr.core.SolrCore; import org.apache.solr.core.SolrCoreInitializationException; -import org.apache.solr.core.NodeRole; import org.apache.solr.handler.component.HttpShardHandler; import org.apache.solr.logging.MDCLoggingContext; import org.apache.solr.search.SolrIndexSearcher; @@ -1088,11 +1087,11 @@ public class ZkController implements Closeable { 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)); - if(cc.nodeRole.role() != NodeRole.Type.data) { - //this is a non-data node - ops.add(Op.create(ZkStateReader.NODE_ROLES + "/" + nodeName, Utils.toJSON(cc.nodeRole), zkClient.getZkACLProvider().getACLsToAdd(nodePath), CreateMode.EPHEMERAL)); - } + // Create the roles node as well + ops.add(Op.create(ZkStateReader.NODE_ROLES + "/" + nodeName, + Utils.toJSON(cc.nodeRoles), zkClient.getZkACLProvider().getACLsToAdd(nodePath), CreateMode.EPHEMERAL)); + zkClient.multi(ops, true); } 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 5f726c9..aaa9591 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 @@ -257,17 +257,16 @@ public class Assign { public static void filterNonDataNodes(DistribStateManager zk, List<String> liveNodes) { try { zk.forEachChild(ZkStateReader.NODE_ROLES, (name, data) -> { - if(data != null && data.getData() != null && data.getData().length >0) { + if (data != null && data.getData() != null && data.getData().length > 0) { @SuppressWarnings("unchecked") - Map<String,Object> map = (Map<String, Object>) Utils.fromJSON(data.getData()); - if(Boolean.FALSE.equals(map.get(NodeRole.HAS_DATA))) { + List<String> roles = (List<String>) Utils.fromJSON(data.getData()); + if (roles.contains(NodeRole.Role.DATA.toString()) == false) { liveNodes.remove(name); } } }); - } catch (Exception e) { - throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unable to communicate with Zookeeper"); + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Problem fetching roles from Zookeeper", e); } } diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerRoleCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerRoleCmd.java index 409c480..35c4bd6 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerRoleCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/OverseerRoleCmd.java @@ -64,7 +64,7 @@ public class OverseerRoleCmd implements CollApiCmds.CollectionApiCommand { 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 + if ("false".equals(message.getStr("persist"))) {// no need to persist to roles.json runPrioritizer(); return; } 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 40d5f89..44c63db 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -246,7 +246,7 @@ public class CoreContainer { private final ObjectCache objectCache = new ObjectCache(); - public final NodeRole nodeRole = new NodeRole(System.getProperty(NodeRole.NODE_ROLE)); + public final NodeRole nodeRoles = new NodeRole(System.getProperty(NodeRole.NODE_ROLES_PROP)); private final ClusterSingletons clusterSingletons = new ClusterSingletons( () -> getZkController() != null && @@ -947,7 +947,7 @@ public class CoreContainer { }); clusterSingletons.setReady(); - if(nodeRole.role() == NodeRole.Type.overseer) { + if (nodeRoles.getRoles().contains(NodeRole.Role.OVERSEER)) { try { log.info("This node is started as an overseer"); zkSys.getZkController().setPreferredOverseer(); diff --git a/solr/core/src/java/org/apache/solr/core/NodeRole.java b/solr/core/src/java/org/apache/solr/core/NodeRole.java index f0dd794..01c96fd 100644 --- a/solr/core/src/java/org/apache/solr/core/NodeRole.java +++ b/solr/core/src/java/org/apache/solr/core/NodeRole.java @@ -17,53 +17,62 @@ package org.apache.solr.core; import java.io.IOException; +import java.lang.invoke.MethodHandles; import java.util.*; + +import org.apache.solr.common.IteratorWriter; import org.apache.solr.common.MapWriter; import org.apache.solr.common.StringUtils; import org.apache.solr.common.util.StrUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class NodeRole implements IteratorWriter { + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + public static final String NODE_ROLES_PROP = "solr.node.roles"; -public class NodeRole implements MapWriter { - private boolean hasData = true; - private Type role =Type.data; + private final Set<Role> roles; public NodeRole(String role) { if (StringUtils.isEmpty(role)) { + // if no roles were specified, assume "data" role for backcompat reasons + roles = Set.of(Role.DATA); return; } - Set<String> roles = new HashSet<>(StrUtils.split(role, ',')); - if (roles.isEmpty()) return; - for (String s : roles) { - if (Type.data.name().equals(s)) { - hasData = true; - continue; - } - if (Type.overseer.name().equals(s)) { - this.role = Type.overseer; - hasData = false; - } else { - throw new RuntimeException("Unknown type"); - } + Set<String> rolesSet = new HashSet<>(StrUtils.split(role, ',')); + if (rolesSet.isEmpty()) { + // if no roles were specified, assume "data" role for backcompat reasons + roles = Set.of(Role.DATA); + return; + } + roles = new TreeSet<>(); + for (String r: rolesSet) { + roles.add(Role.valueOfCaseInsensitive(r)); } - - } - - public boolean hasData() { - return hasData; } - public Type role() { - return role; + public Set<Role> getRoles() { + return roles; } @Override - public void writeMap(EntryWriter ew) throws IOException { - ew.put(HAS_DATA, hasData); - ew.put("role", role.name()); + public void writeIter(ItemWriter iw) throws IOException { + for (Role role: roles) iw.add(role.toString()); } - public enum Type { - data, overseer; + public enum Role { + DATA, OVERSEER; + + public static Role valueOfCaseInsensitive(String value) { + // Given a user string "overseer", convert to OVERSEER and return the enum value + String canonicalValue = value.toUpperCase().replace(' ', '_'); + return Role.valueOf(canonicalValue); + } + + @Override + public String toString() { + return super.toString().toLowerCase(); + } } - public static final String HAS_DATA = "hasData"; - public static final String NODE_ROLE = "solr.node.role"; } 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 9541191..5f512d2 100644 --- a/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java +++ b/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java @@ -18,6 +18,7 @@ package org.apache.solr.handler; import java.io.IOException; +import java.lang.invoke.MethodHandles; import java.util.*; import com.google.common.collect.Maps; @@ -46,6 +47,8 @@ import org.apache.solr.handler.admin.ConfigSetsHandler; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; import org.apache.zookeeper.KeeperException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import static org.apache.solr.client.solrj.SolrRequest.METHOD.DELETE; import static org.apache.solr.client.solrj.SolrRequest.METHOD.GET; @@ -74,17 +77,19 @@ public class ClusterAPI { 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) @SuppressWarnings("unchecked") + // nocommit: it must also output all data nodes that didn't start with -Dsolr.node.roles public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { - Map <String, Map<String,Object>> result = new LinkedHashMap<>(); + Map <String, List<String>> result = new LinkedHashMap<>(); collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(). forEachChild(ZkStateReader.NODE_ROLES, (node, data) -> { - if(data != null && data.getData() != null) { - result.put(node, (Map<String, Object>) Utils.fromJSON(data.getData())); + if (data != null && data.getData() != null) { + result.put(node, ((List<String>) Utils.fromJSON(data.getData()))); } }); rsp.add("node-roles", result); @@ -102,10 +107,10 @@ public class ClusterAPI { public static List<String> getNodesByRole(String role, DistribStateManager zk) throws InterruptedException, IOException, KeeperException { List<String> result = new ArrayList<>(); zk.forEachChild(ZkStateReader.NODE_ROLES, (node, data) -> { - if(data != null && data.getData() != null) { + if (data != null && data.getData() != null) { @SuppressWarnings("unchecked") - Map<String, Object> roleData = (Map<String, Object>) Utils.fromJSON(data.getData()); - if(role.equals(roleData.get("role"))) { + List<String> rolesData = (List<String>) Utils.fromJSON(data.getData()); + if (rolesData.contains(role)) { result.add(node); } } 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 42c5a0f..e5f2ee1 100644 --- a/solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java @@ -19,6 +19,8 @@ package org.apache.solr.cloud; import java.lang.invoke.MethodHandles; import java.util.List; +import java.util.TreeSet; + import org.apache.solr.client.solrj.embedded.JettySolrRunner; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.request.V2Request; @@ -49,47 +51,20 @@ public class NodeRolesTest extends SolrCloudTestCase { JettySolrRunner j0 = cluster.getJettySolrRunner(0); JettySolrRunner j1 = null, j2 = null; - System.setProperty(NodeRole.NODE_ROLE, "overseer"); - try { - j1 = cluster.startJettySolrRunner(); - } finally { - System.clearProperty(NodeRole.NODE_ROLE); - } + j1 = startNodeWithRoles("overseer"); - V2Response rsp = new V2Request - .Builder("/cluster/node-roles") - .GET() - .build() - .process(cluster.getSolrClient()); - assertEquals(Boolean.FALSE, rsp._get(List.of("node-roles", j1.getNodeName(), NodeRole.HAS_DATA), null)); - assertEquals("overseer", rsp._get(List.of("node-roles", j1.getNodeName(), "role"), null)); + V2Response rsp = new V2Request.Builder("/cluster/node-roles").GET().build().process(cluster.getSolrClient()); + assertEquals(List.of("overseer"), rsp._get(List.of("node-roles", j1.getNodeName()), null)); OverseerRolesTest.waitForNewOverseer(20, j1.getNodeName(), false); - //start another node that is overseer but has data - System.setProperty(NodeRole.NODE_ROLE, "overseer,data"); - try { - j2 = cluster.startJettySolrRunner(); - } finally { - System.clearProperty(NodeRole.NODE_ROLE); - } - rsp = new V2Request - .Builder("/cluster/node-roles") - .GET() - .build() - .process(cluster.getSolrClient()); - assertEquals(Boolean.TRUE, rsp._get(List.of("node-roles", j2.getNodeName(), NodeRole.HAS_DATA), null)); - assertEquals("overseer", rsp._get(List.of("node-roles", j2.getNodeName(), "role"), null)); - - rsp = new V2Request - .Builder("/cluster/node-roles/overseer") - .GET() - .build() - .process(cluster.getSolrClient()); - List<String> overseerNodes = (List<String>) rsp._get("nodes", null); - assertTrue(overseerNodes.contains(j1.getNodeName())); - assertTrue(overseerNodes.contains(j2.getNodeName())); + //start another node that is overseer but has data + j2 = startNodeWithRoles("overseer,data"); + rsp = new V2Request.Builder("/cluster/node-roles").GET().build().process(cluster.getSolrClient()); + assertEquals(List.of("data", "overseer"), rsp._get(List.of("node-roles", j2.getNodeName()), null)); + rsp = new V2Request.Builder("/cluster/node-roles/overseer").GET().build().process(cluster.getSolrClient()); + assertEquals(new TreeSet<String>(List.of(j1.getNodeName(), j2.getNodeName())), new TreeSet<String>((List<String>) rsp._get("nodes", null))); String COLLECTION_NAME = "TEST_ROLES"; CollectionAdminRequest @@ -97,18 +72,26 @@ public class NodeRolesTest extends SolrCloudTestCase { .process(cluster.getSolrClient()); cluster.waitForActiveCollection(COLLECTION_NAME, 3, 3); - String overseer = j1.getNodeName(); + // 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, overseer)); + .forEachReplica((s, replica) -> assertNotEquals(replica.node, dedicatedOverseer)); - //now shutdown the overseer + // Shutdown the dedicated overseer, make sure that node disappears from the roles output j1.stop(); - rsp = new V2Request - .Builder("/cluster/node-roles") - .GET() - .build() - .process(cluster.getSolrClient()); - assertNull(rsp._get(List.of("node-roles", overseer), null)); + rsp = new V2Request.Builder("/cluster/node-roles").GET().build().process(cluster.getSolrClient()); + assertNull(rsp._get(List.of("node-roles", dedicatedOverseer), null)); + } + + private JettySolrRunner startNodeWithRoles(String roles) throws Exception { + JettySolrRunner jetty; + System.setProperty(NodeRole.NODE_ROLES_PROP, roles); + try { + jetty = cluster.startJettySolrRunner(); + } finally { + System.clearProperty(NodeRole.NODE_ROLES_PROP); + } + return jetty; } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/DistribStateManager.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/DistribStateManager.java index ebd4db7..84010c8 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/DistribStateManager.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/DistribStateManager.java @@ -143,21 +143,21 @@ public interface DistribStateManager extends SolrCloseable { } } } - default void forEachChild(String path, BiConsumer<String, VersionedData> fun) + default void forEachChild(String path, BiConsumer<String, VersionedData> fun) throws InterruptedException, IOException, KeeperException { List<String> children = null; try { - children = listData(ZkStateReader.NODE_ROLES); + children = listData(path); } catch (NoSuchElementException e) { return; } - if(!children.isEmpty()) { - for (String node : children) { + if (!children.isEmpty()) { + for (String node: children) { VersionedData data = null; try { - data = getData(ZkStateReader.NODE_ROLES + "/" + node); + data = getData(path + "/" + node); } catch (NoSuchElementException e) { - //this node probably was deleted + // this node must've been deleted continue; } fun.accept(node, data);
