diff --git a/core/src/main/java/org/apache/accumulo/core/lock/ServiceLock.java b/core/src/main/java/org/apache/accumulo/core/lock/ServiceLock.java index 23e2b25ccf7..01941239259 100644 --- a/core/src/main/java/org/apache/accumulo/core/lock/ServiceLock.java +++ b/core/src/main/java/org/apache/accumulo/core/lock/ServiceLock.java @@ -72,26 +72,26 @@ public enum LockLossReason { LOCK_DELETED, SESSION_EXPIRED } - public interface LockWatcher { + public interface AccumuloLockWatcher { + + void acquiredLock(); + + void failedToAcquireLock(Exception e); + void lostLock(LockLossReason reason); /** * lost the ability to monitor the lock node, and its status is unknown */ void unableToMonitorLockNode(Exception e); - } - - public interface AccumuloLockWatcher extends LockWatcher { - void acquiredLock(); - void failedToAcquireLock(Exception e); } private final ServiceLockPath path; protected final ZooSession zooKeeper; private final Prefix vmLockPrefix; - private LockWatcher lockWatcher; + private AccumuloLockWatcher lockWatcher; private String lockNodeName; private volatile boolean lockWasAcquired; private volatile boolean watchingParent; @@ -112,45 +112,11 @@ public ServiceLock(ZooSession zookeeper, ServiceLockPath path, UUID uuid) { } } - private static class LockWatcherWrapper implements AccumuloLockWatcher { - - boolean acquiredLock = false; - final LockWatcher lw; - - public LockWatcherWrapper(LockWatcher lw2) { - this.lw = lw2; - } - - @Override - public void acquiredLock() { - acquiredLock = true; - } - - @Override - public void failedToAcquireLock(Exception e) { - LOG.debug("Failed to acquire lock", e); - } - - @Override - public void lostLock(LockLossReason reason) { - lw.lostLock(reason); - } - - @Override - public void unableToMonitorLockNode(Exception e) { - lw.unableToMonitorLockNode(e); - } - - } - - public synchronized boolean tryLock(LockWatcher lw, ServiceLockData lockData) + public synchronized boolean tryLock(AccumuloLockWatcher lw, ServiceLockData lockData) throws KeeperException, InterruptedException { - LockWatcherWrapper lww = new LockWatcherWrapper(lw); - - lock(lww, lockData); - - if (lww.acquiredLock) { + lock(lw, lockData); + if (isLocked()) { return true; } @@ -370,7 +336,7 @@ public void process(WatchedEvent event) { } private void lostLock(LockLossReason reason) { - LockWatcher localLw = lockWatcher; + AccumuloLockWatcher localLw = lockWatcher; lockNodeName = null; lockId = null; lockWatcher = null; @@ -538,7 +504,7 @@ public synchronized void unlock() throws InterruptedException, KeeperException { throw new IllegalStateException(); } - LockWatcher localLw = lockWatcher; + AccumuloLockWatcher localLw = lockWatcher; String localLock = lockNodeName; lockNodeName = null; diff --git a/core/src/main/java/org/apache/accumulo/core/lock/ServiceLockSupport.java b/core/src/main/java/org/apache/accumulo/core/lock/ServiceLockSupport.java index fad535fefa5..4f2c8d77d40 100644 --- a/core/src/main/java/org/apache/accumulo/core/lock/ServiceLockSupport.java +++ b/core/src/main/java/org/apache/accumulo/core/lock/ServiceLockSupport.java @@ -26,7 +26,6 @@ import org.apache.accumulo.core.fate.zookeeper.ZooUtil.NodeExistsPolicy; import org.apache.accumulo.core.lock.ServiceLock.AccumuloLockWatcher; import org.apache.accumulo.core.lock.ServiceLock.LockLossReason; -import org.apache.accumulo.core.lock.ServiceLock.LockWatcher; import org.apache.accumulo.core.lock.ServiceLockPaths.ServiceLockPath; import org.apache.accumulo.core.util.Halt; import org.apache.zookeeper.KeeperException; @@ -138,8 +137,8 @@ public boolean isLockAcquired() { return acquiredLock; } - public boolean isFailedToAcquireLock() { - return failedToAcquireLock; + public boolean cannotRetryLocking() { + return acquiredLock && !failedToAcquireLock; } } @@ -147,8 +146,9 @@ public boolean isFailedToAcquireLock() { /** * Lock Watcher used by non-HA services */ - public static class ServiceLockWatcher implements LockWatcher { + public static class ServiceLockWatcher implements AccumuloLockWatcher { + private final Logger log = LoggerFactory.getLogger(ServiceLockWatcher.class); private final Type server; private final Supplier shutdownComplete; private final Consumer lostLockAction; @@ -160,6 +160,16 @@ public ServiceLockWatcher(Type server, Supplier shutdownComplete, this.lostLockAction = lostLockAction; } + @Override + public void acquiredLock() { + LOG.debug("Acquired {} lock", server); + } + + @Override + public void failedToAcquireLock(Exception e) { + log.debug("Failed to acquire lock", e); + } + @Override public void lostLock(final LockLossReason reason) { if (shutdownComplete.get()) { diff --git a/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java b/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java index 9772d4bd9f6..503be0f56d1 100644 --- a/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java +++ b/server/compactor/src/main/java/org/apache/accumulo/compactor/Compactor.java @@ -83,7 +83,7 @@ import org.apache.accumulo.core.file.FileSKVIterator; import org.apache.accumulo.core.iteratorsImpl.system.SystemIteratorUtil; import org.apache.accumulo.core.lock.ServiceLock; -import org.apache.accumulo.core.lock.ServiceLock.LockWatcher; +import org.apache.accumulo.core.lock.ServiceLock.AccumuloLockWatcher; import org.apache.accumulo.core.lock.ServiceLockData; import org.apache.accumulo.core.lock.ServiceLockData.ServiceDescriptor; import org.apache.accumulo.core.lock.ServiceLockData.ServiceDescriptors; @@ -373,8 +373,9 @@ protected void announceExistence(HostAndPort clientAddress) getContext().getServerPaths().createCompactorPath(getResourceGroup(), clientAddress); ServiceLockSupport.createNonHaServiceLockPath(Type.COMPACTOR, zoo, path); compactorLock = new ServiceLock(getContext().getZooSession(), path, compactorId); - LockWatcher lw = new ServiceLockWatcher(Type.COMPACTOR, () -> getShutdownComplete().get(), - (type) -> getContext().getLowMemoryDetector().logGCInfo(getConfiguration())); + AccumuloLockWatcher lw = + new ServiceLockWatcher(Type.COMPACTOR, () -> getShutdownComplete().get(), + (type) -> getContext().getLowMemoryDetector().logGCInfo(getConfiguration())); try { for (int i = 0; i < 25; i++) { diff --git a/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java b/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java index bc5edfa1681..42bcac77b3a 100644 --- a/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java +++ b/server/gc/src/main/java/org/apache/accumulo/gc/SimpleGarbageCollector.java @@ -405,7 +405,7 @@ private void getZooLock(HostAndPort addr) throws KeeperException, InterruptedExc break; } - if (!gcLockWatcher.isFailedToAcquireLock()) { + if (gcLockWatcher.cannotRetryLocking()) { throw new IllegalStateException("gc lock in unknown state"); } diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java index 3eb2d5437f9..8d684cb2dff 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/Manager.java @@ -1401,7 +1401,7 @@ private ServiceLockData getManagerLock(final ServiceLockPath zManagerLoc) break; } - if (!managerLockWatcher.isFailedToAcquireLock()) { + if (managerLockWatcher.cannotRetryLocking()) { throw new IllegalStateException("manager lock in unknown state"); } diff --git a/server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java b/server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java index 070b93266d5..94425cf7d51 100644 --- a/server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java +++ b/server/monitor/src/main/java/org/apache/accumulo/monitor/Monitor.java @@ -760,7 +760,7 @@ private void getMonitorLock(HostAndPort monitorLocation) UUID zooLockUUID = UUID.randomUUID(); monitorLock = new ServiceLock(context.getZooSession(), monitorLockPath, zooLockUUID); HAServiceLockWatcher monitorLockWatcher = - new HAServiceLockWatcher(Type.MONITOR, () -> isShutdownRequested()); + new HAServiceLockWatcher(Type.MONITOR, this::isShutdownRequested); while (true) { monitorLock.lock(monitorLockWatcher, @@ -774,7 +774,7 @@ private void getMonitorLock(HostAndPort monitorLocation) break; } - if (!monitorLockWatcher.isFailedToAcquireLock()) { + if (monitorLockWatcher.cannotRetryLocking()) { throw new IllegalStateException("monitor lock in unknown state"); } diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java index 71303627227..3bc1438d6fd 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java @@ -70,7 +70,7 @@ import org.apache.accumulo.core.fate.zookeeper.ZooUtil.NodeExistsPolicy; import org.apache.accumulo.core.file.blockfile.cache.impl.BlockCacheConfiguration; import org.apache.accumulo.core.lock.ServiceLock; -import org.apache.accumulo.core.lock.ServiceLock.LockWatcher; +import org.apache.accumulo.core.lock.ServiceLock.AccumuloLockWatcher; import org.apache.accumulo.core.lock.ServiceLockData; import org.apache.accumulo.core.lock.ServiceLockData.ServiceDescriptor; import org.apache.accumulo.core.lock.ServiceLockData.ServiceDescriptors; @@ -320,8 +320,9 @@ private ServiceLock announceExistence() { ServiceLockSupport.createNonHaServiceLockPath(Type.SCAN_SERVER, zoo, zLockPath); serverLockUUID = UUID.randomUUID(); scanServerLock = new ServiceLock(getContext().getZooSession(), zLockPath, serverLockUUID); - LockWatcher lw = new ServiceLockWatcher(Type.SCAN_SERVER, () -> getShutdownComplete().get(), - (type) -> context.getLowMemoryDetector().logGCInfo(getConfiguration())); + AccumuloLockWatcher lw = + new ServiceLockWatcher(Type.SCAN_SERVER, () -> getShutdownComplete().get(), + (type) -> context.getLowMemoryDetector().logGCInfo(getConfiguration())); for (int i = 0; i < 120 / 5; i++) { zoo.putPersistentData(zLockPath.toString(), new byte[0], NodeExistsPolicy.SKIP); diff --git a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java index 43be28270d6..4d466e3391d 100644 --- a/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java +++ b/server/tserver/src/main/java/org/apache/accumulo/tserver/TabletServer.java @@ -80,7 +80,7 @@ import org.apache.accumulo.core.fate.zookeeper.ZooUtil.NodeExistsPolicy; import org.apache.accumulo.core.file.blockfile.cache.impl.BlockCacheConfiguration; import org.apache.accumulo.core.lock.ServiceLock; -import org.apache.accumulo.core.lock.ServiceLock.LockWatcher; +import org.apache.accumulo.core.lock.ServiceLock.AccumuloLockWatcher; import org.apache.accumulo.core.lock.ServiceLockData; import org.apache.accumulo.core.lock.ServiceLockData.ServiceDescriptor; import org.apache.accumulo.core.lock.ServiceLockData.ServiceDescriptors; @@ -490,8 +490,9 @@ private void announceExistence() { UUID tabletServerUUID = UUID.randomUUID(); tabletServerLock = new ServiceLock(getContext().getZooSession(), zLockPath, tabletServerUUID); - LockWatcher lw = new ServiceLockWatcher(Type.TABLET_SERVER, () -> getShutdownComplete().get(), - (type) -> context.getLowMemoryDetector().logGCInfo(getConfiguration())); + AccumuloLockWatcher lw = + new ServiceLockWatcher(Type.TABLET_SERVER, () -> getShutdownComplete().get(), + (type) -> context.getLowMemoryDetector().logGCInfo(getConfiguration())); for (int i = 0; i < 120 / 5; i++) { zoo.putPersistentData(zLockPath.toString(), new byte[0], NodeExistsPolicy.SKIP); diff --git a/test/src/main/java/org/apache/accumulo/test/fate/TestLock.java b/test/src/main/java/org/apache/accumulo/test/fate/TestLock.java index 598aed5c7e7..54b58eba4af 100644 --- a/test/src/main/java/org/apache/accumulo/test/fate/TestLock.java +++ b/test/src/main/java/org/apache/accumulo/test/fate/TestLock.java @@ -108,7 +108,7 @@ public void acquiredLock() { @Override public void failedToAcquireLock(Exception e) { - log.warn("Failed to acquire ZooKeeper lock for test, msg: " + e.getMessage()); + log.warn("Failed to acquire ZooKeeper lock for test", e); } } } diff --git a/test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java b/test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java index 3437310e348..fa48aa1f1ff 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/ZombieTServer.java @@ -33,8 +33,8 @@ import org.apache.accumulo.core.fate.zookeeper.ZooReaderWriter; import org.apache.accumulo.core.fate.zookeeper.ZooUtil.NodeExistsPolicy; import org.apache.accumulo.core.lock.ServiceLock; +import org.apache.accumulo.core.lock.ServiceLock.AccumuloLockWatcher; import org.apache.accumulo.core.lock.ServiceLock.LockLossReason; -import org.apache.accumulo.core.lock.ServiceLock.LockWatcher; import org.apache.accumulo.core.lock.ServiceLockData; import org.apache.accumulo.core.lock.ServiceLockData.ThriftService; import org.apache.accumulo.core.manager.thrift.TabletServerStatus; @@ -144,7 +144,17 @@ public static void main(String[] args) throws Exception { metricsInfo.init(MetricsInfo.serviceTags(context.getInstanceName(), "zombie.server", serverPort.address, ResourceGroupId.DEFAULT)); - LockWatcher lw = new LockWatcher() { + AccumuloLockWatcher lw = new AccumuloLockWatcher() { + + @Override + public void acquiredLock() { + + } + + @Override + public void failedToAcquireLock(Exception e) { + log.warn("Failed to acquire lock", e); + } @SuppressFBWarnings(value = "DM_EXIT", justification = "System.exit() is a bad idea here, but okay for now, since it's a test")