SOLR-18569: Wait on init for ephemeral node-role cleanup

changelog/unreleased/SOLR-18569-wait-for-node-role-deletion-on-startup.yml

@@ -0,0 +1,9 @@
+title: >
+  Solr startup no longer hangs if ephemeral "node-role" znodes from a previous process are still present.
+type: fixed
+authors:
+  - name: Jason Gerlowski
+  - name: Nick Shanin
+links:
+  - name: SOLR-18569
+    url: https://issues.apache.org/jira/browse/SOLR-18569

solr/core/src/java/org/apache/solr/cloud/ZkController.java

@@ -1171,39 +1171,69 @@ private void init() {
     }
   }
 
-  private void checkForExistingEphemeralNode() throws KeeperException, InterruptedException {
+  /**
+   * Waits for any ephemeral znodes left by a previous process with this node name to disappear.
+   *
+   * <p>Covers both the {@code /live_nodes} entry and this node's entries under any {@code
+   * /node_roles/<role>/<mode>}. Throws if any are still present after twice the ZK session timeout.
+   *
+   * @throws SolrException if ephemeral nodes were not all removed by the end of the waiting period
+   */
+  // package-private for testing
+  void checkForExistingEphemeralNode() throws KeeperException, InterruptedException {
     String nodeName = getNodeName();
-    String nodePath = ZkStateReader.LIVE_NODES_ZKNODE + "/" + nodeName;
 
-    if (!zkClient.exists(nodePath)) {
-      return;
+    List<String> candidatePaths = new ArrayList<>();
+    candidatePaths.add(ZkStateReader.LIVE_NODES_ZKNODE + "/" + nodeName);
+    for (NodeRoles.Role role : NodeRoles.Role.values()) {
+      for (String mode : role.supportedModes()) {
+        candidatePaths.add(NodeRoles.getZNodeForRoleMode(role, mode) + "/" + nodeName);
+      }
     }
 
-    final CountDownLatch deletedLatch = new CountDownLatch(1);
-    Stat stat =
-        zkClient.exists(
-            nodePath,
-            event -> {
-              if (Watcher.Event.EventType.None.equals(event.getType())) {
-                return;
-              }
-              if (Watcher.Event.EventType.NodeDeleted.equals(event.getType())) {
-                deletedLatch.countDown();
-              }
-            });
+    // Set a deletion watch on each znode that exists; the latch counts down as each is deleted
+    final List<String> watchedPaths = new ArrayList<>();
+    final List<CountDownLatch> latches = new ArrayList<>();
+    for (String path : candidatePaths) {
+      if (!zkClient.exists(path)) {
+        continue;
+      }
+      final CountDownLatch deletedLatch = new CountDownLatch(1);
+      Stat stat =
+          zkClient.exists(
+              path,
+              event -> {
+                if (Watcher.Event.EventType.None.equals(event.getType())) {
+                  return;
+                }
+                if (Watcher.Event.EventType.NodeDeleted.equals(event.getType())) {
+                  deletedLatch.countDown();
+                }
+              });
+      if (stat == null) {
+        // znode suddenly disappeared but that's okay
+        continue;
+      }
+      watchedPaths.add(path);
+      latches.add(deletedLatch);
+    }
 
-    if (stat == null) {
-      // znode suddenly disappeared but that's okay
+    if (watchedPaths.isEmpty()) {
       return;
     }
 
-    boolean deleted =
-        deletedLatch.await(zkClient.getZkSessionTimeout() * 2L, TimeUnit.MILLISECONDS);
-    if (!deleted) {
-      throw new SolrException(
-          ErrorCode.SERVER_ERROR,
-          "A previous ephemeral live node still exists. "
-              + "Solr cannot continue. Please ensure that no other Solr process using the same port is running already.");
+    // A single overall deadline, shared by all of the znodes
+    final long deadlineNanos =
+        System.nanoTime() + TimeUnit.MILLISECONDS.toNanos(zkClient.getZkSessionTimeout() * 2L);
+    for (int i = 0; i < latches.size(); i++) {
+      long remainingNanos = deadlineNanos - System.nanoTime();
+      if (!latches.get(i).await(Math.max(remainingNanos, 0), TimeUnit.NANOSECONDS)) {
+        throw new SolrException(
+            ErrorCode.SERVER_ERROR,
+            "An ephemeral node from a previous process representing this Solr instance still exists: "
+                + watchedPaths.get(i)
+                + ". Solr cannot continue. Please ensure that no other Solr process using the same port is running already.");
+      }
     }
   }
 

solr/core/src/test/org/apache/solr/cloud/ZkControllerTest.java

@@ -59,6 +59,7 @@
 import org.apache.solr.core.CloudConfig;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.CoreDescriptor;
+import org.apache.solr.core.NodeRoles;
 import org.apache.solr.core.PluginInfo;
 import org.apache.solr.core.SolrXmlConfig;
 import org.apache.solr.handler.admin.CoreAdminHandler;
@@ -222,6 +223,108 @@ public void testLiveNodeDataStored() throws Exception {
     }
   }
 
+  @Test
+  public void testStartupWaitsForLeftoverEphemeralNodesToDisappear() throws Exception {
+    final String staleRoleEntry = ephemeralNodeRolePath(NodeRoles.Role.OVERSEER);
+    runWithLeftoverEphemeralNodes(
+        "testStartupWaitsForLeftoverEphemeralNodes",
+        staleRoleEntry,
+        (zkController, otherClient, leftoverPaths) -> {
+          // Remove the leftovers shortly after the check starts waiting on them
+          AtomicReference<Exception> deleterFailure = new AtomicReference<>();
+          Thread deleter =
+              new Thread(
+                  () -> {
+                    try {
+                      Thread.sleep(1000);
+                      for (String path : leftoverPaths) {
+                        otherClient.delete(path, -1);
+                      }
+                    } catch (Exception e) {
+                      deleterFailure.set(e);
+                    }
+                  });
+          deleter.start();
+          try {
+            // Returns normally once everything is gone
+            zkController.checkForExistingEphemeralNode();
+            for (String path : leftoverPaths) {
+              assertFalse(
+                  path + " should be gone when the check returns", otherClient.exists(path));
+            }
+          } finally {
+            deleter.join(TimeUnit.SECONDS.toMillis(30));
+          }
+          assertFalse("Deleter thread did not finish", deleter.isAlive());
+          assertNull("Deleter thread failed", deleterFailure.get());
+        });
+  }
+
+  @Test
+  public void testStartupFailsIfLeftoverEphemeralNodeNeverDisappears() throws Exception {
+    final String staleRoleEntry = ephemeralNodeRolePath(NodeRoles.Role.OVERSEER);
+    runWithLeftoverEphemeralNodes(
+        "testStartupFailsIfLeftoverEphemeralNode",
+        staleRoleEntry,
+        (zkController, otherClient, leftoverPaths) -> {
+          // Only the role entry is left behind, so the message must name it specifically
+          SolrException e =
+              expectThrows(SolrException.class, zkController::checkForExistingEphemeralNode);
+          assertEquals(SolrException.ErrorCode.SERVER_ERROR.code, e.code());
+          assertTrue(
+              "Message should name the leftover path: " + e.getMessage(),
+              e.getMessage().contains(staleRoleEntry));
+        });
+  }
+
+  /** The path of this test node's entry under a mode of 'role' that the node isn't using itself */
+  private static String ephemeralNodeRolePath(NodeRoles.Role role) {
+    String unusedMode = NodeRoles.MODE_PREFERRED; // default roles use 'allowed'
+    assertTrue(role.supportedModes().contains(unusedMode));
+    return NodeRoles.getZNodeForRoleMode(role, unusedMode) + "/" + TEST_NODE_NAME;
+  }
+
+  private static final String TEST_NODE_NAME = "127.0.0.1:8983_solr";
+
+  @FunctionalInterface
+  private interface LeftoverNodesCheck {
+    void run(ZkController zkController, SolrZkClient otherClient, List<String> leftoverPaths)
+        throws Exception;
+  }
+
+  /**
+   * Starts a ZkController, removes its own ephemeral entries, then has a different ZK client create
+   * the given ephemeral paths (simulating a previous process with the same node name that hasn't
+   * fully gone away) before running 'check'.
+   */
+  private void runWithLeftoverEphemeralNodes(
+      String dirName, String leftoverPath, LeftoverNodesCheck check) throws Exception {
+    ZkTestServer server = new ZkTestServer(createTempDir(dirName));
+    try {
+      server.run();
+      CoreContainer cc = getCoreContainer();
+      // Short session timeout, as the check waits for twice the session timeout
+      CloudConfig cloudConfig =
+          new CloudConfig.CloudConfigBuilder("127.0.0.1", 8983).setZkClientTimeout(3000).build();
+      ZkController zkController = new ZkController(cc, server.getZkAddress(), 3000, cloudConfig);
+      try (SolrZkClient otherClient =
+          new SolrZkClient.Builder()
+              .withUrl(server.getZkAddress())
+              .withTimeout(TIMEOUT, TimeUnit.MILLISECONDS)
+              .build()) {
+        assertEquals(TEST_NODE_NAME, zkController.getNodeName());
+        zkController.removeEphemeralLiveNode();
+        otherClient.create(leftoverPath, null, CreateMode.EPHEMERAL);
+        check.run(zkController, otherClient, List.of(leftoverPath));
+      } finally {
+        zkController.close();
+        cc.shutdown();
+      }
+    } finally {
+      server.shutdown();
+    }
+  }
+
   @LogLevel(value = "org.apache.solr.cloud=DEBUG;org.apache.solr.cloud.overseer=DEBUG")
   @Test
   public void testPublishAndWaitForDownStates() throws Exception {