aboutsummaryrefslogtreecommitdiffstats
path: root/zkfacade
diff options
context:
space:
mode:
authorHåkon Hallingstad <hakon@yahooinc.com>2022-05-05 14:25:11 +0200
committerHåkon Hallingstad <hakon@yahooinc.com>2022-05-05 14:25:11 +0200
commit33fdf29bf12b52f9113a9e94ae29e0bedc35686d (patch)
tree6b11d7898e342c06e5ef49157161d10c6b28116f /zkfacade
parent9e4c738e9e994124d96cede78449d2d558d4977f (diff)
Remove LockStats debug
Diffstat (limited to 'zkfacade')
-rw-r--r--zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java47
-rw-r--r--zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockStats.java10
-rw-r--r--zkfacade/src/main/java/com/yahoo/vespa/curator/stats/ThreadLockStats.java8
3 files changed, 11 insertions, 54 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 a5d2397dadb..9634d2220c0 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java
@@ -9,9 +9,6 @@ import com.yahoo.vespa.curator.stats.ThreadLockStats;
import org.apache.curator.framework.recipes.locks.InterProcessLock;
import java.time.Duration;
-import java.time.Instant;
-import java.util.HashMap;
-import java.util.Map;
import java.util.concurrent.TimeUnit;
/**
@@ -24,26 +21,17 @@ import java.util.concurrent.TimeUnit;
*/
public class Lock implements Mutex {
- // TODO(hakon): Remove once debugging is done
- private final Object monitor = new Object();
- private long nextSequenceNumber = 0;
- private final Map<Long, Long> reentriesByThreadId = new HashMap<>();
- private final Instant created = Instant.now();
- private Curator curator;
-
private final InterProcessLock mutex;
private final String lockPath;
public Lock(String lockPath, Curator curator) {
this(lockPath, curator.createMutex(lockPath));
- this.curator = curator;
}
/** Public for testing only */
public Lock(String lockPath, InterProcessLock mutex) {
this.lockPath = lockPath;
this.mutex = mutex;
- this.curator = null;
}
/** Take the lock with the given timeout. This may be called multiple times from the same thread - each matched by a close */
@@ -65,38 +53,7 @@ public class Lock implements Mutex {
" to acquire lock '" + lockPath + "'");
}
- invoke(+1L, (lockPath, debug) -> threadLockStats.lockAcquired(debug), lockPath);
- }
-
- @FunctionalInterface
- private interface BiConsumer2 {
- void accept(String lockPath, String debug);
- }
-
- // TODO(hakon): Remove once debugging is unnecessary
- private void invoke(long reentryCountDiff, BiConsumer2 consumer, String lockPath) {
- long threadId = Thread.currentThread().getId();
- final long sequenceNumber;
- final Map<Long, Long> reentriesByThreadIdCopy;
- synchronized (monitor) {
- sequenceNumber = nextSequenceNumber++;
- reentriesByThreadId.merge(threadId, reentryCountDiff, (oldValue, argumentValue) -> {
- long sum = oldValue + argumentValue /* == reentryCountDiff */;
- if (sum == 0) {
- // Remove from map
- return null;
- } else {
- return sum;
- }
- });
- reentriesByThreadIdCopy = Map.copyOf(reentriesByThreadId);
- }
-
- String debug = "thread " + threadId + " Lock 0x" + Integer.toHexString(System.identityHashCode(this)) +
- "@" + created + " Curator 0x" + Integer.toHexString(System.identityHashCode(curator)) +
- " lock " + lockPath + " #" + sequenceNumber +
- ", reentries by thread ID = " + reentriesByThreadIdCopy;
- consumer.accept(lockPath, debug);
+ threadLockStats.lockAcquired();
}
@Override
@@ -105,7 +62,7 @@ public class Lock implements Mutex {
// Update metrics now before release() to avoid double-counting time in locked state.
// The lockPath must be sent down as close() may be invoked in an order not necessarily
// equal to the reverse order of acquires.
- invoke(-1L, threadLockStats::preRelease, lockPath);
+ threadLockStats.preRelease(lockPath);
try {
mutex.release();
threadLockStats.postRelease(lockPath);
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockStats.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockStats.java
index a37034b7547..0897078b94d 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockStats.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/LockStats.java
@@ -64,28 +64,28 @@ public class LockStats {
}
/** Must be invoked only after the first and non-reentry acquisition of the lock. */
- void notifyOfThreadHoldingLock(Thread currentThread, String lockPath, String debug) {
+ void notifyOfThreadHoldingLock(Thread currentThread, String lockPath) {
Thread oldThread = lockPathsHeld.put(lockPath, currentThread);
if (oldThread != null) {
getLockMetrics(lockPath).incrementAcquireWithoutReleaseCount();
logger.warning("Thread " + currentThread.getName() + " reports it has the lock on " +
lockPath + ", but thread " + oldThread.getName() +
- " has not reported it released the lock. " + debug);
+ " has not reported it released the lock");
}
}
/** Must be invoked only before the last and non-reentry release of the lock. */
- void notifyOfThreadReleasingLock(Thread currentThread, String lockPath, String debug) {
+ void notifyOfThreadReleasingLock(Thread currentThread, String lockPath) {
Thread oldThread = lockPathsHeld.remove(lockPath);
if (oldThread == null) {
getLockMetrics(lockPath).incrementNakedReleaseCount();
logger.warning("Thread " + currentThread.getName() + " is releasing the lock " + lockPath +
- ", but nobody owns that lock. " + debug);
+ ", but nobody owns that lock");
} else if (oldThread != currentThread) {
getLockMetrics(lockPath).incrementForeignReleaseCount();
logger.warning("Thread " + currentThread.getName() +
" is releasing the lock " + lockPath + ", but it was owned by thread " +
- oldThread.getName() + ". " + debug);
+ oldThread.getName());
}
}
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/ThreadLockStats.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/ThreadLockStats.java
index 24305539be5..f0b26e61870 100644
--- a/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/ThreadLockStats.java
+++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/stats/ThreadLockStats.java
@@ -98,7 +98,7 @@ public class ThreadLockStats {
}
/** Mutable method (see class doc) */
- public void lockAcquired(String debug) {
+ public void lockAcquired() {
withLastLockAttempt(lockAttempt -> {
// Note on the order of lockAcquired() vs notifyOfThreadHoldingLock(): When the latter is
// invoked, other threads may query e.g. isAcquired() on the lockAttempt, which would
@@ -107,18 +107,18 @@ public class ThreadLockStats {
lockAttempt.lockAcquired();
if (!lockAttempt.isReentry()) {
- LockStats.getGlobal().notifyOfThreadHoldingLock(thread, lockAttempt.getLockPath(), debug);
+ LockStats.getGlobal().notifyOfThreadHoldingLock(thread, lockAttempt.getLockPath());
}
});
}
/** Mutable method (see class doc) */
- public void preRelease(String path, String debug) {
+ public void preRelease(String path) {
withLastLockAttemptFor(path, lockAttempt -> {
// Note on the order of these two statement: Same concerns apply here as in lockAcquired().
if (!lockAttempt.isReentry()) {
- LockStats.getGlobal().notifyOfThreadReleasingLock(thread, lockAttempt.getLockPath(), debug);
+ LockStats.getGlobal().notifyOfThreadReleasingLock(thread, lockAttempt.getLockPath());
}
lockAttempt.preRelease();