Handle IllegalStateException in serviceLock

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

@@ -155,7 +155,11 @@ public synchronized boolean tryLock(LockWatcher lw, byte[] data)
 
     LockWatcherWrapper lww = new LockWatcherWrapper(lw);
 
-    lock(lww, data);
+    try {
+      lock(lww, data);
+    } catch (IllegalStateException e) {
+      lww.failedToAcquireLock(e);
+    }
 
     if (lww.acquiredLock) {
       return true;

core/src/test/java/org/apache/accumulo/core/fate/zookeeper/ServiceLockTest.java

@@ -18,18 +18,40 @@
  */
 package org.apache.accumulo.core.fate.zookeeper;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.easymock.EasyMock.anyObject;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.eq;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.UUID;
 
+import org.apache.accumulo.core.fate.zookeeper.ServiceLock.AccumuloLockWatcher;
+import org.apache.accumulo.core.fate.zookeeper.ServiceLock.LockWatcher;
+import org.apache.accumulo.core.fate.zookeeper.ServiceLock.ServiceLockPath;
+import org.apache.accumulo.core.util.ServerServices;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.data.Stat;
+import org.easymock.EasyMock;
 import org.junit.jupiter.api.Test;
 
 public class ServiceLockTest {
 
+  private final String ZPATH = "/test";
+  private final ServiceLockPath path = ServiceLock.path(ZPATH);
+
   @Test
   public void testSortAndFindLowestPrevPrefix() {
     List<String> children = new ArrayList<>();
@@ -103,4 +125,73 @@ public void uuidTest() {
     assertTrue(candidate.contains(uuid));
     assertTrue(candidate.contains(seq));
   }
+
+  @Test
+  public void determineLockOwnershipTest() throws Exception {
+    final long ephemeralOwner = 123456789L;
+    Stat existsStat = new Stat();
+    existsStat.setEphemeralOwner(ephemeralOwner);
+
+    ZooKeeper zk = createMock(ZooKeeper.class);
+    expect(zk.exists(eq(ZPATH), anyObject(ServiceLock.class))).andReturn(existsStat);
+
+    replay(zk);
+
+    ServiceLock serviceLock = new ServiceLock(zk, path, UUID.randomUUID());
+    AccumuloLockWatcher mockLockWatcher = EasyMock.createMock(AccumuloLockWatcher.class);
+
+    Method determineLockOwnershipMethod = ServiceLock.class
+        .getDeclaredMethod("determineLockOwnership", String.class, AccumuloLockWatcher.class);
+    determineLockOwnershipMethod.setAccessible(true);
+
+    String testEphemeralNode = "zlock#test-uuid#0000000001";
+
+    InvocationTargetException exception = assertThrows(InvocationTargetException.class, () -> {
+      determineLockOwnershipMethod.invoke(serviceLock, testEphemeralNode, mockLockWatcher);
+    });
+
+    assertEquals(exception.getTargetException().getClass(), IllegalStateException.class);
+    assertTrue(exception.getTargetException().getMessage()
+        .contains("Called determineLockOwnership() when ephemeralNodeName == null"));
+
+    verify(zk);
+  }
+
+  @Test
+  public void tryLockCleanupOnFailureTest() throws Exception {
+    LockWatcher mockLockWatcher = EasyMock.createMock(LockWatcher.class);
+
+    final long ephemeralOwner = 123456789L;
+    final String zlock = "zlock#test-uuid#0000000001";
+    Stat existsStat = new Stat();
+    existsStat.setEphemeralOwner(ephemeralOwner);
+
+    ZooKeeper zk = createMock(ZooKeeper.class);
+    expect(zk.exists(eq(ZPATH), anyObject(ServiceLock.class))).andReturn(existsStat);
+    expect(zk.getChildren(eq(ZPATH + "/" + zlock), anyObject(null))).andReturn(List.of());
+    zk.delete(eq(ZPATH + "/" + zlock), eq(-1));
+    expectLastCall();
+
+    replay(zk);
+
+    ServiceLock serviceLock = new ServiceLock(zk, path, UUID.randomUUID());
+
+    Field createdNodeNameField = ServiceLock.class.getDeclaredField("createdNodeName");
+    createdNodeNameField.setAccessible(true);
+    createdNodeNameField.set(serviceLock, zlock);
+
+    Field lockWasAcquiredField = ServiceLock.class.getDeclaredField("lockWasAcquired");
+    lockWasAcquiredField.setAccessible(true);
+    lockWasAcquiredField.set(serviceLock, false);
+
+    serviceLock.tryLock(mockLockWatcher,
+        new ServerServices("9443", ServerServices.Service.COMPACTOR_CLIENT).toString()
+            .getBytes(UTF_8));
+
+    createdNodeNameField.setAccessible(true);
+    // Cast to string since field should be set to null
+    String nullCreatedNodeName = (String) createdNodeNameField.get(serviceLock);
+    assertNull(nullCreatedNodeName, "createdNodeName was not cleaned up after failed lock");
+    verify(zk);
+  }
 }