Use timeout from request parameter

4 files changed, 37 insertions, 29 deletions

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 5fcf835ddab..581c6e0394e 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
@@ -380,12 +380,12 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
 
     // ---------------- Convergence ----------------------------------------------------------------
 
-    public HttpResponse checkServiceForConfigConvergence(ApplicationId applicationId, String hostAndPort, URI uri) {
-        return convergeChecker.checkService(getApplication(applicationId), hostAndPort, uri);
+    public HttpResponse checkServiceForConfigConvergence(ApplicationId applicationId, String hostAndPort, URI uri, Duration timeout) {
+        return convergeChecker.checkService(getApplication(applicationId), hostAndPort, uri, timeout);
     }
 
-    public HttpResponse servicesToCheckForConfigConvergence(ApplicationId applicationId, URI uri) {
-        return convergeChecker.servicesToCheck(getApplication(applicationId), uri);
+    public HttpResponse servicesToCheckForConfigConvergence(ApplicationId applicationId, URI uri, Duration timeout) {
+        return convergeChecker.servicesToCheck(getApplication(applicationId), uri, timeout);
     }
 
     // ---------------- Session operations ----------------------------------------------------------------
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java
index 4978f5f274d..63f48547220 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/ConfigConvergenceChecker.java
@@ -9,6 +9,7 @@ import com.yahoo.config.model.api.PortInfo;
 import com.yahoo.config.model.api.ServiceInfo;
 import com.yahoo.slime.Cursor;
 import com.yahoo.vespa.config.server.http.JSONResponse;
+import org.glassfish.jersey.client.ClientProperties;
 import org.glassfish.jersey.client.proxy.WebResourceFactory;
 
 import javax.ws.rs.GET;
@@ -18,6 +19,7 @@ import javax.ws.rs.client.Client;
 import javax.ws.rs.client.ClientBuilder;
 import javax.ws.rs.client.WebTarget;
 import java.net.URI;
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
@@ -33,6 +35,7 @@ import java.util.stream.Collectors;
  * @author hmusum
  */
 public class ConfigConvergenceChecker extends AbstractComponent {
+
     private static final String statePath = "/state/v1/";
     private static final String configSubPath = "config";
     private final static Set<String> serviceTypesToCheck = new HashSet<>(Arrays.asList(
@@ -45,7 +48,6 @@ public class ConfigConvergenceChecker extends AbstractComponent {
     ));
 
     private final StateApiFactory stateApiFactory;
-    private final Client client = ClientBuilder.newClient();
 
     @Inject
     public ConfigConvergenceChecker() {
@@ -56,25 +58,25 @@ public class ConfigConvergenceChecker extends AbstractComponent {
         this.stateApiFactory = stateApiFactory;
     }
 
-    public ServiceListResponse servicesToCheck(Application application, URI uri) {
+    public ServiceListResponse servicesToCheck(Application application, URI uri, Duration timeout) {
         List<ServiceInfo> servicesToCheck = new ArrayList<>();
         application.getModel().getHosts()
                    .forEach(host -> host.getServices().stream()
                                         .filter(service -> serviceTypesToCheck.contains(service.getServiceType()))
                                         .forEach(service -> getStatePort(service).ifPresent(port -> servicesToCheck.add(service))));
 
-        long currentGeneration = getServiceGeneration(servicesToCheck);
+        long currentGeneration = getServiceGeneration(servicesToCheck, timeout);
         return new ServiceListResponse(200, servicesToCheck, uri, application.getApplicationGeneration(),
                                        currentGeneration);
     }
 
-    public ServiceResponse checkService(Application application, String hostAndPortToCheck, URI uri) {
+    public ServiceResponse checkService(Application application, String hostAndPortToCheck, URI uri, Duration timeout) {
         Long wantedGeneration = application.getApplicationGeneration();
         try {
             if (! hostInApplication(application, hostAndPortToCheck))
                 return ServiceResponse.createHostNotFoundInAppResponse(uri, hostAndPortToCheck, wantedGeneration);
 
-            long currentGeneration = getServiceGeneration(URI.create("http://" + hostAndPortToCheck));
+            long currentGeneration = getServiceGeneration(URI.create("http://" + hostAndPortToCheck), timeout);
             boolean converged = currentGeneration >= wantedGeneration;
             return ServiceResponse.createOkResponse(uri, hostAndPortToCheck, wantedGeneration, currentGeneration, converged);
         } catch (ProcessingException e) { // e.g. if we cannot connect to the service to find generation
@@ -84,11 +86,6 @@ public class ConfigConvergenceChecker extends AbstractComponent {
         }
     }
 
-    @Override
-    public void deconstruct() {
-        client.close();
-    }
-
     @Path(statePath)
     public interface StateApi {
         @Path(configSubPath)
@@ -100,6 +97,13 @@ public class ConfigConvergenceChecker extends AbstractComponent {
         StateApi createStateApi(Client client, URI serviceUri);
     }
 
+    private static Client createClient(Duration timeout) {
+        return ClientBuilder.newBuilder()
+                            .property(ClientProperties.CONNECT_TIMEOUT, (int) timeout.toMillis())
+                            .property(ClientProperties.READ_TIMEOUT, (int) timeout.toMillis())
+                            .build();
+    }
+
     private static Optional<Integer> getStatePort(ServiceInfo service) {
         return service.getPorts().stream()
                 .filter(port -> port.getTags().contains("state"))
@@ -117,7 +121,7 @@ public class ConfigConvergenceChecker extends AbstractComponent {
     }
 
     /** Get service generation for a list of services. Returns the minimum generation of all services */
-    private long getServiceGeneration(List<ServiceInfo> services) {
+    private long getServiceGeneration(List<ServiceInfo> services, Duration timeout) {
         List<URI> serviceUris = services.stream()
                                         .map(s -> "http://" + s.getHostName() + ":" + getStatePort(s).get())
                                         .map(URI::create)
@@ -125,7 +129,7 @@ public class ConfigConvergenceChecker extends AbstractComponent {
         long generation = -1;
         for (URI uri : serviceUris) {
             try {
-                long serviceGeneration = getServiceGeneration(uri);
+                long serviceGeneration = getServiceGeneration(uri, timeout);
                 if (generation == -1 || serviceGeneration < generation) {
                     generation = serviceGeneration;
                 }
@@ -136,9 +140,14 @@ public class ConfigConvergenceChecker extends AbstractComponent {
         return generation;
     }
 
-    private long getServiceGeneration(URI serviceUri) {
-        StateApi state = stateApiFactory.createStateApi(client, serviceUri);
-        return generationFromContainerState(state.config());
+    private long getServiceGeneration(URI serviceUri, Duration timeout) {
+        Client client = createClient(timeout);
+        try {
+            StateApi state = stateApiFactory.createStateApi(client, serviceUri);
+            return generationFromContainerState(state.config());
+        } finally {
+            client.close();
+        }
     }
 
     private boolean hostInApplication(Application application, String hostPort) {
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java
index 473ec913f50..a734c3a70db 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java
@@ -57,11 +57,12 @@ public class ApplicationHandler extends HttpHandler {
     public HttpResponse handleGET(HttpRequest request) {
         ApplicationId applicationId = getApplicationIdFromRequest(request);
         Tenant tenant = verifyTenantAndApplication(applicationId);
+        Duration timeout = HttpHandler.getRequestTimeout(request, Duration.ofSeconds(5));
 
         if (isServiceConvergeRequest(request)) {
             // Expects both hostname and port in the request (hostname:port)
             String hostAndPort = getHostNameFromRequest(request);
-            return applicationRepository.checkServiceForConfigConvergence(applicationId, hostAndPort, request.getUri());
+            return applicationRepository.checkServiceForConfigConvergence(applicationId, hostAndPort, request.getUri(), timeout);
         }
 
         if (isClusterControllerStatusRequest(request)) {
@@ -88,11 +89,10 @@ public class ApplicationHandler extends HttpHandler {
         }
 
         if (isServiceConvergeListRequest(request)) {
-            return applicationRepository.servicesToCheckForConfigConvergence(applicationId, request.getUri());
+            return applicationRepository.servicesToCheckForConfigConvergence(applicationId, request.getUri(), timeout);
         }
 
         if (isFiledistributionStatusRequest(request)) {
-            Duration timeout = HttpHandler.getRequestTimeout(request, Duration.ofSeconds(5));
             return applicationRepository.filedistributionStatus(applicationId, timeout);
         }
 
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ConfigConvergenceCheckerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ConfigConvergenceCheckerTest.java
index 71052c8b463..67a04e0b3d6 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/ConfigConvergenceCheckerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/ConfigConvergenceCheckerTest.java
@@ -21,14 +21,13 @@ import org.junit.rules.TemporaryFolder;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.net.URI;
+import java.time.Duration;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
 
-import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
 import static com.yahoo.vespa.config.server.application.ConfigConvergenceChecker.ServiceResponse;
+import static org.junit.Assert.assertEquals;
 
 /**
  * @author Ulf Lilleengen
@@ -66,7 +65,7 @@ public class ConfigConvergenceCheckerTest {
     public void service_convergence() throws Exception {
         ServiceResponse serviceResponse = checker.checkService(application,
                                                                "localhost:1337",
-                                                               URI.create("http://foo:234/serviceconverge/localhost:1337"));
+                                                               URI.create("http://foo:234/serviceconverge/localhost:1337"), Duration.ofSeconds(5));
         assertEquals(200, serviceResponse.getStatus());
         assertJsonEquals("{\n" +
                          "  \"url\": \"http://foo:234/serviceconverge/localhost:1337\",\n" +
@@ -79,7 +78,7 @@ public class ConfigConvergenceCheckerTest {
 
         ServiceResponse hostMissingResponse = checker.checkService(application,
                                                                    "notPresent:1337",
-                                                                   URI.create("http://foo:234/serviceconverge/notPresent:1337"));
+                                                                   URI.create("http://foo:234/serviceconverge/notPresent:1337"), Duration.ofSeconds(5));
         assertEquals(410, hostMissingResponse.getStatus());
         assertJsonEquals("{\n" +
                          "  \"url\": \"http://foo:234/serviceconverge/notPresent:1337\",\n" +
@@ -92,7 +91,7 @@ public class ConfigConvergenceCheckerTest {
 
     @Test
     public void service_list_convergence() throws Exception {
-        HttpResponse serviceListResponse = checker.servicesToCheck(application, URI.create("http://foo:234/serviceconverge"));
+        HttpResponse serviceListResponse = checker.servicesToCheck(application, URI.create("http://foo:234/serviceconverge"), Duration.ofSeconds(5));
         assertEquals(200, serviceListResponse.getStatus());
         assertJsonEquals("{\n" +
                          "  \"services\": [\n" +
@@ -120,7 +119,7 @@ public class ConfigConvergenceCheckerTest {
                                                   Version.fromIntValues(0, 0, 0),
                                                   MetricUpdater.createTestUpdater(), appId);
         currentGeneration.put(URI.create("http://host2:1234"), 4L);
-        serviceListResponse = checker.servicesToCheck(application, URI.create("http://foo:234/serviceconverge"));
+        serviceListResponse = checker.servicesToCheck(application, URI.create("http://foo:234/serviceconverge"), Duration.ofSeconds(5));
         assertEquals(200, serviceListResponse.getStatus());
         assertJsonEquals("{\n" +
                          "  \"services\": [\n" +