Use specific error-code in response when unable to acquire application lock

* Throw ApplicationLockException if acquiring lock fails (due to timeout)
  and use response with with new error-code for that case
VESPA-6501

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..e213aff9f2c 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 = "Exception acquiring lock '/provision/v1/locks/foo/bar/default': Timed out after waiting PT1M";
+        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