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 16e3ec8cd8e31f5b05fcef0daa3feda4e2893f95
Author: Noble Paul <[email protected]>
AuthorDate: Tue Nov 16 15:58:02 2021 +1100

    use a separate node
---
 .../java/org/apache/solr/cloud/ZkController.java   | 56 ++++++++---------
 .../impl/SimpleClusterAbstractionsImpl.java        | 59 +++++++++++-------
 .../src/java/org/apache/solr/core/NodeRole.java    | 70 ++++------------------
 .../test/org/apache/solr/core/TestNodeRole.java    | 69 ---------------------
 .../apache/solr/common/cloud/ZkStateReader.java    |  1 +
 5 files changed, 76 insertions(+), 179 deletions(-)

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 48c26a4..00274aa 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
@@ -73,7 +73,13 @@ import org.apache.solr.common.util.SolrNamedThreadFactory;
 import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.common.util.URLUtil;
 import org.apache.solr.common.util.Utils;
-import org.apache.solr.core.*;
+import org.apache.solr.core.CloseHook;
+import org.apache.solr.core.CloudConfig;
+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;
@@ -100,7 +106,6 @@ import static 
org.apache.solr.common.cloud.ZkStateReader.NODE_NAME_PROP;
 import static org.apache.solr.common.cloud.ZkStateReader.REJOIN_AT_HEAD_PROP;
 import static org.apache.solr.common.cloud.ZkStateReader.SHARD_ID_PROP;
 import static 
