diff options
-rw-r--r-- | config-model/src/main/java/com/yahoo/vespa/model/container/ContainerThreadpool.java | 4 | ||||
-rw-r--r-- | container-core/src/main/java/com/yahoo/container/handler/ThreadPoolProvider.java | 11 | ||||
-rw-r--r-- | container-core/src/main/java/com/yahoo/container/handler/threadpool/ContainerThreadpoolImpl.java (renamed from container-core/src/main/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadpool.java) | 12 | ||||
-rw-r--r-- | container-core/src/test/java/com/yahoo/container/handler/threadpool/ContainerThreadPoolImplTest.java (renamed from container-core/src/test/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadPoolTest.java) | 8 | ||||
-rw-r--r-- | node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java | 21 | ||||
-rw-r--r-- | node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java | 8 | ||||
-rw-r--r-- | node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java | 3 | ||||
-rw-r--r-- | searchlib/abi-spec.json | 10 |
8 files changed, 51 insertions, 26 deletions
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerThreadpool.java b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerThreadpool.java index 7111a88fc01..489e4cc135a 100644 --- a/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerThreadpool.java +++ b/config-model/src/main/java/com/yahoo/vespa/model/container/ContainerThreadpool.java @@ -4,7 +4,7 @@ package com.yahoo.vespa.model.container; import com.yahoo.container.bundle.BundleInstantiationSpecification; import com.yahoo.container.handler.threadpool.ContainerThreadPool; import com.yahoo.container.handler.threadpool.ContainerThreadpoolConfig; -import com.yahoo.container.handler.threadpool.DefaultContainerThreadpool; +import com.yahoo.container.handler.threadpool.ContainerThreadpoolImpl; import com.yahoo.osgi.provider.model.ComponentModel; import com.yahoo.text.XML; import com.yahoo.vespa.model.container.component.SimpleComponent; @@ -26,7 +26,7 @@ public class ContainerThreadpool extends SimpleComponent implements ContainerThr super(new ComponentModel( BundleInstantiationSpecification.getFromStrings( "threadpool@" + name, - DefaultContainerThreadpool.class.getName(), + ContainerThreadpoolImpl.class.getName(), null))); this.name = name; this.userOptions = userOptions; 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 bc3c35cb78e..1818a3d97b4 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 @@ -6,15 +6,16 @@ import com.yahoo.component.AbstractComponent; import com.yahoo.container.di.componentgraph.Provider; import com.yahoo.container.handler.threadpool.ContainerThreadPool; import com.yahoo.container.handler.threadpool.ContainerThreadpoolConfig; -import com.yahoo.container.handler.threadpool.DefaultContainerThreadpool; +import com.yahoo.container.handler.threadpool.ContainerThreadpoolImpl; import com.yahoo.container.protect.ProcessTerminator; import com.yahoo.jdisc.Metric; import java.util.concurrent.Executor; /** - * 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. + * A configurable thread pool provider for the jdisc default threadpool. + * This provides the worker threads used for normal request processing. + * Request an {@link Executor} injected in your component constructor if you want to use it. * * @author Steinar Knutsen * @author baldersheim @@ -26,11 +27,11 @@ public class ThreadPoolProvider extends AbstractComponent implements Provider<Ex @Inject public ThreadPoolProvider(ThreadpoolConfig config, Metric metric) { - this.threadpool = new DefaultContainerThreadpool(translateConfig(config), metric); + this.threadpool = new ContainerThreadpoolImpl(translateConfig(config), metric); } public ThreadPoolProvider(ThreadpoolConfig config, Metric metric, ProcessTerminator processTerminator) { - this.threadpool = new DefaultContainerThreadpool(translateConfig(config), metric, processTerminator); + this.threadpool = new ContainerThreadpoolImpl(translateConfig(config), metric, processTerminator); } /** diff --git a/container-core/src/main/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadpool.java b/container-core/src/main/java/com/yahoo/container/handler/threadpool/ContainerThreadpoolImpl.java index 638336e51d8..73845c13fe8 100644 --- a/container-core/src/main/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadpool.java +++ b/container-core/src/main/java/com/yahoo/container/handler/threadpool/ContainerThreadpoolImpl.java @@ -22,25 +22,25 @@ import java.util.logging.Logger; * @author bratseth * @author bjorncs */ -public class DefaultContainerThreadpool extends AbstractComponent implements AutoCloseable, ContainerThreadPool { +public class ContainerThreadpoolImpl extends AbstractComponent implements AutoCloseable, ContainerThreadPool { - private static final Logger log = Logger.getLogger(DefaultContainerThreadpool.class.getName()); + private static final Logger log = Logger.getLogger(ContainerThreadpoolImpl.class.getName()); private static final int MIN_QUEUE_SIZE = 650; private static final int MIN_THREADS_WHEN_SCALE_FACTOR = 8; private final ExecutorServiceWrapper threadpool; @Inject - public DefaultContainerThreadpool(ContainerThreadpoolConfig config, Metric metric) { + public ContainerThreadpoolImpl(ContainerThreadpoolConfig config, Metric metric) { this(config, metric, new ProcessTerminator()); } - public DefaultContainerThreadpool(ContainerThreadpoolConfig config, Metric metric, ProcessTerminator processTerminator) { + public ContainerThreadpoolImpl(ContainerThreadpoolConfig config, Metric metric, ProcessTerminator processTerminator) { this(config, metric, processTerminator, Runtime.getRuntime().availableProcessors()); } - DefaultContainerThreadpool(ContainerThreadpoolConfig config, Metric metric, ProcessTerminator processTerminator, - int cpus) { + ContainerThreadpoolImpl(ContainerThreadpoolConfig config, Metric metric, ProcessTerminator processTerminator, + int cpus) { String name = config.name(); int maxThreads = maxThreads(config, cpus); int minThreads = minThreads(config, maxThreads, cpus); diff --git a/container-core/src/test/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadPoolTest.java b/container-core/src/test/java/com/yahoo/container/handler/threadpool/ContainerThreadPoolImplTest.java index b56d89cafb3..536f7f599f2 100644 --- a/container-core/src/test/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadPoolTest.java +++ b/container-core/src/test/java/com/yahoo/container/handler/threadpool/ContainerThreadPoolImplTest.java @@ -20,7 +20,7 @@ import static org.junit.Assert.fail; * @author Steinar Knutsen * @author bjorncs */ -public class DefaultContainerThreadPoolTest { +public class ContainerThreadPoolImplTest { private static final int CPUS = 16; @@ -28,7 +28,7 @@ public class DefaultContainerThreadPoolTest { public final void testThreadPool() throws InterruptedException { Metric metrics = new MetricMock(); ContainerThreadpoolConfig config = new ContainerThreadpoolConfig(new ContainerThreadpoolConfig.Builder().maxThreads(1)); - ContainerThreadPool threadPool = new DefaultContainerThreadpool(config, metrics); + ContainerThreadPool threadPool = new ContainerThreadpoolImpl(config, metrics); Executor exec = threadPool.executor(); Tuple2<Receiver.MessageState, Boolean> reply; FlipIt command = new FlipIt(); @@ -66,7 +66,7 @@ public class DefaultContainerThreadPoolTest { .maxThreads(maxThreads) .minThreads(maxThreads) .queueSize(queueSize)); - ContainerThreadPool threadPool = new DefaultContainerThreadpool( + ContainerThreadPool threadPool = new ContainerThreadpoolImpl( config, metric, new MockProcessTerminator(), CPUS); ExecutorServiceWrapper wrapper = (ExecutorServiceWrapper) threadPool.executor(); WorkerCompletionTimingThreadPoolExecutor executor = (WorkerCompletionTimingThreadPoolExecutor)wrapper.delegate(); @@ -128,7 +128,7 @@ public class DefaultContainerThreadPoolTest { .maxThreadExecutionTimeSeconds(1)); MockProcessTerminator terminator = new MockProcessTerminator(); Metric metrics = new MetricMock(); - ContainerThreadPool threadPool = new DefaultContainerThreadpool(config, metrics, terminator); + ContainerThreadPool threadPool = new ContainerThreadpoolImpl(config, metrics, terminator); // No dying when threads hang shorter than max thread execution time threadPool.executor().execute(new Hang(500)); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java index d544ea76983..3aa09b1b667 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/Nodes.java @@ -773,12 +773,15 @@ public class Nodes { public Mutex lockUnallocated() { return db.lockInactive(); } /** Returns the unallocated/application lock, and the node acquired under that lock. */ - public Optional<NodeMutex> lockAndGet(Node node) { + public Optional<NodeMutex> lockAndGet(Node node) { return lockAndGet(node, Optional.empty()); } + + /** Returns the unallocated/application lock, and the node acquired under that lock. */ + public Optional<NodeMutex> lockAndGet(Node node, Optional<Duration> timeout) { Node staleNode = node; final int maxRetries = 4; for (int i = 0; i < maxRetries; ++i) { - Mutex lockToClose = lock(staleNode); + Mutex lockToClose = timeout.isPresent() ? lock(staleNode, timeout.get()) : lock(staleNode); try { // As an optimization we first try finding the node in the same state Optional<Node> freshNode = node(staleNode.hostname(), staleNode.state()); @@ -813,6 +816,11 @@ public class Nodes { } /** Returns the unallocated/application lock, and the node acquired under that lock. */ + public Optional<NodeMutex> lockAndGet(String hostname, Duration timeout) { + return node(hostname).flatMap(node -> lockAndGet(node, Optional.of(timeout))); + } + + /** Returns the unallocated/application lock, and the node acquired under that lock. */ public NodeMutex lockAndGetRequired(Node node) { return lockAndGet(node).orElseThrow(() -> new NoSuchNodeException("No node with hostname '" + node.hostname() + "'")); } @@ -822,10 +830,19 @@ public class Nodes { return lockAndGet(hostname).orElseThrow(() -> new NoSuchNodeException("No node with hostname '" + hostname + "'")); } + /** Returns the unallocated/application lock, and the node acquired under that lock. */ + public NodeMutex lockAndGetRequired(String hostname, Duration timeout) { + return lockAndGet(hostname, timeout).orElseThrow(() -> new NoSuchNodeException("No node with hostname '" + hostname + "'")); + } + private Mutex lock(Node node) { return node.allocation().isPresent() ? lock(node.allocation().get().owner()) : lockUnallocated(); } + private Mutex lock(Node node, Duration timeout) { + return node.allocation().isPresent() ? lock(node.allocation().get().owner(), timeout) : lockUnallocated(); + } + private Node requireNode(String hostname) { return node(hostname).orElseThrow(() -> new NoSuchNodeException("No node with hostname '" + hostname + "'")); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java index a9abc352d8c..13dd458c041 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClient.java @@ -69,6 +69,7 @@ public class CuratorDatabaseClient { private static final Path firmwareCheckPath = root.append("firmwareCheck"); private static final Path archiveUrisPath = root.append("archiveUris"); + // TODO: Explain reasoning behind timeout value (why its it as high as 10 minutes?) private static final Duration defaultLockTimeout = Duration.ofMinutes(10); private final NodeSerializer nodeSerializer; @@ -319,7 +320,12 @@ public class CuratorDatabaseClient { /** Acquires the single cluster-global, reentrant lock for all non-active nodes */ public Lock lockInactive() { - return db.lock(lockPath.append("unallocatedLock"), defaultLockTimeout); + return lockInactive(defaultLockTimeout); + } + + /** Acquires the single cluster-global, reentrant lock for all non-active nodes */ + public Lock lockInactive(Duration timeout) { + return db.lock(lockPath.append("unallocatedLock"), timeout); } /** Acquires the single cluster-global, reentrant lock for active nodes of this application */ diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java index fa6c44e6851..d8e1828b10c 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/restapi/NodePatcher.java @@ -29,6 +29,7 @@ import com.yahoo.yolean.Exceptions; import java.io.InputStream; import java.time.Clock; +import java.time.Duration; import java.time.Instant; import java.util.ArrayList; import java.util.HashMap; @@ -86,7 +87,7 @@ public class NodePatcher { Map<String, Inspector> recursiveFields = Maps.filterKeys(fields, RECURSIVE_FIELDS::contains); // Patch - NodeMutex nodeMutex = nodeRepository.nodes().lockAndGetRequired(hostname); + NodeMutex nodeMutex = nodeRepository.nodes().lockAndGetRequired(hostname, Duration.ofSeconds(10)); // timeout should match the one used by clients patch(nodeMutex, regularFields, root, false); patchIpConfig(hostname, ipConfigFields); if (nodeMutex.node().type().isHost()) { diff --git a/searchlib/abi-spec.json b/searchlib/abi-spec.json index 5a534562d32..3081c88ec99 100644 --- a/searchlib/abi-spec.json +++ b/searchlib/abi-spec.json @@ -353,12 +353,9 @@ "public" ], "methods": [ - "public static com.yahoo.searchlib.rankingexpression.Reference fromIdentifier(java.lang.String)", "public void <init>(java.lang.String, com.yahoo.searchlib.rankingexpression.rule.Arguments, java.lang.String)", "public com.yahoo.searchlib.rankingexpression.rule.Arguments arguments()", "public java.lang.String output()", - "public static com.yahoo.searchlib.rankingexpression.Reference simple(java.lang.String, java.lang.String)", - "public static java.util.Optional simple(java.lang.String)", "public boolean isIdentifier()", "public boolean isSimple()", "public java.util.Optional simpleArgument()", @@ -369,6 +366,9 @@ "public java.lang.String toString()", "public java.lang.StringBuilder toString(java.lang.StringBuilder, com.yahoo.searchlib.rankingexpression.rule.SerializationContext, java.util.Deque, com.yahoo.searchlib.rankingexpression.rule.CompositeNode)", "public int compareTo(com.yahoo.searchlib.rankingexpression.Reference)", + "public static com.yahoo.searchlib.rankingexpression.Reference fromIdentifier(java.lang.String)", + "public static com.yahoo.searchlib.rankingexpression.Reference simple(java.lang.String, java.lang.String)", + "public static java.util.Optional simple(java.lang.String)", "public bridge synthetic int compareTo(java.lang.Object)" ], "fields": [] @@ -1598,7 +1598,6 @@ "public void <init>(java.lang.String, java.util.List, java.lang.String)", "public void <init>(com.yahoo.searchlib.rankingexpression.Reference)", "public java.lang.String getName()", - "public int hashCode()", "public com.yahoo.searchlib.rankingexpression.rule.Arguments getArguments()", "public com.yahoo.searchlib.rankingexpression.rule.ReferenceNode setArguments(java.util.List)", "public java.lang.String getOutput()", @@ -1608,7 +1607,8 @@ "public com.yahoo.searchlib.rankingexpression.Reference reference()", "public com.yahoo.tensor.TensorType type(com.yahoo.tensor.evaluation.TypeContext)", "public com.yahoo.searchlib.rankingexpression.evaluation.Value evaluate(com.yahoo.searchlib.rankingexpression.evaluation.Context)", - "public com.yahoo.searchlib.rankingexpression.rule.CompositeNode setChildren(java.util.List)" + "public com.yahoo.searchlib.rankingexpression.rule.CompositeNode setChildren(java.util.List)", + "public int hashCode()" ], "fields": [] }, |