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 60c41705e38fb74000708e730799dff5c5b6b51b
Author: Noble Paul <[email protected]>
AuthorDate: Tue Dec 28 13:41:22 2021 +1100

    cleanup and more test assertions
---
 .../src/java/org/apache/solr/core/NodeRoles.java   | 19 +++---
 .../test/org/apache/solr/cloud/NodeRolesTest.java  | 67 +++++++++++++---------
 .../org/apache/solr/cloud/OverseerRolesTest.java   |  2 +-
 solr/solr-ref-guide/src/node-roles.adoc            |  2 -
 .../apache/solr/common/cloud/ZkStateReader.java    |  2 +-
 5 files changed, 49 insertions(+), 43 deletions(-)

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 317f771..2eb2b62 100644
--- a/solr/core/src/java/org/apache/solr/core/NodeRoles.java
+++ b/solr/core/src/java/org/apache/solr/core/NodeRoles.java
@@ -30,12 +30,9 @@ public class NodeRoles {
 
   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";*/
-
+  /**
+   * 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.
@@ -60,7 +57,7 @@ public class NodeRoles {
     }
     for(Role r: Role.values()) {
       if (!roles.containsKey(r)) {
-        roles.put(r, r.defaultMode());
+        roles.put(r, r.modeWhenRoleIsAbsent());
       }
     }
     nodeRoles = Collections.unmodifiableMap(roles);
@@ -82,11 +79,11 @@ public class NodeRoles {
   public enum Mode {
     ON, OFF, ALLOWED, PREFERRED, DISALLOWED;
 
-    @Override
     /**
      * 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);
     }
@@ -99,7 +96,7 @@ public class NodeRoles {
         return Set.of(Mode.ON, Mode.OFF);
       }
       @Override
-      public Mode defaultMode() {
+      public Mode modeWhenRoleIsAbsent() {
         return Mode.OFF;
       }
     },
@@ -109,7 +106,7 @@ public class NodeRoles {
         return Set.of(Mode.ALLOWED, Mode.PREFERRED, Mode.DISALLOWED);
       }
       @Override
-      public Mode defaultMode() {
+      public Mode modeWhenRoleIsAbsent() {
         return Mode.DISALLOWED;
       }
     };
@@ -132,7 +129,7 @@ public class NodeRoles {
     /**
      * Default mode for a role in nodes where this role is not specified.
      */
-    public abstract Mode defaultMode();
+    public abstract Mode modeWhenRoleIsAbsent();
 
     @Override
     public String toString() {
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 0fc0e95..fb05e34 100644
--- a/solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/NodeRolesTest.java
@@ -21,7 +21,6 @@ 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;
@@ -49,37 +48,25 @@ public class NodeRolesTest extends SolrCloudTestCase {
     shutdownCluster();
   }
 
-  @SuppressWarnings("unchecked")
+  @SuppressWarnings("rawtypes")
   public void testRoleIntegration() throws Exception {
     JettySolrRunner j0 = cluster.getJettySolrRunner(0);
-    JettySolrRunner j1, j2;
-    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"));
+    testSupportedRolesAPI();
 
-    j1 = startNodeWithRoles("overseer:preferred,data:off");
+    // 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");
 
-    rsp = new 
V2Request.Builder("/cluster/node-roles").GET().build().process(cluster.getSolrClient());
-    assertEquals(j1.getNodeName(), 
rsp._getStr("node-roles/overseer/preferred[0]", null));
-    assertEquals(j1.getNodeName(), rsp._getStr("node-roles/data/off[0]", 
null));
-    OverseerRolesTest.waitForNewOverseer(20, j1.getNodeName(), false);
+    V2Response rsp;
+    OverseerRolesTest.waitForNewOverseer(20, j1.getNodeName(), true);
 
-    // start another node that is overseer but has data
-    j2 = startNodeWithRoles("overseer:preferred,data:on");
-    rsp = new 
V2Request.Builder("/cluster/node-roles").GET().build().process(cluster.getSolrClient());
+    // 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");
 
-    assertTrue( ((Collection)rsp._get("node-roles/overseer/preferred", 
Collections.emptyList())).contains(j2.getNodeName()));
-    assertTrue( ((Collection)rsp._get("node-roles/data/on", 
Collections.emptyList())).contains(j2.getNodeName()));
-
-    rsp = new 
V2Request.Builder("/cluster/node-roles/role/overseer").GET().build().process(cluster.getSolrClient());
-
-    assertTrue( ((Collection)rsp._get("node-roles/overseer/preferred", 
Collections.emptyList())).contains(j2.getNodeName()));
-    assertTrue( ((Collection)rsp._get("node-roles/overseer/preferred", 
Collections.emptyList())).contains(j1.getNodeName()));
-
-    rsp = new 
V2Request.Builder("/cluster/node-roles/role/overseer/preferred").GET().build().process(cluster.getSolrClient());
-    assertTrue( ((Collection)rsp._get("node-roles/overseer/preferred", 
Collections.emptyList())).contains(j2.getNodeName()));
-    assertTrue( ((Collection)rsp._get("node-roles/overseer/preferred", 
Collections.emptyList())).contains(j1.getNodeName()));
+    // validate the preferred overseers
+    validateNodeRoles(j2.getNodeName(), "node-roles/overseer/" + 
overseerModeOnDataNode, j1.getNodeName(), "node-roles/overseer/preferred");
 
     String COLLECTION_NAME = "TEST_ROLES";
     CollectionAdminRequest
@@ -94,8 +81,32 @@ public class NodeRolesTest extends SolrCloudTestCase {
 
     // Shutdown the dedicated overseer, make sure that node disappears from 
the roles output
     j1.stop();
-    rsp = new 
V2Request.Builder("/cluster/node-roles/role/overseer/preferred").GET().build().process(cluster.getSolrClient());
-    assertFalse (((Collection) rsp._get("node-roles/overseer/preferred" , 
null)). contains(j1.getNodeName()));
+
+    // 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 {
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 508e74e..6e59f09 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 static void waitForNewOverseer(int seconds, Predicate<String> state, 
boolean failOnIntermediateTransition) throws Exception {
+  public 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) {
diff --git a/solr/solr-ref-guide/src/node-roles.adoc 
b/solr/solr-ref-guide/src/node-roles.adoc
index 455de14..aa36fe6e3 100644
--- a/solr/solr-ref-guide/src/node-roles.adoc
+++ b/solr/solr-ref-guide/src/node-roles.adoc
@@ -93,12 +93,10 @@ curl http://localhost:8983/api/cluster/node-roles/supported
 {
   "supported-roles":{
     "data":{
-      "defaultIfAbsent":"off",
       "modes":["off",
         "on"]
     },
     "overseer":{
-      "defaultIfAbsent":"disallowed",
       "modes":["disallowed",
         "allowed",
         "preferred"]
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 8814cc9..d35bd8f 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
@@ -108,7 +108,7 @@ public class ZkStateReader implements SolrCloseable {
    * 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 & remove support for roles.json in an upcoming release.
+   * 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";

Reply via email to