org.apache.solr.common.params.CollectionParams.CollectionAction.ADDROLE;
-import static org.apache.solr.common.params.CoreAdminParams.ACTION;
 import static org.apache.zookeeper.ZooDefs.Ids.OPEN_ACL_UNSAFE;
 
 /**
@@ -461,7 +466,6 @@ public class ZkController implements Closeable {
     });
 
     init();
-    persistRoles();
 
     if (distributedClusterStateUpdater.isDistributedStateUpdate()) {
       this.overseerJobQueue = null;
@@ -476,28 +480,6 @@ public class ZkController implements Closeable {
     assert ObjectReleaseTracker.track(this);
   }
 
-  @SuppressWarnings("unchecked")
-  private void persistRoles() throws InterruptedException {
-    try {
-      zkClient.atomicUpdate(ZkStateReader.ROLES,
-              bytes -> {
-                Map<String, Object> map = bytes == null ? null :
-                        (Map<String, Object>) Utils.fromJSON(bytes);
-                map = cc.nodeRole.modifyRoleData(map,
-                        getNodeName());
-                return map == null ? null : Utils.toJSON(map);
-              });
-      if (cc.nodeRole.role() == NodeRole.Type.overseer) {
-        getOverseerCollectionQueue()
-                .offer(Utils.toJSON(Utils.makeMap(ACTION, ADDROLE.name(),
-                        "role", NodeRole.Type.overseer,
-                        "node", getNodeName())));
-      }
-    } catch (KeeperException e) {
-      throw new SolrException(ErrorCode.SERVER_ERROR, "can't update roles");
-    }
-  }
-
   /**
    * <p>Verifies if /clusterstate.json exists in Zookeepeer, and if it does 
and is not empty, refuses to start and outputs
    * a helpful message regarding collection migration.</p>
@@ -873,6 +855,7 @@ public class ZkController implements Closeable {
       throws KeeperException, InterruptedException, IOException {
     ZkCmdExecutor cmdExecutor = new 
ZkCmdExecutor(zkClient.getZkClientTimeout());
     cmdExecutor.ensureExists(ZkStateReader.LIVE_NODES_ZKNODE, zkClient);
+    cmdExecutor.ensureExists(ZkStateReader.NODE_ROLES, zkClient);
     cmdExecutor.ensureExists(ZkStateReader.COLLECTIONS_ZKNODE, zkClient);
     cmdExecutor.ensureExists(ZkStateReader.ALIASES, zkClient);
     byte[] emptyJson = "{}".getBytes(StandardCharsets.UTF_8);
@@ -1105,6 +1088,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));
+
+    }
     zkClient.multi(ops, true);
   }
 
@@ -2210,6 +2198,10 @@ public class ZkController implements Closeable {
 
   public void checkOverseerDesignate() {
     try {
+      if(cc.nodeRole.role() == NodeRole.Type.overseer) {
+        setPreferredOverseer();
+        return;
+      }
       byte[] data = zkClient.getData(ZkStateReader.ROLES, null, new Stat(), 
true);
       if (data == null) return;
       Map<?,?> roles = (Map<?,?>) Utils.fromJSON(data);
@@ -2217,11 +2209,7 @@ public class ZkController implements Closeable {
       List<?> nodeList = (List<?>) roles.get("overseer");
       if (nodeList == null) return;
       if (nodeList.contains(getNodeName())) {
-        ZkNodeProps props = new ZkNodeProps(Overseer.QUEUE_OPERATION, 
ADDROLE.toString().toLowerCase(Locale.ROOT),
-            "node", getNodeName(),
-            "role", "overseer");
-        log.info("Going to add role {} ", props);
-        getOverseerCollectionQueue().offer(Utils.toJSON(props));
+       setPreferredOverseer();
       }
     } catch (NoNodeException nne) {
       return;
@@ -2230,6 +2218,14 @@ public class ZkController implements Closeable {
     }
   }
 
+  private void setPreferredOverseer() throws KeeperException, 
InterruptedException {
+    ZkNodeProps props = new ZkNodeProps(Overseer.QUEUE_OPERATION, 
ADDROLE.toString().toLowerCase(Locale.ROOT),
+        "node", getNodeName(),
+        "role", "overseer");
+    log.info("Going to add role {} ", props);
+    getOverseerCollectionQueue().offer(Utils.toJSON(props));
+  }
+
   public CoreContainer getCoreContainer() {
     return cc;
   }
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 d7453d6..f403aed 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
@@ -33,7 +33,6 @@ 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.solr.core.NodeRole;
 import org.apache.zookeeper.KeeperException;
 
 import javax.annotation.Nonnull;
@@ -61,27 +60,8 @@ class SimpleClusterAbstractionsImpl {
     private final Set<Node> liveNodes;
     private final ClusterState clusterState;
 
-    @SuppressWarnings("unchecked")
     ClusterImpl(SolrCloudManager solrCloudManager) throws IOException {
-      Set<String> liveNodes = 
solrCloudManager.getClusterStateProvider().getLiveNodes();
-      try {
-        if 
(solrCloudManager.getDistribStateManager().hasData(ZkStateReader.ROLES)) {
-          VersionedData rolesData = 
solrCloudManager.getDistribStateManager().getData(ZkStateReader.ROLES);
-          if (rolesData != null && rolesData.getData().length > 0) {
-            Map<String, Object> roles = (Map<String, Object>) 
Utils.fromJSON(rolesData.getData());
-            List<String> noReplicasNodes = (List<String>) 
roles.get(NodeRole.NO_REPLICAS);
-            if (noReplicasNodes != null && !noReplicasNodes.isEmpty()) {
-              liveNodes = new HashSet<>(liveNodes);
-              liveNodes.removeAll(noReplicasNodes);
-            }
-          }
-        }
-
-      } catch (NoSuchElementException nsee) {
-        //no problem
-      } catch (Exception e) {
-        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unable 
to communicate with Zookeeper");
-      }
+      Set<String> liveNodes = filterNonDataNodes(solrCloudManager);
       this.liveNodes = NodeImpl.getNodes(liveNodes);
       clusterState = 
solrCloudManager.getClusterStateProvider().getClusterState();
     }
@@ -108,6 +88,43 @@ class SimpleClusterAbstractionsImpl {
     }
   }
 
+  private static Set<String> filterNonDataNodes(SolrCloudManager 
solrCloudManager) {
+    Set<String> liveNodes = 
solrCloudManager.getClusterStateProvider().getLiveNodes();
+    Set<String> liveNodesCopy = liveNodes;
+    try {
+      List<String> nodesWithRoles =  
solrCloudManager.getDistribStateManager().listData(ZkStateReader.NODE_ROLES);
+      if(!nodesWithRoles.isEmpty()) {
+        for (String node : nodesWithRoles) {
+          VersionedData data = null;
+          try {
+            data = solrCloudManager.getDistribStateManager()
+                    .getData(ZkStateReader.NODE_ROLES + "/" + node);
+          } catch (NoSuchElementException e) {
+            //this node probably went down
+            continue;
+          }
+          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 == liveNodes) {
+                //the map provided should not be modified. So we make a copy
+                liveNodesCopy = new HashSet<>(liveNodes);
+              }
+              liveNodesCopy.remove(node);
+            }
+          }
+        }
+      }
+
+    } catch (NoSuchElementException nsee) {
+      //no problem
+    } catch (Exception e) {
+      throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unable to 
communicate with Zookeeper");
+    }
+    return liveNodesCopy;
+  }
+
 
   static class NodeImpl implements Node {
     public final String nodeName;
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 1de7832..7feb742 100644
--- a/solr/core/src/java/org/apache/solr/core/NodeRole.java
+++ b/solr/core/src/java/org/apache/solr/core/NodeRole.java
@@ -16,19 +16,18 @@
  */
 package org.apache.solr.core;
 
