diff options
6 files changed, 20 insertions, 202 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java index 06b413a97ce..a59d38c7603 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/Deployment.java @@ -16,8 +16,8 @@ import com.yahoo.vespa.config.server.session.LocalSession; import com.yahoo.vespa.config.server.session.PrepareParams; import com.yahoo.vespa.config.server.session.Session; import com.yahoo.vespa.config.server.session.SilentDeployLogger; -import com.yahoo.vespa.config.server.tenant.ActivateLock; import com.yahoo.vespa.config.server.tenant.Tenant; +import com.yahoo.vespa.curator.Lock; import java.time.Clock; import java.time.Duration; @@ -128,16 +128,7 @@ public class Deployment implements com.yahoo.config.provision.Deployment { TimeoutBudget timeoutBudget = new TimeoutBudget(clock, timeout); long sessionId = session.getSessionId(); validateSessionStatus(session); - ActivateLock activateLock = tenant.getActivateLock(); - boolean activateLockAcquired = false; - try { - log.log(LogLevel.DEBUG, "Trying to acquire lock " + activateLock + " for session " + sessionId); - activateLockAcquired = activateLock.acquire(timeoutBudget, ignoreLockFailure); - if ( ! activateLockAcquired) { - throw new ActivationConflictException("Did not get activate lock for session " + sessionId + " within " + timeout); - } - - log.log(LogLevel.DEBUG, "Lock acquired " + activateLock + " for session " + sessionId); + try (Lock lock = tenant.getSessionLock(timeout)) { NestedTransaction transaction = new NestedTransaction(); transaction.add(deactivateCurrentActivateNew(applicationRepository.getActiveSession(session.getApplicationId()), session, ignoreSessionStaleFailure)); @@ -150,12 +141,6 @@ public class Deployment implements com.yahoo.config.provision.Deployment { throw e; } catch (Exception e) { throw new InternalServerException("Error activating application", e); - } finally { - if (activateLockAcquired) { - log.log(LogLevel.DEBUG, "Trying to release lock " + activateLock + " for session " + sessionId); - activateLock.release(); - log.log(LogLevel.DEBUG, "Lock released " + activateLock + " for session " + sessionId); - } } log.log(LogLevel.INFO, session.logPre() + "Session " + sessionId + " activated successfully using " + diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ActivateLock.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ActivateLock.java deleted file mode 100644 index 2fa6c57d9b0..00000000000 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/ActivateLock.java +++ /dev/null @@ -1,45 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.config.server.tenant; - -import com.yahoo.path.Path; -import com.yahoo.vespa.config.server.TimeoutBudget; -import com.yahoo.vespa.curator.Curator; -import com.yahoo.vespa.curator.recipes.CuratorLock; - -import java.util.concurrent.TimeUnit; - -/** - * A lock to protect session activation. - * - * @author Ulf Lilleengen - */ -public class ActivateLock { - - private static final String ACTIVATE_LOCK_NAME = "activateLock"; - private final CuratorLock curatorLock; - - public ActivateLock(Curator curator, Path rootPath) { - this.curatorLock = new CuratorLock(curator, rootPath.append(ACTIVATE_LOCK_NAME).getAbsolute()); - } - - public boolean acquire(TimeoutBudget timeoutBudget, boolean ignoreLockError) { - try { - return curatorLock.tryLock(timeoutBudget.timeLeft().toMillis(), TimeUnit.MILLISECONDS); - } catch (Exception e) { - if (!ignoreLockError) { - throw new RuntimeException(e); - } - return false; - } - } - - public void release() { - curatorLock.unlock(); - } - - @Override - public String toString() { - return "ActivateLock (" + curatorLock + "), has lock: " + curatorLock.hasLock(); - } - -} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java index 1e4c19f5f46..a68f4a396cd 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java @@ -11,8 +11,10 @@ import com.yahoo.vespa.config.server.session.LocalSessionRepo; import com.yahoo.vespa.config.server.session.RemoteSessionRepo; import com.yahoo.vespa.config.server.session.SessionFactory; import com.yahoo.vespa.curator.Curator; +import com.yahoo.vespa.curator.Lock; import org.apache.zookeeper.data.Stat; +import java.time.Duration; import java.time.Instant; import java.util.Optional; @@ -26,8 +28,9 @@ import java.util.Optional; */ public class Tenant implements TenantHandlerProvider { - public static final String SESSIONS = "sessions"; - public static final String APPLICATIONS = "applications"; + static final String SESSIONS = "sessions"; + static final String APPLICATIONS = "applications"; + static final String SESSION_LOCK_PATH = "activateLock"; private final TenantName name; private final RemoteSessionRepo remoteSessionRepo; @@ -35,7 +38,7 @@ public class Tenant implements TenantHandlerProvider { private final SessionFactory sessionFactory; private final LocalSessionRepo localSessionRepo; private final TenantApplications applicationRepo; - private final ActivateLock activateLock; + private final Lock sessionLock; private final RequestHandler requestHandler; private final ReloadHandler reloadHandler; private final TenantFileSystemDirs tenantFileSystemDirs; @@ -58,7 +61,7 @@ public class Tenant implements TenantHandlerProvider { this.remoteSessionRepo = remoteSessionRepo; this.sessionFactory = sessionFactory; this.localSessionRepo = localSessionRepo; - this.activateLock = new ActivateLock(curator, path); + this.sessionLock = createLock(curator, path); this.applicationRepo = applicationRepo; this.tenantFileSystemDirs = tenantFileSystemDirs; this.curator = curator; @@ -108,11 +111,11 @@ public class Tenant implements TenantHandlerProvider { } /** - * The activation lock for this - * @return lock + * This lock allows activation and deactivation of sessions under this tenant. */ - public ActivateLock getActivateLock() { - return activateLock; + public Lock getSessionLock(Duration timeout) { + sessionLock.acquire(timeout); + return sessionLock; } @Override @@ -162,4 +165,10 @@ public class Tenant implements TenantHandlerProvider { curator.delete(path); } + private static Lock createLock(Curator curator, Path tenantPath) { + Path lockPath = tenantPath.append(SESSION_LOCK_PATH); + curator.create(lockPath); + return new Lock(lockPath.getAbsolute(), curator); + } + } 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 91140fcf713..bc47a02ea19 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java @@ -163,7 +163,7 @@ public class Curator implements AutoCloseable { return new DistributedAtomicLong(curatorFramework, path, new ExponentialBackoffRetry(BASE_SLEEP_TIME, MAX_RETRIES)); } - /** For internal use; prefer creating a {@link com.yahoo.vespa.curator.recipes.CuratorLock} */ + /** For internal use; prefer creating a {@link com.yahoo.vespa.curator.Lock} */ public InterProcessLock createMutex(String lockPath) { return new InterProcessMutex(curatorFramework, lockPath); } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/recipes/CuratorLock.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/recipes/CuratorLock.java deleted file mode 100644 index 89460e7916d..00000000000 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/recipes/CuratorLock.java +++ /dev/null @@ -1,80 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.curator.recipes; - -import com.yahoo.vespa.curator.Curator; -import org.apache.curator.framework.recipes.locks.InterProcessLock; - -import java.util.concurrent.TimeUnit; -import java.util.concurrent.locks.Condition; -import java.util.concurrent.locks.Lock; - -/** - * @author Ulf Lilleengen - * @since 5.1 - */ -public class CuratorLock implements Lock { - - private final InterProcessLock mutex; - - public CuratorLock(Curator curator, String lockPath) { - this.mutex = curator.createMutex(lockPath); - } - - public boolean hasLock() { - return mutex.isAcquiredInThisProcess(); - } - - @Override - public void lock() { - try { - mutex.acquire(); - } catch (Exception e) { - throw new CuratorLockException(e); - } - } - - @Override - public void lockInterruptibly() throws InterruptedException { - throw new UnsupportedOperationException(); - } - - @Override - public boolean tryLock() { - try { - return tryLock(0, TimeUnit.MILLISECONDS); - } catch (InterruptedException e) { - throw new CuratorLockException(e); - } - } - - @Override - public boolean tryLock(long time, TimeUnit unit) throws InterruptedException { - try { - return mutex.acquire(time, unit); - } catch (InterruptedException e) { - throw e; - } catch (Exception e) { - throw new CuratorLockException(e); - } - } - - @Override - public void unlock() { - try { - mutex.release(); - } catch (Exception e) { - throw new CuratorLockException(e); - } - } - - @Override - public Condition newCondition() { - throw new UnsupportedOperationException(); - } - - - @Override - public String toString() { - return mutex.toString(); - } -} diff --git a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorLockTest.java b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorLockTest.java deleted file mode 100644 index 53d2df2441b..00000000000 --- a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorLockTest.java +++ /dev/null @@ -1,51 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.curator; - -import com.yahoo.vespa.curator.mock.MockCurator; -import com.yahoo.vespa.curator.recipes.CuratorLock; -import com.yahoo.vespa.curator.recipes.CuratorLockException; -import org.junit.Before; -import org.junit.Test; - -import java.util.concurrent.TimeUnit; - -import static org.junit.Assert.assertFalse; - -/** - * @author Ulf Lilleengen - * @since 5.1 - */ -public class CuratorLockTest { - - private MockCurator curator; - private CuratorLock curatorLock; - - @Before - public void setupLock() { - curator = new MockCurator(); - curatorLock = new CuratorLock(curator, "/foo"); - } - - @Test - public void testAcquireNormal() { - curator.timeoutOnLock = false; - curator.throwExceptionOnLock = false; - curatorLock.lock(); - curatorLock.unlock(); - } - - @Test - public void testLockTimeout() throws InterruptedException { - curator.timeoutOnLock = true; - assertFalse(curatorLock.tryLock(0, TimeUnit.MILLISECONDS)); - curatorLock.unlock(); - } - - @Test(expected = CuratorLockException.class) - public void testLockError() { - curator.throwExceptionOnLock = true; - curatorLock.lock(); - curatorLock.unlock(); - } - -} |