diff options
author | Jon Marius Venstad <jonmv@users.noreply.github.com> | 2019-02-26 13:57:01 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-02-26 13:57:01 +0100 |
commit | 77dcd5080bdb869d272b57b6b15303bc094ff18e (patch) | |
tree | b12afe6f05b177fc204934e73f562a6709eb1817 | |
parent | bbd4f90e8c9906845ed027f102667acc6ccc371d (diff) | |
parent | 2da5e9556bc6114031b6e8d9e6c8632d23df8ebb (diff) |
Merge pull request #8609 from vespa-engine/jvenstad/fix-config-model-inconsitency
Jvenstad/fix config model inconsitency
22 files changed, 65 insertions, 301 deletions
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/AllocatedHosts.java b/config-provisioning/src/main/java/com/yahoo/config/provision/AllocatedHosts.java index 28c5d475e19..e2b2933ede3 100644 --- a/config-provisioning/src/main/java/com/yahoo/config/provision/AllocatedHosts.java +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/AllocatedHosts.java @@ -97,8 +97,7 @@ public class AllocatedHosts { } private static Optional<Flavor> flavorFromSlime(Inspector object, Optional<NodeFlavors> nodeFlavors) { - return nodeFlavors.map(flavorMapper -> flavorMapper.getFlavor(object.field(hostSpecFlavor).asString())) - .orElse(Optional.empty()); + return nodeFlavors.flatMap(flavorMapper -> flavorMapper.getFlavor(object.field(hostSpecFlavor).asString())); } private static Optional<String> optionalString(Inspector inspector) { diff --git a/configserver/pom.xml b/configserver/pom.xml index cf75eb3b999..f346cde63a3 100644 --- a/configserver/pom.xml +++ b/configserver/pom.xml @@ -240,6 +240,7 @@ <arg>-Xlint:deprecation</arg> <arg>-Xlint:all</arg> <arg>-Xlint:-serial</arg> + <arg>-Xlint:-try</arg> <arg>-Werror</arg> </compilerArgs> </configuration> diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index 6aa5ad81309..ea7b6a88a9c 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -161,22 +161,22 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye } public PrepareResult prepareAndActivate(Tenant tenant, long sessionId, PrepareParams prepareParams, - boolean ignoreLockFailure, boolean ignoreSessionStaleFailure, Instant now) { + boolean ignoreSessionStaleFailure, Instant now) { PrepareResult result = prepare(tenant, sessionId, prepareParams, now); - activate(tenant, sessionId, prepareParams.getTimeoutBudget(), ignoreLockFailure, ignoreSessionStaleFailure); + activate(tenant, sessionId, prepareParams.getTimeoutBudget(), ignoreSessionStaleFailure); return result; } public PrepareResult deploy(CompressedApplicationInputStream in, PrepareParams prepareParams) { - return deploy(in, prepareParams, false, false, clock.instant()); + return deploy(in, prepareParams, false, clock.instant()); } public PrepareResult deploy(CompressedApplicationInputStream in, PrepareParams prepareParams, - boolean ignoreLockFailure, boolean ignoreSessionStaleFailure, Instant now) { + boolean ignoreSessionStaleFailure, Instant now) { File tempDir = Files.createTempDir(); PrepareResult prepareResult; try { - prepareResult = deploy(decompressApplication(in, tempDir), prepareParams, ignoreLockFailure, ignoreSessionStaleFailure, now); + prepareResult = deploy(decompressApplication(in, tempDir), prepareParams, ignoreSessionStaleFailure, now); } finally { cleanupTempDirectory(tempDir); } @@ -184,15 +184,15 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye } public PrepareResult deploy(File applicationPackage, PrepareParams prepareParams) { - return deploy(applicationPackage, prepareParams, false, false, Instant.now()); + return deploy(applicationPackage, prepareParams, false, Instant.now()); } public PrepareResult deploy(File applicationPackage, PrepareParams prepareParams, - boolean ignoreLockFailure, boolean ignoreSessionStaleFailure, Instant now) { + boolean ignoreSessionStaleFailure, Instant now) { ApplicationId applicationId = prepareParams.getApplicationId(); long sessionId = createSession(applicationId, prepareParams.getTimeoutBudget(), applicationPackage); Tenant tenant = tenantRepository.getTenant(applicationId.tenant()); - return prepareAndActivate(tenant, sessionId, prepareParams, ignoreLockFailure, ignoreSessionStaleFailure, now); + return prepareAndActivate(tenant, sessionId, prepareParams, ignoreSessionStaleFailure, now); } /** @@ -271,11 +271,9 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye public ApplicationId activate(Tenant tenant, long sessionId, TimeoutBudget timeoutBudget, - boolean ignoreLockFailure, boolean ignoreSessionStaleFailure) { LocalSession localSession = getLocalSession(tenant, sessionId); Deployment deployment = deployFromPreparedSession(localSession, tenant, timeoutBudget.timeLeft()); - deployment.setIgnoreLockFailure(ignoreLockFailure); deployment.setIgnoreSessionStaleFailure(ignoreSessionStaleFailure); deployment.activate(); return localSession.getApplicationId(); 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..21716730825 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; @@ -56,7 +56,6 @@ public class Deployment implements com.yahoo.config.provision.Deployment { /** Whether this model should be validated (only takes effect if prepared=false) */ private boolean validate; - private boolean ignoreLockFailure = false; private boolean ignoreSessionStaleFailure = false; private Deployment(LocalSession session, ApplicationRepository applicationRepository, @@ -90,11 +89,6 @@ public class Deployment implements com.yahoo.config.provision.Deployment { timeout, clock, true, true, session.getVespaVersion(), isBootstrap); } - public Deployment setIgnoreLockFailure(boolean ignoreLockFailure) { - this.ignoreLockFailure = ignoreLockFailure; - return this; - } - public Deployment setIgnoreSessionStaleFailure(boolean ignoreSessionStaleFailure) { this.ignoreSessionStaleFailure = ignoreSessionStaleFailure; return this; @@ -128,16 +122,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 +135,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 " + @@ -186,12 +165,12 @@ public class Deployment implements com.yahoo.config.provision.Deployment { return sessionId; } - private Transaction deactivateCurrentActivateNew(LocalSession currentActiveSession, LocalSession session, boolean ignoreStaleSessionFailure) { - Transaction transaction = session.createActivateTransaction(); - if (isValidSession(currentActiveSession)) { - checkIfActiveHasChanged(session, currentActiveSession, ignoreStaleSessionFailure); - checkIfActiveIsNewerThanSessionToBeActivated(session.getSessionId(), currentActiveSession.getSessionId()); - transaction.add(currentActiveSession.createDeactivateTransaction().operations()); + private Transaction deactivateCurrentActivateNew(LocalSession active, LocalSession prepared, boolean ignoreStaleSessionFailure) { + Transaction transaction = prepared.createActivateTransaction(); + if (isValidSession(active)) { + checkIfActiveHasChanged(prepared, active, ignoreStaleSessionFailure); + checkIfActiveIsNewerThanSessionToBeActivated(prepared.getSessionId(), active.getSessionId()); + transaction.add(active.createDeactivateTransaction().operations()); } return transaction; } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/host/HostRegistries.java b/configserver/src/main/java/com/yahoo/vespa/config/server/host/HostRegistries.java index b4bda7fcbb9..c25ab0315a3 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/host/HostRegistries.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/host/HostRegistries.java @@ -4,7 +4,8 @@ package com.yahoo.vespa.config.server.host; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.TenantName; -import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; /** * Component to hold host registries. @@ -14,7 +15,7 @@ import java.util.HashMap; public class HostRegistries { private final HostRegistry<TenantName> tenantHostRegistry = new HostRegistry<>(); - private final HashMap<TenantName, HostRegistry<ApplicationId>> applicationHostRegistries = new HashMap<>(); + private final Map<TenantName, HostRegistry<ApplicationId>> applicationHostRegistries = new ConcurrentHashMap<>(); public HostRegistry<TenantName> getTenantHostRegistry() { return tenantHostRegistry; @@ -29,4 +30,5 @@ public class HostRegistries { applicationHostRegistries.put(tenant, applicationIdHostRegistry); return applicationIdHostRegistry; } + } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/SessionHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/SessionHandler.java index 63703e45e0c..59d12e037e9 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/SessionHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/SessionHandler.java @@ -75,10 +75,6 @@ public class SessionHandler extends HttpHandler { return new DeployHandlerLogger(deployLog.get().setArray("log"), verbose, app); } - protected static boolean shouldIgnoreLockFailure(HttpRequest request) { - return request.getBooleanProperty("force"); - } - /** * True if this request should ignore activation failure because the session was made from an active session that is not active now * @param request a {@link com.yahoo.container.jdisc.HttpRequest} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java index 411e24cdbc6..7f409e4c8fa 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationApiHandler.java @@ -54,7 +54,6 @@ public class ApplicationApiHandler extends SessionHandler { PrepareParams prepareParams = PrepareParams.fromHttpRequest(request, tenantName, zookeeperBarrierTimeout); PrepareResult result = applicationRepository.prepareAndActivate(tenant, sessionId, prepareParams, - shouldIgnoreLockFailure(request), shouldIgnoreSessionStaleFailure(request), Instant.now()); return new SessionPrepareAndActivateResponse(result, tenantName, request, prepareParams.getApplicationId(), zone); @@ -70,7 +69,6 @@ public class ApplicationApiHandler extends SessionHandler { PrepareResult result = applicationRepository.deploy(CompressedApplicationInputStream.createFromCompressedStream(request.getData(), request.getHeader(contentTypeHeader)), prepareParams, - shouldIgnoreLockFailure(request), shouldIgnoreSessionStaleFailure(request), Instant.now()); return new SessionPrepareAndActivateResponse(result, tenantName, request, prepareParams.getApplicationId(), zone); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandler.java index 2ccef8b85df..b34fe490c3a 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandler.java @@ -50,7 +50,6 @@ public class SessionActiveHandler extends SessionHandler { TimeoutBudget timeoutBudget = getTimeoutBudget(request, DEFAULT_ACTIVATE_TIMEOUT); final Long sessionId = getSessionIdV2(request); ApplicationId applicationId = applicationRepository.activate(tenant, sessionId, timeoutBudget, - shouldIgnoreLockFailure(request), shouldIgnoreSessionStaleFailure(request)); ApplicationMetaData metaData = applicationRepository.getMetadataFromSession(tenant, sessionId); return new SessionActiveResponse(metaData.getSlime(), request, applicationId, sessionId, zone); 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/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java index ed62a093a75..bcdd14f8344 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java @@ -174,7 +174,7 @@ public class TenantRepository implements ConnectionStateListener, PathChildrenCa try { f.getValue().get(); } catch (ExecutionException e) { - log.log(LogLevel.WARNING, "Failed to create tenant " + tenantName); + log.log(LogLevel.WARNING, "Failed to create tenant " + tenantName, e); failed.add(tenantName); } catch (InterruptedException e) { log.log(LogLevel.WARNING, "Interrupted while creating tenant '" + tenantName + "'", e); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java index ad25777cdf8..3d94dedf651 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java @@ -316,7 +316,7 @@ public class ApplicationRepositoryTest { ApplicationId applicationId = applicationId(); long sessionId = applicationRepository.createSession(applicationId, timeoutBudget, appDir.getAppDir()); return applicationRepository.prepareAndActivate(tenantRepository.getTenant(applicationId.tenant()), - sessionId, prepareParams(), false, false, Instant.now()); + sessionId, prepareParams(), false, Instant.now()); } private PrepareResult deployApp(File applicationPackage) { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java index c96113b9462..7ffb9552cf8 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/deploy/DeployTester.java @@ -177,7 +177,7 @@ public class DeployTester { if (vespaVersion != null) paramsBuilder.vespaVersion(vespaVersion); - return applicationRepository.deploy(new File(applicationPath), paramsBuilder.build(), false, false, now); + return applicationRepository.deploy(new File(applicationPath), paramsBuilder.build(), false, now); } public AllocatedHosts getAllocatedHostsOf(ApplicationId applicationId) { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java index 10ee1a22bab..37784b313b6 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java @@ -4,16 +4,18 @@ package com.yahoo.vespa.config.server.session; import com.google.common.io.Files; import com.yahoo.component.Version; import com.yahoo.config.application.api.ApplicationFile; +import com.yahoo.config.model.application.provider.BaseDeployLogger; +import com.yahoo.config.model.application.provider.FilesApplicationPackage; +import com.yahoo.config.model.application.provider.MockFileRegistry; import com.yahoo.config.provision.AllocatedHosts; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.HostSpec; import com.yahoo.config.provision.NetworkPorts; import com.yahoo.config.provision.TenantName; import com.yahoo.path.Path; -import com.yahoo.config.model.application.provider.*; import com.yahoo.slime.Slime; import com.yahoo.transaction.NestedTransaction; -import com.yahoo.vespa.config.server.*; +import com.yahoo.vespa.config.server.SuperModelGenerationCounter; import com.yahoo.vespa.config.server.application.MemoryTenantApplications; import com.yahoo.vespa.config.server.deploy.DeployHandlerLogger; import com.yahoo.vespa.config.server.deploy.TenantFileSystemDirs; @@ -30,10 +32,16 @@ import org.junit.Test; import java.io.File; import java.time.Instant; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Optional; import static org.hamcrest.core.Is.is; -import static org.junit.Assert.*; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** * @author Ulf Lilleengen @@ -210,4 +218,5 @@ public class LocalSessionTest { return new DeployHandlerLogger(new Slime().get(), verbose, new ApplicationId.Builder().tenant("testtenant").applicationName("testapp").build()); } + } diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/EagerCountingCuratorTransaction.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/EagerCountingCuratorTransaction.java deleted file mode 100644 index cd50e98616c..00000000000 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/EagerCountingCuratorTransaction.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.hosted.provision.persistence; - -import com.yahoo.transaction.AbstractTransaction; -import com.yahoo.transaction.Transaction; -import com.yahoo.vespa.curator.recipes.CuratorCounter; - -/** - * A curator transaction of curator counting operations which increments during prepare - * such that the counter is also increased if there is a commit error. - */ -class EagerCountingCuratorTransaction extends AbstractTransaction { - - /** Creates a counting curator transaction containing a single increment operation */ - public EagerCountingCuratorTransaction(CuratorCounter counter) { - add(new CountingCuratorOperation(counter)); - } - - @Override - public void prepare() { - for (Operation operation : operations()) - ((CountingCuratorOperation)operation).next(); - } - - @Override - public void commit() { } - - @Override - public void rollbackOrLog() { } - - static class CountingCuratorOperation implements Transaction.Operation { - - private final CuratorCounter counter; - - public CountingCuratorOperation(CuratorCounter counter) { - this.counter = counter; - } - - public void next() { - counter.next(); - } - - } - -} diff --git a/service-monitor/pom.xml b/service-monitor/pom.xml index c6504ff9315..0826826f433 100644 --- a/service-monitor/pom.xml +++ b/service-monitor/pom.xml @@ -130,6 +130,7 @@ <compilerArgs> <arg>-Xlint:all</arg> <arg>-Xlint:-serial</arg> + <arg>-Xlint:-try</arg> <arg>-Werror</arg> </compilerArgs> </configuration> diff --git a/service-monitor/src/main/java/com/yahoo/vespa/service/model/ServiceModelProvider.java b/service-monitor/src/main/java/com/yahoo/vespa/service/model/ServiceModelProvider.java index dbcc6ff7b9b..c6947810fa0 100644 --- a/service-monitor/src/main/java/com/yahoo/vespa/service/model/ServiceModelProvider.java +++ b/service-monitor/src/main/java/com/yahoo/vespa/service/model/ServiceModelProvider.java @@ -38,11 +38,6 @@ public class ServiceModelProvider implements Supplier<ServiceModel> { @Override public ServiceModel get() { try (LatencyMeasurement measurement = metrics.startServiceModelSnapshotLatencyMeasurement()) { - // Reference 'measurement' in a dummy statement, otherwise the compiler - // complains about "auto-closeable resource is never referenced in body of - // corresponding try statement". Why hasn't javac fixed this!? - dummy(measurement); - // WARNING: The monitor manager may be out-of-sync with duper model (no locking) List<ApplicationInfo> applicationInfos = duperModelManager.getApplicationInfos(); @@ -50,5 +45,4 @@ public class ServiceModelProvider implements Supplier<ServiceModel> { } } - private void dummy(LatencyMeasurement measurement) {} } 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/mock/MemoryFileSystem.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MemoryFileSystem.java index d9494c38fc8..95d75928988 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MemoryFileSystem.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MemoryFileSystem.java @@ -116,7 +116,7 @@ class MemoryFileSystem extends FileSystem { /** The content of this node, never null. This buffer is effectively immutable. */ private byte[] content; - private Map<String, Node> children = new LinkedHashMap<>(); + private Map<String, Node> children = Collections.synchronizedMap(new LinkedHashMap<>()); private Node(Node parent, String name) { this(parent, name, new byte[0]); diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java index 85fa0ded838..3d0fc933e90 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/mock/MockCurator.java @@ -61,7 +61,7 @@ import org.apache.curator.framework.recipes.cache.PathChildrenCacheListener; import org.apache.curator.framework.recipes.locks.InterProcessLock; import org.apache.curator.framework.recipes.locks.InterProcessSemaphoreMutex; import org.apache.curator.framework.state.ConnectionStateListener; -import org.apache.curator.utils.*; +import org.apache.curator.utils.EnsurePath; import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.Watcher; @@ -73,12 +73,12 @@ import java.time.Duration; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.Executor; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; @@ -113,7 +113,7 @@ public class MockCurator extends Curator { private final MemoryFileSystem fileSystem = new MemoryFileSystem(); /** Atomic counters. A more accurate mock would store these as files in the file system */ - private final Map<String, MockAtomicCounter> atomicCounters = new HashMap<>(); + private final Map<String, MockAtomicCounter> atomicCounters = new ConcurrentHashMap<>(); /** Listeners to changes to a particular path */ private final ListenerMap listeners = new ListenerMap(); @@ -361,8 +361,8 @@ public class MockCurator extends Curator { /** The regular listener implementation which notifies registered file and directory listeners */ private class ListenerMap extends Listeners { - private final Map<Path, PathChildrenCacheListener> directoryListeners = new HashMap<>(); - private final Map<Path, NodeCacheListener> fileListeners = new HashMap<>(); + private final Map<Path, PathChildrenCacheListener> directoryListeners = new ConcurrentHashMap<>(); + private final Map<Path, NodeCacheListener> fileListeners = new ConcurrentHashMap<>(); public void add(Path path, PathChildrenCacheListener listener) { directoryListeners.put(path, listener); @@ -376,8 +376,8 @@ public class MockCurator extends Curator { public void notify(Path path, PathChildrenCacheEvent event) { try { // Snapshot directoryListeners in case notification leads to new directoryListeners added - Set<Map.Entry<Path, PathChildrenCacheListener>> directoryLlistenerSnapshot = new HashSet<>(directoryListeners.entrySet()); - for (Map.Entry<Path, PathChildrenCacheListener> listener : directoryLlistenerSnapshot) { + Set<Map.Entry<Path, PathChildrenCacheListener>> directoryListenerSnapshot = new HashSet<>(directoryListeners.entrySet()); + for (Map.Entry<Path, PathChildrenCacheListener> listener : directoryListenerSnapshot) { if (path.isChildOf(listener.getKey())) listener.getValue().childEvent(curatorFramework, event); } 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(); - } - -} |