-import com.google.common.collect.ImmutableSet;
+import java.io.IOException;
 import java.util.*;
+import org.apache.solr.common.MapWriter;
 import org.apache.solr.common.StringUtils;
 import org.apache.solr.common.util.StrUtils;
 
-public class NodeRole {
-  private boolean hasData;
-  private Type role;
+public class NodeRole implements MapWriter {
+  private boolean hasData = true;
+  private Type role  =Type.data;
 
   public NodeRole(String role) {
     if (StringUtils.isEmpty(role)) {
-      hasData = true;
-      this.role = Type.data;
       return;
     }
     Set<String> roles = new HashSet<>(StrUtils.split(role, ','));
@@ -58,65 +57,18 @@ public class NodeRole {
     return role;
   }
 
-  @SuppressWarnings("unchecked")
-  public Map<String, Object> modifyRoleData(Map<String, Object> rolesData, 
String nodeName) {
-    if (this.role == Type.data) {
-      //this is a normal data node, nothing to do
-      if (rolesData == null) return null;// no role info, return
-      if (removeStaleRoles(nodeName, rolesData, Collections.emptySet())) {
-        return rolesData;
-      } else {
-        return null;
-      }
-    } else {
-      if (rolesData == null) rolesData = new HashMap<>();
-      List<String> nodes = (List<String>) 
rolesData.computeIfAbsent(role.name(), s -> new ArrayList<String>());
-      boolean isModified = false;
-      if (!nodes.contains(nodeName)) {
-        nodes.add(nodeName);
-        isModified = true;
-      }
-      List<String> nonReplicaNodes = (List<String>) 
rolesData.computeIfAbsent(NO_REPLICAS, s -> new ArrayList<String>());
-      if (hasData) {
-        if(nonReplicaNodes.contains(nodeName)) {
-          nonReplicaNodes.remove(nodeName);
-          isModified = true;
-        }
-      } else {
-        if(!nonReplicaNodes.contains(nodeName)){
-          nonReplicaNodes.add(nodeName);
-          isModified = true;
-        }
-      }
-      if (isModified || removeStaleRoles(nodeName, rolesData, 
ImmutableSet.of(role.name(), NO_REPLICAS))) {
-        return rolesData;
-      } else {
-        return null;
-      }
-    }
-
-  }
-
-  @SuppressWarnings("unchecked")
-  private boolean removeStaleRoles(String nodeName, Map<String, Object> 
rolesData, Set<String> ignore) {
-    boolean[] isModified = new boolean[]{false};
-    rolesData.forEach((s, o) -> {
-      if (ignore.contains(s)) return;
-      if (o instanceof List) {
-        List<String> list = (List<String>) o;
-        if (list.contains(nodeName)) {
-          isModified[0] = true;
-          list.remove(nodeName);
-        }
-      }
-    });
-    return isModified[0];
+  @Override
+  public void writeMap(EntryWriter ew) throws IOException {
+    ew.put("hasData", hasData);
+    ew.put("role", role.name());
   }
 
   public enum Type {
     data, overseer;
   }
 
+
+
   public static final String NO_REPLICAS = "no-replicas";
 
 }
diff --git a/solr/core/src/test/org/apache/solr/core/TestNodeRole.java 
b/solr/core/src/test/org/apache/solr/core/TestNodeRole.java
deleted file mode 100644
index b328983..0000000
--- a/solr/core/src/test/org/apache/solr/core/TestNodeRole.java
+++ /dev/null
@@ -1,69 +0,0 @@
-/*
- * 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.Collection;
-import java.util.Map;
-import org.apache.solr.SolrTestCase;
-import org.apache.solr.common.util.Utils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-public class TestNodeRole extends SolrTestCase {
-  private static final Logger log = 
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
-
-
-  @SuppressWarnings("rawtypes")
-  public void testZkData() {
-    //empty
-    Map<String,Object> rolesData = null;
-    //start a node with overseer role
-    rolesData = new NodeRole("overseer").modifyRoleData(rolesData, "node1");
-    assertEquals("node1", _val(rolesData,  "overseer[0]"));
-    assertEquals("node1", _val(rolesData,  "no-replicas[0]"));
-    //now start another node with overseer
-    rolesData = new NodeRole("overseer").modifyRoleData(rolesData, "node2");
-    assertEquals("node2", _val(rolesData,  "overseer[1]"));
-    assertEquals("node2", _val(rolesData,  "no-replicas[1]"));
-    //now start another node with overseer,data
-    rolesData = new NodeRole("overseer,data").modifyRoleData(rolesData, 
"node3");
-
-    assertEquals("node3", _val(rolesData, "overseer[2]"));
-    assertEquals(2, ((Collection) _val(rolesData, "no-replicas")).size() );
-    //now restart node2 with no role
-
-    rolesData = new NodeRole(null).modifyRoleData(rolesData, "node2");
-    assertEquals("node3", _val(rolesData, "overseer[1]"));
-    assertEquals(1, ((Collection) _val(rolesData, "no-replicas")).size() );
-    //now restart node1 with the original values
-    assertNull(new NodeRole("overseer").modifyRoleData(rolesData, "node1"));
-
-    assertEquals("node1", _val(rolesData,  "overseer[0]"));
-    assertEquals("node1", _val(rolesData,  "no-replicas[0]"));
-    //now restart node1 with overseer,data
-    rolesData =  new NodeRole("overseer,data").modifyRoleData(rolesData, 
"node1");
-    assertEquals("node1", _val(rolesData,  "overseer[0]"));
-    assertEquals(0, ((Collection) _val(rolesData, "no-replicas")).size() );
-
-    log.info("rolesdata : {}", Utils.toJSONString(rolesData));
-  }
-
-  private Object _val(Map<String, Object> rolesData, String path) {
-    return Utils.getObjectByPath(rolesData, false, path);
-  }
-}
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 461dbaf..a01cab2 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
@@ -103,6 +103,7 @@ public class ZkStateReader implements SolrCloseable {
   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";
   public static final String ALIASES = "/aliases.json";
   /**
    * This ZooKeeper file is no longer used starting with Solr 9 but keeping 
the name around to check if it

Reply via email to