diff --git a/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java b/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java index c6478baff36..164b52f514a 100644 --- a/core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ServiceLock.java +++ b/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; diff --git a/core/src/test/java/org/apache/accumulo/core/fate/zookeeper/ServiceLockTest.java b/core/src/test/java/org/apache/accumulo/core/fate/zookeeper/ServiceLockTest.java index 44b6fcd4cec..18474230723 100644 --- a/core/src/test/java/org/apache/accumulo/core/fate/zookeeper/ServiceLockTest.java +++ b/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 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); + } }