diff options
7 files changed, 69 insertions, 19 deletions
diff --git a/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationLockException.java b/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationLockException.java new file mode 100644 index 00000000000..1ed74b2f443 --- /dev/null +++ b/config-provisioning/src/main/java/com/yahoo/config/provision/ApplicationLockException.java @@ -0,0 +1,16 @@ +// Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.config.provision; + +/** + * + * Exception thrown when we are unable to get the Zookeeper application lock. + * @author hmusum + * + */ +public class ApplicationLockException extends RuntimeException { + + public ApplicationLockException(Exception e) { + super(e); + } + +} diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpErrorResponse.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpErrorResponse.java index d1746b1ee24..3574336174f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpErrorResponse.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpErrorResponse.java @@ -18,7 +18,7 @@ import static com.yahoo.jdisc.Response.Status.*; * @since 5.1 */ public class HttpErrorResponse extends HttpResponse { - Logger log = Logger.getLogger(HttpErrorResponse.class.getName()); + private static final Logger log = Logger.getLogger(HttpErrorResponse.class.getName()); private final Slime slime = new Slime(); public HttpErrorResponse(int code, final String errorType, final String msg) { @@ -32,14 +32,15 @@ public class HttpErrorResponse extends HttpResponse { } public enum errorCodes { - NOT_FOUND, + APPLICATION_LOCK_FAILURE, BAD_REQUEST, - METHOD_NOT_ALLOWED, INTERNAL_SERVER_ERROR, INVALID_APPLICATION_PACKAGE, - UNKNOWN_VESPA_VERSION, + METHOD_NOT_ALLOWED, + NOT_FOUND, OUT_OF_CAPACITY, - REQUEST_TIMEOUT + REQUEST_TIMEOUT, + UNKNOWN_VESPA_VERSION } public static HttpErrorResponse notFoundError(String msg) { @@ -74,6 +75,10 @@ public class HttpErrorResponse extends HttpResponse { return new HttpErrorResponse(REQUEST_TIMEOUT, errorCodes.REQUEST_TIMEOUT.name(), message); } + public static HttpErrorResponse applicationLockFailure(String msg) { + return new HttpErrorResponse(INTERNAL_SERVER_ERROR, errorCodes.APPLICATION_LOCK_FAILURE.name(), msg); + } + @Override public void render(OutputStream stream) throws IOException { new JsonFormat(true).encode(stream, slime); diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java index d7600dccc6c..c379e24854f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/HttpHandler.java @@ -1,6 +1,7 @@ // Copyright 2016 Yahoo Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.config.server.http; +import com.yahoo.config.provision.ApplicationLockException; import com.yahoo.container.jdisc.HttpRequest; import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.container.jdisc.LoggingRequestHandler; @@ -46,9 +47,7 @@ public class HttpHandler extends LoggingRequestHandler { } } catch (NotFoundException | com.yahoo.vespa.config.server.NotFoundException e) { return HttpErrorResponse.notFoundError(getMessage(e, request)); - } catch (BadRequestException e) { - return HttpErrorResponse.badRequest(getMessage(e, request)); - } catch (IllegalArgumentException | IllegalStateException e) { + } catch (BadRequestException | IllegalArgumentException | IllegalStateException e) { return HttpErrorResponse.badRequest(getMessage(e, request)); } catch (InvalidApplicationException e) { return HttpErrorResponse.invalidApplicationPackage(getMessage(e, request)); @@ -60,6 +59,8 @@ public class HttpHandler extends LoggingRequestHandler { return HttpErrorResponse.unknownVespaVersion(getMessage(e, request)); } catch (RequestTimeoutException e) { return HttpErrorResponse.requestTimeout(getMessage(e, request)); + } catch (ApplicationLockException e) { + return HttpErrorResponse.applicationLockFailure(getMessage(e, request)); } catch (Exception e) { e.printStackTrace(); return HttpErrorResponse.internalServerError(getMessage(e, request)); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java index bee1229c093..f10a7ffd502 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionPrepareHandlerTest.java @@ -2,12 +2,14 @@ package com.yahoo.vespa.config.server.http.v2; import com.google.common.collect.ImmutableMap; +import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.cloud.config.ConfigserverConfig; import com.yahoo.config.application.api.ApplicationFile; import com.yahoo.config.application.api.DeployLogger; import com.yahoo.config.model.api.ServiceInfo; import com.yahoo.config.model.test.MockApplicationPackage; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ApplicationLockException; import com.yahoo.config.provision.OutOfCapacityException; import com.yahoo.config.provision.TenantName; import com.yahoo.container.jdisc.HttpResponse; @@ -301,14 +303,33 @@ public class SessionPrepareHandlerTest extends SessionHandlerTest { String message = "No nodes available"; SessionThrowingException session = new SessionThrowingException(new OutOfCapacityException(message)); localRepo.addSession(session); - HttpResponse response = createHandler(addTestTenant()).handle(SessionHandlerTest.createTestRequest(pathPrefix, HttpRequest.Method.PUT, Cmd.PREPARED, 1L)); + HttpResponse response = createHandler(addTestTenant()) + .handle(SessionHandlerTest.createTestRequest(pathPrefix, HttpRequest.Method.PUT, Cmd.PREPARED, 1L)); assertEquals(400, response.getStatus()); + Slime data = getData(response); + assertThat(data.get().field("error-code").asString(), is(HttpErrorResponse.errorCodes.OUT_OF_CAPACITY.name())); + assertThat(data.get().field("message").asString(), is(message)); + } + + @Test + public void test_application_lock_failure() throws InterruptedException, IOException { + String message = "Timed out after waiting PT1M to acquire lock '/provision/v1/locks/foo/bar/default'"; + SessionThrowingException session = new SessionThrowingException(new ApplicationLockException(new UncheckedTimeoutException(message))); + localRepo.addSession(session); + HttpResponse response = createHandler(addTestTenant()) + .handle(SessionHandlerTest.createTestRequest(pathPrefix, HttpRequest.Method.PUT, Cmd.PREPARED, 1L)); + assertEquals(500, response.getStatus()); + Slime data = getData(response); + assertThat(data.get().field("error-code").asString(), is(HttpErrorResponse.errorCodes.APPLICATION_LOCK_FAILURE.name())); + assertThat(data.get().field("message").asString(), is(message)); + } + + private Slime getData(HttpResponse response) throws IOException { ByteArrayOutputStream baos = new ByteArrayOutputStream(); response.render(baos); Slime data = new Slime(); new JsonDecoder().decode(data, baos.toByteArray()); - assertThat(data.get().field("error-code").asString(), is(HttpErrorResponse.errorCodes.OUT_OF_CAPACITY.name())); - assertThat(data.get().field("message").asString(), is(message)); + return data; } private static void assertResponse(HttpResponse response, String activateString) throws IOException { 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 b2ffe05b2d4..65dd8a67898 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 @@ -62,7 +62,6 @@ public class CuratorDatabase { CuratorMutex lock = locks.computeIfAbsent(path, (pathArg) -> new CuratorMutex(pathArg.getAbsolute(), curator.framework())); lock.acquire(timeout); return lock; - } // --------- Write operations ------------------------------------------------------------------------------ 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 dc6bda01619..54292247115 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 @@ -3,7 +3,9 @@ package com.yahoo.vespa.hosted.provision.persistence; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.datatype.joda.JodaModule; +import com.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.config.provision.ApplicationId; +import com.yahoo.config.provision.ApplicationLockException; import com.yahoo.config.provision.Environment; import com.yahoo.config.provision.NodeFlavors; import com.yahoo.config.provision.Zone; @@ -269,12 +271,17 @@ public class CuratorDatabaseClient { /** Acquires the single cluster-global, reentrant lock for active nodes of this application */ public CuratorMutex lock(ApplicationId application) { - return lock(lockPath(application), defaultLockTimeout); + 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) { - return lock(lockPath(application), timeout); + try { + return lock(lockPath(application), timeout); + } + catch (UncheckedTimeoutException e) { + throw new ApplicationLockException(e); + } } private CuratorMutex lock(Path path, Duration timeout) { diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorMutex.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorMutex.java index 62be5c649db..442dface372 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorMutex.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/persistence/CuratorMutex.java @@ -1,13 +1,13 @@ // Copyright 2016 Yahoo Inc. 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.google.common.util.concurrent.UncheckedTimeoutException; import com.yahoo.transaction.Mutex; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.recipes.locks.InterProcessMutex; import java.time.Duration; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; /** * A cluster-wide reentrant mutex which is released on (the last symmetric) close @@ -26,15 +26,16 @@ public class CuratorMutex implements Mutex { /** Take the lock with the given timeout. This may be called multiple times from the same thread - each matched by a close */ public void acquire(Duration timeout) { + boolean acquired; try { - boolean acquired = mutex.acquire(timeout.toMillis(), TimeUnit.MILLISECONDS); - if ( ! acquired) { - throw new TimeoutException("Timed out after waiting " + timeout.toString()); - } + acquired = mutex.acquire(timeout.toMillis(), TimeUnit.MILLISECONDS); } catch (Exception e) { throw new RuntimeException("Exception acquiring lock '" + lockPath + "'", e); } + + if (! acquired) throw new UncheckedTimeoutException("Timed out after waiting " + timeout.toString() + + " to acquire lock + '" + lockPath + "'"); } @Override |