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 71c11f9abf8a06dda53deff079a8cddb1c1e955d Author: Noble Paul <[email protected]> AuthorDate: Wed Nov 17 16:55:19 2021 +1100 added test --- .../apache/solr/cloud/api/collections/Assign.java | 24 +++++- .../cloud/api/collections/CreateCollectionCmd.java | 2 +- .../solr/cloud/api/collections/RestoreCmd.java | 2 +- .../impl/SimpleClusterAbstractionsImpl.java | 34 +------- .../java/org/apache/solr/core/CoreContainer.java | 2 +- .../src/java/org/apache/solr/core/NodeRole.java | 15 ++-- .../java/org/apache/solr/handler/ClusterAPI.java | 3 +- .../test/org/apache/solr/cloud/NodeRolesTest.java | 97 ++++++++++++++++++++++ .../org/apache/solr/cloud/OverseerRolesTest.java | 4 +- 9 files changed, 133 insertions(+), 50 deletions(-) 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 c6702ef..5f726c9 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 @@ -51,7 +51,9 @@ import org.apache.solr.common.cloud.Slice; import org.apache.solr.common.cloud.ZkNodeProps; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.StrUtils; +import org.apache.solr.common.util.Utils; import org.apache.solr.core.CoreContainer; +import org.apache.solr.core.NodeRole; import org.apache.solr.util.NumberUtils; import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.KeeperException; @@ -227,7 +229,9 @@ public class Assign { return false; } - public static List<String> getLiveOrLiveAndCreateNodeSetList(final Set<String> liveNodes, final ZkNodeProps message, final Random random) { + public static List<String> getLiveOrLiveAndCreateNodeSetList(final Set<String> liveNodes, final ZkNodeProps message, final Random random, + DistribStateManager zk) { + List<String> nodeList; final String createNodeSetStr = message.getStr(CREATE_NODE_SET); final List<String> createNodeList = (createNodeSetStr == null) ? null : @@ -243,12 +247,30 @@ public class Assign { } } else { nodeList = new ArrayList<>(liveNodes); + filterNonDataNodes(zk, nodeList); Collections.shuffle(nodeList, random); } return nodeList; } + 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) { + @SuppressWarnings("unchecked") + Map<String,Object> map = (Map<String, Object>) Utils.fromJSON(data.getData()); + if(Boolean.FALSE.equals(map.get(NodeRole.HAS_DATA))) { + liveNodes.remove(name); + } + } + }); + + } catch (Exception e) { + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unable to communicate with Zookeeper"); + } + } + static class ReplicaCount { public final String nodeName; public int thisCollectionNodes = 0; diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java index 66c21b6..e8a185e 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java @@ -406,7 +406,7 @@ public class CreateCollectionCmd implements CollApiCmds.CollectionApiCommand { // but (for now) require that each core goes on a distinct node. List<ReplicaPosition> replicaPositions; - List<String> nodeList = Assign.getLiveOrLiveAndCreateNodeSetList(clusterState.getLiveNodes(), message, CollectionHandlingUtils.RANDOM); + List<String> nodeList = Assign.getLiveOrLiveAndCreateNodeSetList(clusterState.getLiveNodes(), message, CollectionHandlingUtils.RANDOM, cloudManager.getDistribStateManager()); if (nodeList.isEmpty()) { log.warn("It is unusual to create a collection ({}) without cores.", collectionName); diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java index 2732cb9..af247de 100644 --- a/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java +++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/RestoreCmd.java @@ -182,7 +182,7 @@ public class RestoreCmd implements CollApiCmds.CollectionApiCommand { this.shardHandler = ccc.newShardHandler(); this.nodeList = Assign.getLiveOrLiveAndCreateNodeSetList( - zkStateReader.getClusterState().getLiveNodes(), message, CollectionHandlingUtils.RANDOM); + zkStateReader.getClusterState().getLiveNodes(), message, CollectionHandlingUtils.RANDOM, container.getZkController().getSolrCloudManager().getDistribStateManager()); } @Override 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 16ce53c..48f3f50 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 @@ -19,23 +19,16 @@ package org.apache.solr.cluster.placement.impl; import java.io.IOException; import java.util.*; -import java.util.concurrent.atomic.AtomicReference; -import java.util.function.BiConsumer; import java.util.stream.Collectors; import com.google.common.collect.Maps; import org.apache.solr.client.solrj.cloud.SolrCloudManager; -import org.apache.solr.client.solrj.cloud.VersionedData; import org.apache.solr.cluster.*; -import org.apache.solr.common.SolrException; import org.apache.solr.common.cloud.ClusterState; import org.apache.solr.common.cloud.DocCollection; import org.apache.solr.common.cloud.Slice; -import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.CollectionAdminParams; import org.apache.solr.common.util.Pair; -import org.apache.solr.common.util.Utils; -import org.apache.zookeeper.KeeperException; import javax.annotation.Nonnull; @@ -63,8 +56,7 @@ class SimpleClusterAbstractionsImpl { private final ClusterState clusterState; ClusterImpl(SolrCloudManager solrCloudManager) throws IOException { - Set<String> liveNodes = filterNonDataNodes(solrCloudManager); - this.liveNodes = NodeImpl.getNodes(liveNodes); + liveNodes = NodeImpl.getNodes(solrCloudManager.getClusterStateProvider().getLiveNodes()); clusterState = solrCloudManager.getClusterStateProvider().getClusterState(); } @@ -90,30 +82,6 @@ class SimpleClusterAbstractionsImpl { } } - private static Set<String> filterNonDataNodes(SolrCloudManager solrCloudManager) { - Set<String> liveNodes = solrCloudManager.getClusterStateProvider().getLiveNodes(); - AtomicReference<Set<String>> liveNodesCopy = new AtomicReference<>(liveNodes); - try { - solrCloudManager.getDistribStateManager().forEachChild(ZkStateReader.NODE_ROLES, (name, data) -> { - 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("hasData"))) { - if(liveNodesCopy.get() == liveNodes) { - //the map provided should not be modified. So we make a copy - liveNodesCopy.set(new HashSet<>(liveNodes)); - } - liveNodesCopy.get().remove(name); - } - } - }); - - } catch (Exception e) { - throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unable to communicate with Zookeeper"); - } - return liveNodesCopy.get(); - } - static class NodeImpl implements Node { public final String nodeName; 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 f2c2b78..f85f3d7 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("node.roles")); + public final NodeRole nodeRole = new NodeRole(System.getProperty(NodeRole.NODE_ROLE)); private final ClusterSingletons clusterSingletons = new ClusterSingletons( () -> getZkController() != null && 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 7feb742..f0dd794 100644 --- a/solr/core/src/java/org/apache/solr/core/NodeRole.java +++ b/solr/core/src/java/org/apache/solr/core/NodeRole.java @@ -31,10 +31,7 @@ public class NodeRole implements MapWriter { return; } Set<String> roles = new HashSet<>(StrUtils.split(role, ',')); - if (roles.isEmpty()) { - hasData = true; - return; - } + if (roles.isEmpty()) return; for (String s : roles) { if (Type.data.name().equals(s)) { hasData = true; @@ -42,6 +39,7 @@ public class NodeRole implements MapWriter { } if (Type.overseer.name().equals(s)) { this.role = Type.overseer; + hasData = false; } else { throw new RuntimeException("Unknown type"); } @@ -59,16 +57,13 @@ public class NodeRole implements MapWriter { @Override public void writeMap(EntryWriter ew) throws IOException { - ew.put("hasData", hasData); + ew.put(HAS_DATA, hasData); ew.put("role", role.name()); } public enum Type { data, overseer; } - - - - public static final String NO_REPLICAS = "no-replicas"; - + 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 6f6773d..74b221f 100644 --- a/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java +++ b/solr/core/src/java/org/apache/solr/handler/ClusterAPI.java @@ -75,6 +75,7 @@ public class ClusterAPI { @EndPoint(method = GET, path = "/cluster/node-roles", permission = COLL_READ_PERM) + @SuppressWarnings("unchecked") public void roles(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { Map <String, Map<String,Object>> result = new LinkedHashMap<>(); collectionsHandler.getCoreContainer().getZkController().getSolrCloudManager().getDistribStateManager(). @@ -89,7 +90,7 @@ public class ClusterAPI { @EndPoint(method = GET, path = "/cluster/node-roles/{role}", permission = COLL_READ_PERM) - public void rolesOfNode(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { + public void nodesWithRole(SolrQueryRequest req, SolrQueryResponse rsp) throws Exception { String role = req.getPathTemplateValues().get("role"); List<String> result = new ArrayList<>(); diff --git a/solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java b/solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java new file mode 100644 index 0000000..70ab96b --- /dev/null +++ b/solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java @@ -0,0 +1,97 @@ +package org.apache.solr.cloud; + +import java.lang.invoke.MethodHandles; +import java.util.List; +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.NodeRole; +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; + System.setProperty(NodeRole.NODE_ROLE, "overseer"); + try { + j1 = cluster.startJettySolrRunner(); + } finally { + System.clearProperty(NodeRole.NODE_ROLE); + } + + 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)); + + 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())); + + + String COLLECTION_NAME = "TEST_ROLES"; + CollectionAdminRequest + .createCollection(COLLECTION_NAME, "conf", 3, 1) + .process(cluster.getSolrClient()); + cluster.waitForActiveCollection(COLLECTION_NAME, 3, 3); + + String overseer = j1.getNodeName(); + cluster.getSolrClient().getClusterStateProvider().getCollection(COLLECTION_NAME) + .forEachReplica((s, replica) -> assertNotEquals(replica.node, overseer)); + + //now shutdown the overseer + j1.stop(); + rsp = new V2Request + .Builder("/cluster/node-roles") + .GET() + .build() + .process(cluster.getSolrClient()); + assertNull(rsp._get(List.of("node-roles", overseer), null)); + } + +} diff --git a/solr/core/src/test/org/apache/solr/cloud/OverseerRolesTest.java b/solr/core/src/test/org/apache/solr/cloud/OverseerRolesTest.java index c437a30..508e74e 100644 --- a/solr/core/src/test/org/apache/solr/cloud/OverseerRolesTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/OverseerRolesTest.java @@ -54,7 +54,7 @@ public class OverseerRolesTest extends SolrCloudTestCase { shutdownCluster(); } - private void waitForNewOverseer(int seconds, Predicate<String> state, boolean failOnIntermediateTransition) throws Exception { + private static void waitForNewOverseer(int seconds, Predicate<String> state, boolean failOnIntermediateTransition) throws Exception { TimeOut timeout = new TimeOut(seconds, TimeUnit.SECONDS, TimeSource.NANO_TIME); String current = null; while (timeout.hasTimedOut() == false) { @@ -72,7 +72,7 @@ public class OverseerRolesTest extends SolrCloudTestCase { fail("Timed out waiting for overseer state change. The current overseer is: "+current); } - private void waitForNewOverseer(int seconds, String expected, boolean failOnIntermediateTransition) throws Exception { + public static void waitForNewOverseer(int seconds, String expected, boolean failOnIntermediateTransition) throws Exception { log.info("Expecting node: {}", expected); waitForNewOverseer(seconds, s -> Objects.equals(s, expected), failOnIntermediateTransition); }
