diff options
author | Martin Polden <mpolden@mpolden.no> | 2020-06-04 12:36:24 +0200 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2020-06-05 11:22:58 +0200 |
commit | 7b37ebe49ae07ed4a1093efb34dcd234b539bd5c (patch) | |
tree | 86fdcb61693cf859f536abe44b543ea5dbddddb3 /controller-server/src | |
parent | 79ffe8240ab6cb7f0dc8fd61ee64e25299fb08b9 (diff) |
Retry proxy request with single target
Diffstat (limited to 'controller-server/src')
6 files changed, 55 insertions, 47 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java index e93e4f3529f..49a23a8337b 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ConfigServerRestExecutorImpl.java @@ -1,6 +1,7 @@ // 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.controller.proxy; +import ai.vespa.util.http.retry.Sleeper; import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; import com.yahoo.jdisc.http.HttpRequest.Method; @@ -53,9 +54,12 @@ public class ConfigServerRestExecutorImpl extends AbstractComponent implements C private static final Logger LOG = Logger.getLogger(ConfigServerRestExecutorImpl.class.getName()); private static final Duration PROXY_REQUEST_TIMEOUT = Duration.ofSeconds(10); private static final Duration PING_REQUEST_TIMEOUT = Duration.ofMillis(500); + private static final Duration SINGLE_TARGET_WAIT = Duration.ofSeconds(2); + private static final int SINGLE_TARGET_RETRIES = 3; private static final Set<String> HEADERS_TO_COPY = Set.of("X-HTTP-Method-Override", "Content-Type"); private final CloseableHttpClient client; + private final Sleeper sleeper; @Inject public ConfigServerRestExecutorImpl(ZoneRegistry zoneRegistry, ServiceIdentityProvider sslContextProvider) { @@ -66,22 +70,31 @@ public class ConfigServerRestExecutorImpl extends AbstractComponent implements C this.client = createHttpClient(config, sslContextProvider, new ControllerOrConfigserverHostnameVerifier(zoneRegistry)); + this.sleeper = new Sleeper.Default(); } @Override public ProxyResponse handle(ProxyRequest proxyRequest) throws ProxyException { - // Make a local copy of the list as we want to manipulate it in case of ping problems. - List<URI> allServers = new ArrayList<>(proxyRequest.getTargets()); + List<URI> targets = new ArrayList<>(proxyRequest.getTargets()); StringBuilder errorBuilder = new StringBuilder(); - if (queueFirstServerIfDown(allServers)) { + boolean singleTarget = targets.size() == 1; + if (singleTarget) { + for (int i = 0; i < SINGLE_TARGET_RETRIES - 1; i++) { + targets.add(targets.get(0)); + } + } else if (queueFirstServerIfDown(targets)) { errorBuilder.append("Change ordering due to failed ping."); } - for (URI uri : allServers) { - Optional<ProxyResponse> proxyResponse = proxyCall(uri, proxyRequest, errorBuilder); + + for (URI url : targets) { + Optional<ProxyResponse> proxyResponse = proxyCall(url, proxyRequest, errorBuilder); if (proxyResponse.isPresent()) { return proxyResponse.get(); } + if (singleTarget) { + sleeper.sleep(SINGLE_TARGET_WAIT); + } } throw new ProxyException(ErrorResponse.internalServerError("Failed talking to config servers: " @@ -185,6 +198,7 @@ public class ConfigServerRestExecutorImpl extends AbstractComponent implements C .setConnectionRequestTimeout((int) PING_REQUEST_TIMEOUT.toMillis()) .setSocketTimeout((int) PING_REQUEST_TIMEOUT.toMillis()).build(); httpGet.setConfig(config); + try (CloseableHttpResponse response = client.execute(httpGet)) { if (response.getStatusLine().getStatusCode() == 200) { return false; diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java index f398683567b..a6314df9581 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequest.java @@ -28,33 +28,21 @@ public class ProxyRequest { private final List<URI> targets; private final String targetPath; - /** - * The constructor calls exception if the request is invalid. - * - * @param request the request from the jdisc framework. - * @param targets list of targets this request should be proxied to (targets are tried once in order until a response is returned). - * @param targetPath the path to proxy to. - * @throws ProxyException on errors - */ - public ProxyRequest(HttpRequest request, List<URI> targets, String targetPath) throws ProxyException { - this(request.getMethod(), request.getUri(), request.getJDiscRequest().headers(), request.getData(), - targets, targetPath); - } - - ProxyRequest(Method method, URI requestUri, Map<String, List<String>> headers, InputStream body, - List<URI> targets, String targetPath) throws ProxyException { - Objects.requireNonNull(requestUri, "Request must be non-null"); - if (!requestUri.getPath().endsWith(targetPath)) - throw new ProxyException(ErrorResponse.badRequest(String.format( - "Request path '%s' does not end with proxy path '%s'", requestUri.getPath(), targetPath))); - + ProxyRequest(Method method, URI url, Map<String, List<String>> headers, InputStream body, List<URI> targets, + String path) { + Objects.requireNonNull(url); + if (!url.getPath().endsWith(path)) { + throw new IllegalArgumentException(String.format("Request path '%s' does not end with proxy path '%s'", url.getPath(), path)); + } + if (targets.isEmpty()) { + throw new IllegalArgumentException("targets must be non-empty"); + } this.method = Objects.requireNonNull(method); - this.requestUri = Objects.requireNonNull(requestUri); + this.requestUri = Objects.requireNonNull(url); this.headers = Objects.requireNonNull(headers); this.requestData = body; - this.targets = List.copyOf(targets); - this.targetPath = targetPath.startsWith("/") ? targetPath : "/" + targetPath; + this.targetPath = path.startsWith("/") ? path : "/" + path; } @@ -100,4 +88,16 @@ public class ProxyRequest { return "[targets: " + targets + " request: " + targetPath + "]"; } + /** Create a proxy request that tries all given targets in order */ + public static ProxyRequest tryAll(List<URI> targets, String path, HttpRequest request) { + return new ProxyRequest(request.getMethod(), request.getUri(), request.getJDiscRequest().headers(), + request.getData(), targets, path); + } + + /** Create a proxy request that repeatedly tries a single target */ + public static ProxyRequest tryOne(URI target, String path, HttpRequest request) { + return new ProxyRequest(request.getMethod(), request.getUri(), request.getJDiscRequest().headers(), + request.getData(), List.of(target), path); + } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java index 6ffdea93a1c..a1a7dab2955 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/configserver/ConfigServerApiHandler.java @@ -96,7 +96,7 @@ public class ConfigServerApiHandler extends AuditLoggingRequestHandler { } try { - return proxy.handle(new ProxyRequest(request, List.of(getEndpoint(zoneId)), cfgPath)); + return proxy.handle(ProxyRequest.tryOne(getEndpoint(zoneId), cfgPath, request)); } catch (ProxyException e) { throw new RuntimeException(e); } @@ -126,4 +126,5 @@ public class ConfigServerApiHandler extends AuditLoggingRequestHandler { private URI getEndpoint(ZoneId zoneId) { return CONTROLLER_ZONE.equals(zoneId) ? CONTROLLER_URI : zoneRegistry.getConfigServerVipUri(zoneId); } + } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java index a127a44efb2..f1bb4ba9268 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/zone/v2/ZoneApiHandler.java @@ -20,8 +20,6 @@ import com.yahoo.vespa.hosted.controller.proxy.ProxyException; import com.yahoo.vespa.hosted.controller.proxy.ProxyRequest; import com.yahoo.yolean.Exceptions; -import java.net.URI; -import java.util.List; import java.util.logging.Level; /** @@ -85,7 +83,7 @@ public class ZoneApiHandler extends AuditLoggingRequestHandler { throw new IllegalArgumentException("No such zone: " + zoneId.value()); } try { - return proxy.handle(new ProxyRequest(request, getConfigserverEndpoints(zoneId), path.getRest())); + return proxy.handle(proxyRequest(zoneId, path.getRest(), request)); } catch (ProxyException e) { throw new RuntimeException(e); } @@ -114,13 +112,12 @@ public class ZoneApiHandler extends AuditLoggingRequestHandler { return ErrorResponse.notFoundError("Nothing at " + path); } - private List<URI> getConfigserverEndpoints(ZoneId zoneId) { + private ProxyRequest proxyRequest(ZoneId zoneId, String path, HttpRequest request) { // TODO: Use config server VIP for all zones that have one if (zoneId.region().value().startsWith("aws-") || zoneId.region().value().contains("-aws-")) { - return List.of(zoneRegistry.getConfigServerVipUri(zoneId)); - } else { - return zoneRegistry.getConfigServerUris(zoneId); + return ProxyRequest.tryOne(zoneRegistry.getConfigServerVipUri(zoneId), path, request); } + return ProxyRequest.tryAll(zoneRegistry.getConfigServerUris(zoneId), path, request); } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java index d8373cb8928..15ec138354d 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyRequestTest.java @@ -21,12 +21,6 @@ public class ProxyRequestTest { public final ExpectedException exception = ExpectedException.none(); @Test - public void testEmpty() throws Exception { - exception.expectMessage("Request must be non-null"); - new ProxyRequest(HttpRequest.Method.GET, null, Map.of(), null, List.of(), "/zone/v2"); - } - - @Test public void testBadUri() throws Exception { exception.expectMessage("Request path '/path' does not end with proxy path '/zone/v2/'"); testRequest("http://domain.tld/path", "/zone/v2/"); @@ -67,8 +61,9 @@ public class ProxyRequestTest { } } - private static ProxyRequest testRequest(String url, String pathPrefix) throws ProxyException { - return new ProxyRequest( - HttpRequest.Method.GET, URI.create(url), Map.of(), null, List.of(), pathPrefix); + private static ProxyRequest testRequest(String url, String pathPrefix) { + return new ProxyRequest(HttpRequest.Method.GET, URI.create(url), Map.of(), null, + List.of(URI.create("http://example.com")), pathPrefix); } + } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java index 0aac59321b5..539d4a6dd75 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/proxy/ProxyResponseTest.java @@ -20,7 +20,7 @@ public class ProxyResponseTest { @Test public void testRewriteUrl() throws Exception { ProxyRequest request = new ProxyRequest(HttpRequest.Method.GET, URI.create("http://domain.tld/zone/v2/dev/us-north-1/configserver"), - Map.of(), null, List.of(), "configserver"); + Map.of(), null, List.of(URI.create("http://example.com")), "configserver"); ProxyResponse proxyResponse = new ProxyResponse( request, "response link is http://configserver:1234/bla/bla/", @@ -38,7 +38,7 @@ public class ProxyResponseTest { @Test public void testRewriteSecureUrl() throws Exception { ProxyRequest request = new ProxyRequest(HttpRequest.Method.GET, URI.create("https://domain.tld/zone/v2/prod/eu-south-3/configserver"), - Map.of(), null, List.of(), "configserver"); + Map.of(), null, List.of(URI.create("http://example.com")), "configserver"); ProxyResponse proxyResponse = new ProxyResponse( request, "response link is http://configserver:1234/bla/bla/", @@ -52,4 +52,5 @@ public class ProxyResponseTest { String document = new String(outputStream.toByteArray(), StandardCharsets.UTF_8); assertEquals("response link is https://domain.tld/zone/v2/prod/eu-south-3/bla/bla/", document); } + } |