diff options
author | Håkon Hallingstad <hakon@oath.com> | 2018-06-14 16:04:09 +0200 |
---|---|---|
committer | Håkon Hallingstad <hakon@oath.com> | 2018-06-14 16:04:09 +0200 |
commit | 60773276ccee8a0e181570a59873c3ace4b8dff5 (patch) | |
tree | 17ed47fbaaa73005c068109e492ca1ff33ae8c95 | |
parent | 8b47b122003f768978acdc22f572e8db31c1e36e (diff) |
Only count time with no threads available
Found this edge-case with logAndDie in ThreadPoolProvider:
- A Runnable executes and finishes at time T0
- No Runnables execute for a long time t > maxThreadExecutionTimeSeconds
- At time T1 = T0 + t, a bursts of Runnables N > maxthreads arrives such that
the (maxthreads+1) Runnable will be tried executed and rejected before any
of the first maxthreads Runnables finish.
- Then, logOrDie will be called immediately.
That is, a sufficiently silent period followed by a burst of Runnables cause
the process to die.
This PR will make sure that the process dies only if
maxThreadExecutionTimeSeconds seconds pass with no available threads, like the
documentation says.
I found this as I was looking for reasons why the Cluster Controller have had
logAndDie problems. The CC has maxthreads of 10, and was getting bursts of 6-8
of a particular request type (reduced in PR 6207).
-rw-r--r-- | container-core/src/main/java/com/yahoo/container/handler/ThreadPoolProvider.java | 18 |
1 files changed, 9 insertions, 9 deletions
diff --git a/container-core/src/main/java/com/yahoo/container/handler/ThreadPoolProvider.java b/container-core/src/main/java/com/yahoo/container/handler/ThreadPoolProvider.java index a9d6521e3eb..4bc1063ccbd 100644 --- a/container-core/src/main/java/com/yahoo/container/handler/ThreadPoolProvider.java +++ b/container-core/src/main/java/com/yahoo/container/handler/ThreadPoolProvider.java @@ -1,6 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.container.handler; +import com.google.common.util.concurrent.ForwardingExecutorService; +import com.google.inject.Inject; +import com.yahoo.component.AbstractComponent; +import com.yahoo.concurrent.ThreadFactoryFactory; +import com.yahoo.container.di.componentgraph.Provider; +import com.yahoo.container.protect.ProcessTerminator; +import com.yahoo.jdisc.Metric; + import java.util.concurrent.BlockingQueue; import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; @@ -12,14 +20,6 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; -import com.google.inject.Inject; -import com.yahoo.container.protect.ProcessTerminator; -import com.google.common.util.concurrent.ForwardingExecutorService; -import com.yahoo.component.AbstractComponent; -import com.yahoo.concurrent.ThreadFactoryFactory; -import com.yahoo.container.di.componentgraph.Provider; -import com.yahoo.jdisc.Metric; - /** * A configurable thread pool provider. This provides the worker threads used for normal request processing. * Request an Executor injected in your component constructor if you want to use it. @@ -177,13 +177,13 @@ public class ThreadPoolProvider extends AbstractComponent implements Provider<Ex @Override protected void beforeExecute(Thread t, Runnable r) { super.beforeExecute(t, r); + lastThreadReturnTimeMillis = System.currentTimeMillis(); startedCount.incrementAndGet(); } @Override protected void afterExecute(Runnable r, Throwable t) { super.afterExecute(r, t); - lastThreadReturnTimeMillis = System.currentTimeMillis(); completedCount.incrementAndGet(); } |