diff options
18 files changed, 126 insertions, 47 deletions
diff --git a/configdefinitions/src/vespa/configserver.def b/configdefinitions/src/vespa/configserver.def index f541aac4edd..19c55941988 100644 --- a/configdefinitions/src/vespa/configserver.def +++ b/configdefinitions/src/vespa/configserver.def @@ -3,7 +3,10 @@ namespace=cloud.config rpcport int default=19070 httpport int default=19071 numthreads int default=16 + +# TODO: This seems to be only used by the the status API? If so this is unnecessary duplication and potentially lying: Remove zookeepercfg string default="conf/zookeeper/zookeeper.cfg" + zookeeperserver[].hostname string zookeeperserver[].port int default=2181 # in seconds diff --git a/jdisc_core/src/main/java/com/yahoo/jdisc/Timer.java b/jdisc_core/src/main/java/com/yahoo/jdisc/Timer.java index dfe821ca0e6..c94ebc1ab93 100644 --- a/jdisc_core/src/main/java/com/yahoo/jdisc/Timer.java +++ b/jdisc_core/src/main/java/com/yahoo/jdisc/Timer.java @@ -9,7 +9,7 @@ import com.yahoo.jdisc.core.SystemTimer; * instance of this class into any component that needs to access time, instead of using * <code>System.currentTimeMillis()</code>.</p> * - * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a> + * @author Simon Thoresen */ @ImplementedBy(SystemTimer.class) public interface Timer { @@ -25,5 +25,6 @@ public interface Timer { * @return The difference, measured in milliseconds, between the current time and midnight, January 1, 1970 UTC. * @see java.util.Date */ - public long currentTimeMillis(); + long currentTimeMillis(); + } diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpHeaders.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpHeaders.java index d5cdde300f0..039966133e8 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpHeaders.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/HttpHeaders.java @@ -2,7 +2,7 @@ package com.yahoo.jdisc.http; /** - * @author <a href="mailto:anirudha@yahoo-inc.com">Anirudha Khanna</a> + * @author Anirudha Khanna */ @SuppressWarnings("UnusedDeclaration") public class HttpHeaders { diff --git a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java index d355836e205..8fc1bb26603 100644 --- a/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java +++ b/jdisc_http_service/src/main/java/com/yahoo/jdisc/http/server/jetty/HttpRequestFactory.java @@ -12,11 +12,11 @@ import java.net.URI; import java.util.Enumeration; /** - * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen Hult</a> + * @author Simon Thoresen Hult */ class HttpRequestFactory { - public static HttpRequest newJDiscRequest(final CurrentContainer container, - final HttpServletRequest servletRequest) { + + public static HttpRequest newJDiscRequest(CurrentContainer container, HttpServletRequest servletRequest) { return HttpRequest.newServerRequest( container, getUri(servletRequest), @@ -34,22 +34,18 @@ class HttpRequestFactory { } } - public static void copyHeaders(final HttpServletRequest from, - final HttpRequest to) { - for (final Enumeration<String> it = from.getHeaderNames(); it.hasMoreElements(); ) { - final String key = it.nextElement(); - for (final Enumeration<String> value = from.getHeaders(key); value.hasMoreElements(); ) { + public static void copyHeaders(HttpServletRequest from, HttpRequest to) { + for (Enumeration<String> it = from.getHeaderNames(); it.hasMoreElements(); ) { + String key = it.nextElement(); + for (Enumeration<String> value = from.getHeaders(key); value.hasMoreElements(); ) { to.headers().add(key, value.nextElement()); } } } private static String extraQuote(String queryString) { - // TODO this is just a stopgap measure, we need some sort of sane URI builder, do we have one? - String washed = null; - if (queryString == null) { - return null; - } + // TODO: Use an URI builder + if (queryString == null) return null; int toAndIncluding = -1; for (int i = 0; i < queryString.length(); ++i) { @@ -59,6 +55,7 @@ class HttpRequestFactory { toAndIncluding = i; } + String washed; if (toAndIncluding != (queryString.length() - 1)) { StringBuilder w = new StringBuilder(queryString.substring(0, toAndIncluding + 1)); for (int i = toAndIncluding + 1; i < queryString.length(); ++i) { @@ -91,8 +88,6 @@ class HttpRequestFactory { default: return null; } - } - } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/JobControl.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/JobControl.java index 7ad4d87eea9..6596d2abb1d 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/JobControl.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/JobControl.java @@ -2,7 +2,7 @@ package com.yahoo.vespa.hosted.provision.maintenance; import com.yahoo.vespa.hosted.provision.persistence.CuratorDatabaseClient; -import com.yahoo.vespa.hosted.provision.persistence.CuratorMutex; +import com.yahoo.vespa.curator.Lock; import java.util.HashSet; import java.util.Set; @@ -42,12 +42,12 @@ public class JobControl { /** Returns true if this job is not currently deactivated */ public boolean isActive(String jobSimpleClassName) { - return ! db.readInactiveJobs().contains(jobSimpleClassName); + return ! inactiveJobs().contains(jobSimpleClassName); } /** Set a job active or inactive */ public void setActive(String jobSimpleClassName, boolean active) { - try (CuratorMutex lock = db.lockInactiveJobs()) { + try (Lock lock = db.lockInactiveJobs()) { Set<String> inactiveJobs = db.readInactiveJobs(); if (active) inactiveJobs.remove(jobSimpleClassName); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java index e843fe63f09..a6e058e72f3 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabase.java @@ -5,6 +5,7 @@ import com.google.common.collect.ImmutableList; import com.yahoo.path.Path; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.curator.recipes.CuratorCounter; import com.yahoo.vespa.curator.transaction.CuratorTransaction; @@ -41,7 +42,7 @@ public class CuratorDatabase { * All keys, to allow reentrancy. * This will grow forever with the number of applications seen, but this should be too slow to be a problem. */ - private final ConcurrentHashMap<Path, CuratorMutex> locks = new ConcurrentHashMap<>(); + private final ConcurrentHashMap<Path, Lock> locks = new ConcurrentHashMap<>(); /** * Creates a curator database @@ -58,8 +59,8 @@ public class CuratorDatabase { /** Create a reentrant lock */ // Locks are not cached in the in-memory state - public CuratorMutex lock(Path path, Duration timeout) { - CuratorMutex lock = locks.computeIfAbsent(path, (pathArg) -> new CuratorMutex(pathArg.getAbsolute(), curator.framework())); + public Lock lock(Path path, Duration timeout) { + Lock lock = locks.computeIfAbsent(path, (pathArg) -> new Lock(pathArg.getAbsolute(), curator.framework())); lock.acquire(timeout); return lock; } 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 96f7bc2ceb5..2ad6a18b792 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 @@ -12,6 +12,7 @@ import com.yahoo.log.LogLevel; import com.yahoo.path.Path; import com.yahoo.transaction.NestedTransaction; import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.curator.transaction.CuratorOperations; import com.yahoo.vespa.curator.transaction.CuratorTransaction; import com.yahoo.vespa.hosted.provision.Node; @@ -48,7 +49,6 @@ public class CuratorDatabaseClient { private static final Duration defaultLockTimeout = Duration.ofMinutes(1); private final NodeSerializer nodeSerializer; - private final StringSetSerializer stringSetSerializer = new StringSetSerializer(); private final CuratorDatabase curatorDatabase; @@ -275,17 +275,17 @@ public class CuratorDatabaseClient { } /** Acquires the single cluster-global, reentrant lock for all non-active nodes */ - public CuratorMutex lockInactive() { + public Lock lockInactive() { return lock(root.append("locks").append("unallocatedLock"), defaultLockTimeout); } /** Acquires the single cluster-global, reentrant lock for active nodes of this application */ - public CuratorMutex lock(ApplicationId application) { + public Lock lock(ApplicationId application) { return lock(application, defaultLockTimeout); } /** Acquires the single cluster-global, reentrant lock with the specified timeout for active nodes of this application */ - public CuratorMutex lock(ApplicationId application, Duration timeout) { + public Lock lock(ApplicationId application, Duration timeout) { try { return lock(lockPath(application), timeout); } @@ -294,7 +294,7 @@ public class CuratorDatabaseClient { } } - private CuratorMutex lock(Path path, Duration timeout) { + private Lock lock(Path path, Duration timeout) { return curatorDatabase.lock(path, timeout); } @@ -331,7 +331,7 @@ public class CuratorDatabaseClient { transaction.commit(); } - public CuratorMutex lockInactiveJobs() { + public Lock lockInactiveJobs() { return lock(root.append("locks").append("inactiveJobsLock"), defaultLockTimeout); } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java index 8a95976138a..bf7942a1c56 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/GroupPreparer.java @@ -56,7 +56,7 @@ class GroupPreparer { List<Node> surplusActiveNodes, MutableInteger highestIndex, int nofSpares, BiConsumer<List<Node>, String> debugRecorder) { try (Mutex lock = nodeRepository.lock(application)) { - // A snapshot of nodes before we start the process - used to determind if this is a replacement + // A snapshot of nodes before we start the process - used to determine if this is a replacement List<Node> nodesBefore = nodeRepository.getNodes(application, Node.State.values()); NodeAllocation allocation = new NodeAllocation(application, cluster, requestedNodes, highestIndex, clock); @@ -86,7 +86,7 @@ class GroupPreparer { accepted = allocation.offer(prioritizeNodes(readyNodes, requestedNodes), !canChangeGroup); allocation.update(nodeRepository.reserve(accepted)); - if(nodeRepository.dynamicAllocationEnabled()) { + if (nodeRepository.dynamicAllocationEnabled()) { // Check if we have available capacity on docker hosts that we can allocate if (!allocation.fullfilled()) { // The new dynamic allocation method diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClientTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClientTest.java index 0994b11d663..a9dc5647a0f 100644 --- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClientTest.java +++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/persistence/CuratorDatabaseClientTest.java @@ -8,6 +8,7 @@ import com.yahoo.config.provision.NodeType; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.Zone; import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.curator.mock.MockCurator; import com.yahoo.vespa.hosted.provision.Node; import com.yahoo.vespa.hosted.provision.provisioning.FlavorConfigBuilder; @@ -42,18 +43,18 @@ public class CuratorDatabaseClientTest { public void locks_can_be_acquired_and_released() { ApplicationId app = ApplicationId.from(TenantName.from("testTenant"), ApplicationName.from("testApp"), InstanceName.from("testInstance")); - try (CuratorMutex mutex1 = zkClient.lock(app)) { + try (Lock mutex1 = zkClient.lock(app)) { mutex1.toString(); // reference to avoid warning throw new RuntimeException(); } catch (RuntimeException expected) { } - try (CuratorMutex mutex2 = zkClient.lock(app)) { + try (Lock mutex2 = zkClient.lock(app)) { mutex2.toString(); // reference to avoid warning } - try (CuratorMutex mutex3 = zkClient.lock(app)) { + try (Lock mutex3 = zkClient.lock(app)) { mutex3.toString(); // reference to avoid warning } diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/CopyOnWriteHashMap.java b/vespajlib/src/main/java/com/yahoo/concurrent/CopyOnWriteHashMap.java index 8b6392285c5..aff2b6af2f0 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/CopyOnWriteHashMap.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/CopyOnWriteHashMap.java @@ -7,13 +7,12 @@ import java.util.Map; import java.util.Set; /** - * <p>This is a thread hash map for small collections that are stable once built. Until it is stable there will be a + * This is a thread hash map for small collections that are stable once built. Until it is stable there will be a * race among all threads missing something in the map. They will then clone the map add the missing stuff and then put * it back as active again. Here are no locks, but the cost is that inserts will happen a lot more than necessary. The - * map reference is volatile, but on most multicpu machines that has no cost unless modified.</p> + * map reference is volatile, but on most multicpu machines that has no cost unless modified. * * @author baldersheim - * @since 5.2 */ public class CopyOnWriteHashMap<K, V> implements Map<K, V> { diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/DaemonThreadFactory.java b/vespajlib/src/main/java/com/yahoo/concurrent/DaemonThreadFactory.java index 92b35c4cc0b..4c15b6e2365 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/DaemonThreadFactory.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/DaemonThreadFactory.java @@ -8,7 +8,7 @@ import java.util.concurrent.ThreadFactory; * A simple thread factory that decorates <code>Executors.defaultThreadFactory()</code> * and sets all created threads to be daemon threads. * - * @author <a href="mailto:einarmr@yahoo-inc.com">Einar M R Rosenvinge</a> + * @author Einar M R Rosenvinge */ public class DaemonThreadFactory implements ThreadFactory { private ThreadFactory defaultThreadFactory = Executors.defaultThreadFactory(); diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/EventBarrier.java b/vespajlib/src/main/java/com/yahoo/concurrent/EventBarrier.java index 79402d928fc..8130c369b75 100644 --- a/vespajlib/src/main/java/com/yahoo/concurrent/EventBarrier.java +++ b/vespajlib/src/main/java/com/yahoo/concurrent/EventBarrier.java @@ -16,8 +16,8 @@ import java.util.List; * algorithm would be to make a thread wait for events happening in * other threads to complete. * - * @author <a href="mailto:havardpe@yahoo-inc.com">Haavard Pettersen</a> - * @author <a href="mailto:simon@yahoo-inc.com">Simon Thoresen</a> + * @author Haavard Pettersen + * @author Simon Thoresen */ public class EventBarrier { diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/Lock.java b/vespajlib/src/main/java/com/yahoo/concurrent/Lock.java new file mode 100644 index 00000000000..ca3707f84d6 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/concurrent/Lock.java @@ -0,0 +1,23 @@ +package com.yahoo.concurrent; + +import java.util.concurrent.locks.ReentrantLock; + +/** + * An acquired lock which is released on close + * + * @author bratseth + */ +public final class Lock implements AutoCloseable { + + private final ReentrantLock wrappedLock; + + Lock(ReentrantLock wrappedLock) { + this.wrappedLock = wrappedLock; + } + + /** Releases this lock */ + public void close() { + wrappedLock.unlock(); + } + +} diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/Locks.java b/vespajlib/src/main/java/com/yahoo/concurrent/Locks.java new file mode 100644 index 00000000000..fcac7f31356 --- /dev/null +++ b/vespajlib/src/main/java/com/yahoo/concurrent/Locks.java @@ -0,0 +1,56 @@ +package com.yahoo.concurrent; + +import com.google.common.util.concurrent.UncheckedTimeoutException; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.ReentrantLock; + +/** + * Holds a map of locks indexed on keys of a given type. + * This is suitable in cases where exclusive access should be granted to any one of a set of keyed objects and + * there is a finite collection of keyed objects. + * + * The returned locks are reentrant (i.e the owning thread may call lock multiple times) and auto-closable. + * + * Typical use is + * <code> + * try (Lock lock = locks.lock(id)) { + * exclusive use of the object with key id + * } + * </code> + * + * @author bratseth + */ +public class Locks<TYPE> { + + private final Map<TYPE, ReentrantLock> locks = new ConcurrentHashMap<>(); + + private final long timeoutMs; + + public Locks(int timeout, TimeUnit timeoutUnit) { + timeoutMs = timeoutUnit.toMillis(timeout); + } + + /** + * Locks key. This will block until the key is acquired. + * Users of this <b>must</b> close any lock acquired. + * + * @param key the key to lock + * @return the acquired lock + * @throws UncheckedTimeoutException if the lock could not be acquired within the timeout + */ + public Lock lock(TYPE key) { + try { + ReentrantLock lock = locks.computeIfAbsent(key, k -> new ReentrantLock(true)); + boolean acquired = lock.tryLock(timeoutMs, TimeUnit.MILLISECONDS); + if ( ! acquired) + throw new UncheckedTimeoutException("Timed out waiting for the lock to " + key); + return new Lock(lock); + } catch (InterruptedException e) { + throw new RuntimeException("Interrupted while waiting for lock of " + key); + } + } + +} diff --git a/vespajlib/src/main/java/com/yahoo/path/Path.java b/vespajlib/src/main/java/com/yahoo/path/Path.java index 90c73f7f8c0..7389ca2af54 100644 --- a/vespajlib/src/main/java/com/yahoo/path/Path.java +++ b/vespajlib/src/main/java/com/yahoo/path/Path.java @@ -13,7 +13,6 @@ import java.util.List; * Represents a path represented by a list of elements. Immutable * * @author lulf - * @since 5.1 */ @Beta public final class Path { diff --git a/vespajlib/src/main/java/com/yahoo/transaction/Mutex.java b/vespajlib/src/main/java/com/yahoo/transaction/Mutex.java index 991e1473be8..f258aa2961e 100644 --- a/vespajlib/src/main/java/com/yahoo/transaction/Mutex.java +++ b/vespajlib/src/main/java/com/yahoo/transaction/Mutex.java @@ -8,6 +8,6 @@ package com.yahoo.transaction; */ public interface Mutex extends AutoCloseable { - public void close(); + void close(); } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java index 176e23648d3..80171337ad2 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java @@ -61,6 +61,7 @@ public class Curator { } // Depend on ZooKeeperServer to make sure it is started first + // TODO: Move zookeeperserver config out of configserverconfig (requires update of controller services.xml as well) @Inject public Curator(ConfigserverConfig configserverConfig, ZooKeeperServer server) { this(createConnectionSpec(configserverConfig)); diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorMutex.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java index 76ea6da276d..25e575cdb6a 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorMutex.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Lock.java @@ -1,5 +1,5 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.hosted.provision.persistence; +package com.yahoo.vespa.curator; import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.transaction.Mutex; @@ -14,12 +14,12 @@ import java.util.concurrent.TimeUnit; * * @author bratseth */ -public class CuratorMutex implements Mutex { +public class Lock implements Mutex { private final InterProcessMutex mutex; private final String lockPath; - public CuratorMutex(String lockPath, CuratorFramework curator) { + public Lock(String lockPath, CuratorFramework curator) { this.lockPath = lockPath; mutex = new InterProcessMutex(curator, lockPath); } @@ -35,7 +35,7 @@ public class CuratorMutex implements Mutex { } if (! acquired) throw new UncheckedTimeoutException("Timed out after waiting " + timeout.toString() + - " to acquire lock + '" + lockPath + "'"); + " to acquire lock + '" + lockPath + "'"); } @Override |