Moved getLock() to AbstractServer, added ServiceLock verification thread

core/src/main/java/org/apache/accumulo/core/conf/Property.java

@@ -329,7 +329,7 @@ public enum Property {
           + " was changed and it now can accept multiple class names. The metrics spi was introduced in 2.1.3,"
           + " the deprecated factory is org.apache.accumulo.core.metrics.MeterRegistryFactory.",
       "2.1.0"),
-  GENERAL_SERVER_LOCK_VERIFICATION_INTERVAL("general.server.lock.verification.interval", "2m",
+  GENERAL_SERVER_LOCK_VERIFICATION_INTERVAL("general.server.lock.verification.interval", "0",
       PropertyType.TIMEDURATION,
       "Interval at which the Manager and TabletServer should verify their server locks. A value of zero"
           + " disables this check.",

core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java

@@ -772,14 +772,15 @@ public static boolean deleteLock(ZooReaderWriter zk, ServiceLockPath path, Strin
    * @return true if lock path still exists, false otherwise and on error
    */
   public boolean verifyLockAtSource() {
-    if (getLockPath() == null) {
+    final String lockPath = getLockPath();
+    if (lockPath == null) {
       // lock not set yet
       return false;
     }
     try {
-      return null != this.zooKeeper.exists(getLockPath(), false);
-    } catch (KeeperException | InterruptedException e) {
-      LOG.error("Error verfiying lock at {}", getLockPath(), e);
+      return null != this.zooKeeper.exists(lockPath, false);
+    } catch (KeeperException | InterruptedException | RuntimeException e) {
+      LOG.error("Error verfiying lock at {}", lockPath, e);
       return false;
     }
   }

server/base/src/main/java/org/apache/accumulo/server/AbstractServer.java

@@ -37,6 +37,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Preconditions;
+
 import io.micrometer.core.instrument.MeterRegistry;
 
 public abstract class AbstractServer implements AutoCloseable, MetricsProducer, Runnable {
@@ -158,19 +160,26 @@ public String getApplicationName() {
   public abstract ServiceLock getLock();
 
   public void startServiceLockVerificationThread() {
+    Preconditions.checkState(verificationThread == null);
+    Preconditions.checkState(serverThread != null);
     final long interval =
         getConfiguration().getTimeInMillis(Property.GENERAL_SERVER_LOCK_VERIFICATION_INTERVAL);
     if (interval > 0) {
       verificationThread = Threads.createThread("service-lock-verification-thread",
           OptionalInt.of(Thread.NORM_PRIORITY + 1), () -> {
-            while (true && serverThread.isAlive()) {
+            while (serverThread.isAlive()) {
               ServiceLock lock = getLock();
               try {
+                log.trace(
+                    "ServiceLockVerificationThread - checking ServiceLock existence in ZooKeeper");
                 if (lock != null && !lock.verifyLockAtSource()) {
                   Halt.halt("Lock verification thread could not find lock", -1);
                 }
                 // Need to sleep, not yield when the thread priority is greater than NORM_PRIORITY
                 // so that this thread does not get immediately rescheduled.
+                log.trace(
+                    "ServiceLockVerificationThread - ServiceLock exists in ZooKeeper, sleeping for {}ms",
+                    interval);
                 Thread.sleep(interval);
               } catch (InterruptedException e) {
                 if (serverThread.isAlive()) {
@@ -181,6 +190,9 @@ public void startServiceLockVerificationThread() {
             }
           });
       verificationThread.start();
+    } else {
+      log.debug(
+          "ServiceLockVerificationThread not started as GENERAL_SERVER_LOCK_VERIFICATION_INTERVAL is zero");
     }
   }
 

test/src/main/java/org/apache/accumulo/test/functional/HalfDeadServerWatcherIT.java

@@ -19,7 +19,6 @@
 package org.apache.accumulo.test.functional;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -142,7 +141,14 @@ public void testOne() throws Exception {
       assertTrue(testTabletServerWithStuckWatcherDies());
     } else {
       // This test should time out
-      assertThrows(IllegalStateException.class, () -> testTabletServerWithStuckWatcherDies());
+      try {
+        testTabletServerWithStuckWatcherDies();
+        fail("Test did not time out.");
+      } catch (IllegalStateException e) {
+        if (!e.getMessage().contains("Timeout exceeded")) {
+          fail("Unexpected exception: " + e.getMessage());
+        }
+      }
     }
   }
 
@@ -155,7 +161,14 @@ public void testTwo() throws Exception {
       assertTrue(testTabletServerWithStuckWatcherDies());
     } else {
       // This test should time out
-      assertThrows(IllegalStateException.class, () -> testTabletServerWithStuckWatcherDies());
+      try {
+        testTabletServerWithStuckWatcherDies();
+        fail("Test did not time out.");
+      } catch (IllegalStateException e) {
+        if (!e.getMessage().contains("Timeout exceeded")) {
+          fail("Unexpected exception: " + e.getMessage());
+        }
+      }
     }
   }
 
@@ -164,7 +177,7 @@ public boolean testTabletServerWithStuckWatcherDies() throws Exception {
       String tableName = getUniqueNames(1)[0];
       client.tableOperations().create(tableName);
 
-      // add splits to the table, which should set a watcher on the table node in zookeeper
+      // add splits to the table, which should set a StuckWatcher on the table node in zookeeper
       TreeSet<Text> splits = new TreeSet<>();
       splits.add(new Text("j"));
       splits.add(new Text("t"));