diff options
author | Håkon Hallingstad <hakon@verizonmedia.com> | 2020-09-25 12:18:27 +0200 |
---|---|---|
committer | Håkon Hallingstad <hakon@verizonmedia.com> | 2020-09-25 12:18:27 +0200 |
commit | afbbe9c191ebdaf9ab8d84c727d53825f69fe64f (patch) | |
tree | 33dced7023f3c6cfba58df26df7aefd9db34f62f /zkfacade | |
parent | 032d45a7cdab3ab4f358ac0371a5d6b0eb57415a (diff) |
Remove reentrant lock no longer needed
Diffstat (limited to 'zkfacade')
4 files changed, 30 insertions, 50 deletions
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java index da6e3109695..7fb6b88cf99 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java @@ -9,7 +9,6 @@ import org.apache.curator.framework.recipes.locks.InterProcessLock; import java.time.Duration; import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.ReentrantLock; /** * A cluster-wide re-entrant mutex which is released on (the last symmetric) close. @@ -21,13 +20,11 @@ import java.util.concurrent.locks.ReentrantLock; */ public class Lock implements Mutex { - private final ReentrantLock lock; private final InterProcessLock mutex; private final String lockPath; public Lock(String lockPath, Curator curator) { this.lockPath = lockPath; - this.lock = new ReentrantLock(true); mutex = curator.createMutex(lockPath); } @@ -35,37 +32,26 @@ public class Lock implements Mutex { public void acquire(Duration timeout) throws UncheckedTimeoutException { ThreadLockInfo threadLockInfo = getThreadLockInfo(); threadLockInfo.invokingAcquire(timeout); - try { - if ( ! mutex.acquire(timeout.toMillis(), TimeUnit.MILLISECONDS)) { - threadLockInfo.acquireTimedOut(); - - throw new UncheckedTimeoutException("Timed out after waiting " + timeout + - " to acquire lock '" + lockPath + "'"); - } - threadLockInfo.lockAcquired(); - if ( ! lock.tryLock()) { // Should be available to only this thread, while holding the above mutex. - release(); - threadLockInfo.failedToAcquireReentrantLock(); - throw new IllegalStateException("InterProcessMutex acquired, but guarded lock held by someone else, for lock '" + lockPath + "'"); - } - } - catch (UncheckedTimeoutException | IllegalStateException e) { - throw e; - } - catch (Exception e) { + final boolean acquired; + try { + acquired = mutex.acquire(timeout.toMillis(), TimeUnit.MILLISECONDS); + } catch (Exception e) { + threadLockInfo.acquireFailed(); throw new RuntimeException("Exception acquiring lock '" + lockPath + "'", e); } + + if (!acquired) { + threadLockInfo.acquireTimedOut(); + throw new UncheckedTimeoutException("Timed out after waiting " + timeout + + " to acquire lock '" + lockPath + "'"); + } + threadLockInfo.lockAcquired(); } @Override public void close() { - try { - lock.unlock(); - } - finally { - release(); - } + release(); } private void release() { @@ -79,7 +65,7 @@ public class Lock implements Mutex { } private ThreadLockInfo getThreadLockInfo() { - return ThreadLockInfo.getCurrentThreadLockInfo(lockPath, lock); + return ThreadLockInfo.getCurrentThreadLockInfo(lockPath); } } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockCounters.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockCounters.java index 9052db5ce5a..4d1b48ef7f3 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockCounters.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockCounters.java @@ -11,20 +11,20 @@ import java.util.concurrent.atomic.AtomicInteger; public class LockCounters { final AtomicInteger invokeAcquireCount = new AtomicInteger(0); final AtomicInteger inCriticalRegionCount = new AtomicInteger(0); + final AtomicInteger acquireFailedCount = new AtomicInteger(0); final AtomicInteger acquireTimedOutCount = new AtomicInteger(0); final AtomicInteger lockAcquiredCount = new AtomicInteger(0); final AtomicInteger locksReleasedCount = new AtomicInteger(0); - final AtomicInteger failedToAcquireReentrantLockCount = new AtomicInteger(0); final AtomicInteger noLocksErrorCount = new AtomicInteger(0); final AtomicInteger timeoutOnReentrancyErrorCount = new AtomicInteger(0); public int invokeAcquireCount() { return invokeAcquireCount.get(); } public int inCriticalRegionCount() { return inCriticalRegionCount.get(); } + public int acquireFailedCount() { return acquireFailedCount.get(); } public int acquireTimedOutCount() { return acquireTimedOutCount.get(); } public int lockAcquiredCount() { return lockAcquiredCount.get(); } public int locksReleasedCount() { return locksReleasedCount.get(); } - public int failedToAcquireReentrantLockCount() { return failedToAcquireReentrantLockCount.get(); } public int noLocksErrorCount() { return noLocksErrorCount.get(); } public int timeoutOnReentrancyErrorCount() { return timeoutOnReentrancyErrorCount.get(); } } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockInfo.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockInfo.java index 37481a6b1c7..1e46b1cf668 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockInfo.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockInfo.java @@ -17,7 +17,6 @@ public class LockInfo { private final Thread thread; private final String lockPath; - private final int threadHoldCountOnAcquire; private final Instant acquireInstant; private final Duration timeout; @@ -25,12 +24,12 @@ public class LockInfo { private volatile Optional<Instant> terminalStateInstant = Optional.empty(); private volatile Optional<String> stackTrace = Optional.empty(); - public static LockInfo invokingAcquire(Thread thread, String lockPath, int holdCount, Duration timeout) { - return new LockInfo(thread, lockPath, holdCount, timeout); + public static LockInfo invokingAcquire(Thread thread, String lockPath, Duration timeout) { + return new LockInfo(thread, lockPath, timeout); } public enum LockState { - ACQUIRING(false), TIMED_OUT(true), ACQUIRED(false), FAILED_TO_REENTER(true), RELEASED(true); + ACQUIRING(false), ACQUIRE_FAILED(true), TIMED_OUT(true), ACQUIRED(false), RELEASED(true); private final boolean terminal; @@ -41,17 +40,15 @@ public class LockInfo { private volatile LockState lockState = LockState.ACQUIRING; - private LockInfo(Thread thread, String lockPath, int threadHoldCountOnAcquire, Duration timeout) { + private LockInfo(Thread thread, String lockPath, Duration timeout) { this.thread = thread; this.lockPath = lockPath; - this.threadHoldCountOnAcquire = threadHoldCountOnAcquire; this.acquireInstant = Instant.now(); this.timeout = timeout; } public String getThreadName() { return thread.getName(); } public String getLockPath() { return lockPath; } - public int getThreadHoldCountOnAcquire() { return threadHoldCountOnAcquire; } public Instant getTimeAcquiredWasInvoked() { return acquireInstant; } public Duration getAcquireTimeout() { return timeout; } public LockState getLockState() { return lockState; } @@ -100,8 +97,8 @@ public class LockInfo { this.stackTrace = Optional.of(stackTrace.toString()); } + void acquireFailed() { setTerminalState(LockState.ACQUIRE_FAILED); } void timedOut() { setTerminalState(LockState.TIMED_OUT); } - void failedToAcquireReentrantLock() { setTerminalState(LockState.FAILED_TO_REENTER); } void released() { setTerminalState(LockState.RELEASED); } void lockAcquired() { diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/ThreadLockInfo.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/ThreadLockInfo.java index 0ca9377d360..9f92b6444be 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/ThreadLockInfo.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/ThreadLockInfo.java @@ -12,7 +12,6 @@ import java.util.PriorityQueue; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; /** @@ -37,7 +36,6 @@ public class ThreadLockInfo { private final Thread thread; private final String lockPath; - private final ReentrantLock lock; private final LockCounters lockCountersForPath; /** The locks are reentrant so there may be more than 1 lock for this thread. */ @@ -54,19 +52,18 @@ public class ThreadLockInfo { } /** Returns the per-thread singleton ThreadLockInfo. */ - public static ThreadLockInfo getCurrentThreadLockInfo(String lockPath, ReentrantLock lock) { + public static ThreadLockInfo getCurrentThreadLockInfo(String lockPath) { return locks.computeIfAbsent( Thread.currentThread(), currentThread -> { LockCounters lockCounters = countersByLockPath.computeIfAbsent(lockPath, ignored -> new LockCounters()); - return new ThreadLockInfo(currentThread, lockPath, lock, lockCounters); + return new ThreadLockInfo(currentThread, lockPath, lockCounters); }); } - ThreadLockInfo(Thread currentThread, String lockPath, ReentrantLock lock, LockCounters lockCountersForPath) { + ThreadLockInfo(Thread currentThread, String lockPath, LockCounters lockCountersForPath) { this.thread = currentThread; this.lockPath = lockPath; - this.lock = lock; this.lockCountersForPath = lockCountersForPath; } @@ -78,7 +75,12 @@ public class ThreadLockInfo { public void invokingAcquire(Duration timeout) { lockCountersForPath.invokeAcquireCount.incrementAndGet(); lockCountersForPath.inCriticalRegionCount.incrementAndGet(); - lockInfos.add(LockInfo.invokingAcquire(thread, lockPath, lock.getHoldCount(), timeout)); + lockInfos.add(LockInfo.invokingAcquire(thread, lockPath, timeout)); + } + + /** Mutable method (see class doc) */ + public void acquireFailed() { + removeLastLockInfo(lockCountersForPath.acquireFailedCount, LockInfo::acquireFailed); } /** Mutable method (see class doc) */ @@ -98,11 +100,6 @@ public class ThreadLockInfo { } /** Mutable method (see class doc) */ - public void failedToAcquireReentrantLock() { - removeLastLockInfo(lockCountersForPath.failedToAcquireReentrantLockCount, LockInfo::failedToAcquireReentrantLock); - } - - /** Mutable method (see class doc) */ public void lockReleased() { removeLastLockInfo(lockCountersForPath.locksReleasedCount, LockInfo::released); } |