Make stacktraces for active locks during request handling

2 files changed, 30 insertions, 26 deletions

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 6e58c1ea81b..c9f3ec4b757 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
@@ -9,13 +9,14 @@ import java.util.stream.Stream;
 /**
  * Information about a lock.
  *
- * <p>Should be mutated by a single thread.  Other threads may see an inconsistent state of this instance.</p>
+ * <p>Should be mutated by a single thread, except {@link #fillStackTrace()} which can be
+ * invoked by any threads.  Other threads may see an inconsistent state of this instance.</p>
  *
  * @author hakon
  */
 public class LockInfo {
 
-    private final String threadName;
+    private final Thread thread;
     private final String lockPath;
     private final int threadHoldCountOnAcquire;
     private final Instant acquireInstant;
@@ -25,8 +26,8 @@ public class LockInfo {
     private volatile Optional<Instant> terminalStateInstant = Optional.empty();
     private volatile Optional<String> stackTrace = Optional.empty();
 
-    public static LockInfo invokingAcquire(String threadName, String lockPath, int holdCount, Duration timeout) {
-        return new LockInfo(threadName, lockPath, holdCount, timeout);
+    public static LockInfo invokingAcquire(Thread thread, String lockPath, int holdCount, Duration timeout) {
+        return new LockInfo(thread, lockPath, holdCount, timeout);
     }
 
     public enum LockState {
@@ -41,15 +42,15 @@ public class LockInfo {
 
     private volatile LockState lockState = LockState.ACQUIRING;
 
-    private LockInfo(String threadName, String lockPath, int threadHoldCountOnAcquire, Duration timeout) {
-        this.threadName = threadName;
+    private LockInfo(Thread thread, String lockPath, int threadHoldCountOnAcquire, Duration timeout) {
+        this.thread = thread;
         this.lockPath = lockPath;
         this.threadHoldCountOnAcquire = threadHoldCountOnAcquire;
         this.acquireInstant = Instant.now();
         this.timeout = timeout;
     }
 
-    public String getThreadName() { return threadName; }
+    public String getThreadName() { return thread.getName(); }
     public String getLockPath() { return lockPath; }
     public int getThreadHoldCountOnAcquire() { return threadHoldCountOnAcquire; }
     public Instant getTimeAcquiredWasInvoked() { return acquireInstant; }
@@ -64,28 +65,17 @@ public class LockInfo {
         return terminalStateInstant.map(instant -> Duration.between(acquireInstant, instant)).orElse(Duration.ZERO);
     }
 
-    void timedOut() { setTerminalState(LockState.TIMED_OUT); }
-    void failedToAcquireReentrantLock() { setTerminalState(LockState.FAILED_TO_REENTER); }
-    void released() { setTerminalState(LockState.RELEASED); }
-
-    void lockAcquired() {
-        lockState = LockState.ACQUIRED;
-        lockAcquiredInstant = Optional.of(Instant.now());
-    }
-
-    void setTerminalState(LockState terminalState) {
-        lockState = terminalState;
-        terminalStateInstant = Optional.of(Instant.now());
-    }
-
     /** Fill in the stack trace starting at the caller's stack frame. */
-    void fillStackTrace() {
-        final int elementsToIgnore = 1;
+    public void fillStackTrace() {
+        // This method is public. If invoked concurrently, the this.stackTrace may be updated twice,
+        // which is fine.
+
+        if (this.stackTrace.isPresent()) return;
 
         var stackTrace = new StringBuilder();
 
-        StackTraceElement[] elements = Thread.currentThread().getStackTrace();
-        for (int i = elementsToIgnore; i < elements.length; ++i) {
+        StackTraceElement[] elements = thread.getStackTrace();
+        for (int i = 0; i < elements.length && i < 20; ++i) {
             Stream.of(elements).forEach(element ->
                     stackTrace.append(element.getClassName())
                             .append('(')
@@ -97,4 +87,18 @@ public class LockInfo {
 
         this.stackTrace = Optional.of(stackTrace.toString());
     }
+
+    void timedOut() { setTerminalState(LockState.TIMED_OUT); }
+    void failedToAcquireReentrantLock() { setTerminalState(LockState.FAILED_TO_REENTER); }
+    void released() { setTerminalState(LockState.RELEASED); }
+
+    void lockAcquired() {
+        lockState = LockState.ACQUIRED;
+        lockAcquiredInstant = Optional.of(Instant.now());
+    }
+
+    void setTerminalState(LockState terminalState) {
+        lockState = terminalState;
+        terminalStateInstant = Optional.of(Instant.now());
+    }
 }
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 5e77ec76290..2e4672841bf 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
@@ -77,7 +77,7 @@ public class ThreadLockInfo {
     public void invokingAcquire(Duration timeout) {
         lockCountersForPath.invokeAcquireCount.incrementAndGet();
         lockCountersForPath.inCriticalRegionCount.incrementAndGet();
-        lockInfos.add(LockInfo.invokingAcquire(getThreadName(), lockPath, lock.getHoldCount(), timeout));
+        lockInfos.add(LockInfo.invokingAcquire(thread, lockPath, lock.getHoldCount(), timeout));
     }
 
     /** Mutable method (see class doc) */