diff options
author | jonmv <venstad@gmail.com> | 2022-04-28 22:12:00 +0200 |
---|---|---|
committer | jonmv <venstad@gmail.com> | 2022-04-28 22:12:00 +0200 |
commit | e8e9cd5d722af2efa5489b9eb4a17aa4b58303a4 (patch) | |
tree | 6ef39c96a22b23d20d5f381e0cacfb57cfbb8ad3 | |
parent | 3af9c40612e539660c9a831520066420eb9f88ab (diff) |
Replace Jersey in orchestrator with apache, remove jaxrx_client_utils
44 files changed, 508 insertions, 1652 deletions
diff --git a/http-client/pom.xml b/http-client/pom.xml index a452353cb8a..ebe83a80903 100644 --- a/http-client/pom.xml +++ b/http-client/pom.xml @@ -52,6 +52,12 @@ <version>2.27.2</version> <scope>test</scope> </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> + <artifactId>testutil</artifactId> + <version>${project.version}</version> + <scope>test</scope> + </dependency> </dependencies> </project> diff --git a/http-client/src/main/java/ai/vespa/hosted/client/AbstractHttpClient.java b/http-client/src/main/java/ai/vespa/hosted/client/AbstractHttpClient.java index 21a6c3c9cb9..2055cd6d74a 100644 --- a/http-client/src/main/java/ai/vespa/hosted/client/AbstractHttpClient.java +++ b/http-client/src/main/java/ai/vespa/hosted/client/AbstractHttpClient.java @@ -1,9 +1,12 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.hosted.client; +import ai.vespa.hosted.client.HttpClient.RequestBuilder; import ai.vespa.http.HttpURL; import ai.vespa.http.HttpURL.Path; import ai.vespa.http.HttpURL.Query; +import com.yahoo.concurrent.UncheckedTimeoutException; +import com.yahoo.time.TimeBudget; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; import org.apache.hc.client5.http.protocol.HttpClientContext; @@ -16,12 +19,15 @@ import org.apache.hc.core5.http.ParseException; import org.apache.hc.core5.http.io.entity.EntityUtils; import org.apache.hc.core5.http.io.entity.HttpEntities; import org.apache.hc.core5.http.io.support.ClassicRequestBuilder; +import org.apache.hc.core5.util.Timeout; import java.io.IOException; import java.io.UncheckedIOException; import java.net.URI; import java.time.Duration; +import java.time.Instant; import java.util.List; +import java.util.Optional; import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; import java.util.function.Function; @@ -58,8 +64,6 @@ public abstract class AbstractHttpClient implements HttpClient { private <T> T execute(RequestBuilder builder, BiFunction<ClassicHttpResponse, ClassicHttpRequest, T> handler, ExceptionHandler catcher) { - HttpClientContext context = HttpClientContext.create(); - context.setRequestConfig(builder.config); Throwable thrown = null; for (URI host : builder.hosts) { @@ -72,7 +76,7 @@ public abstract class AbstractHttpClient implements HttpClient { request.setEntity(builder.entity); try { try { - return handler.apply(execute(request, context), request); + return handler.apply(execute(request, contextWithTimeout(builder)), request); } catch (IOException e) { catcher.handle(e, request); @@ -104,6 +108,29 @@ public abstract class AbstractHttpClient implements HttpClient { throw new IllegalStateException("No hosts to perform the request against"); } + private HttpClientContext contextWithTimeout(RequestBuilder builder) { + HttpClientContext context = HttpClientContext.create(); + RequestConfig config = builder.config; + if (builder.deadline != null) { + Optional<Duration> remaining = builder.deadline.timeLeftOrThrow(); + if (remaining.isPresent()) { + config = RequestConfig.copy(config) + .setConnectTimeout(min(config.getConnectTimeout(), remaining.get())) + .setConnectionRequestTimeout(min(config.getConnectionRequestTimeout(), remaining.get())) + .setResponseTimeout(min(config.getResponseTimeout(), remaining.get())) + .build(); + } + } + context.setRequestConfig(config); + return context; + } + + // TimeBudget guarantees remaining duration is positive. + static Timeout min(Timeout first, Duration second) { + long firstMillis = first == null || first.isDisabled() ? second.toMillis() : first.toMilliseconds(); + return Timeout.ofMilliseconds(Math.min(firstMillis, second.toMillis())); + } + @Override public HttpClient.RequestBuilder send(HostStrategy hosts, Method method) { return new RequestBuilder(hosts, method); @@ -120,6 +147,7 @@ public abstract class AbstractHttpClient implements HttpClient { private RequestConfig config = HttpClient.defaultRequestConfig; private ResponseVerifier verifier = HttpClient.throwOnError; private ExceptionHandler catcher = HttpClient.retryAll; + private TimeBudget deadline; private RequestBuilder(HostStrategy hosts, Method method) { if ( ! hosts.iterator().hasNext()) @@ -182,6 +210,12 @@ public abstract class AbstractHttpClient implements HttpClient { } @Override + public HttpClient.RequestBuilder deadline(TimeBudget deadline) { + this.deadline = requireNonNull(deadline); + return this; + } + + @Override public RequestBuilder config(RequestConfig config) { this.config = requireNonNull(config); return this; diff --git a/http-client/src/main/java/ai/vespa/hosted/client/HttpClient.java b/http-client/src/main/java/ai/vespa/hosted/client/HttpClient.java index ff29a51165c..f5805ce5b94 100644 --- a/http-client/src/main/java/ai/vespa/hosted/client/HttpClient.java +++ b/http-client/src/main/java/ai/vespa/hosted/client/HttpClient.java @@ -4,6 +4,7 @@ package ai.vespa.hosted.client; import ai.vespa.http.HttpURL; import ai.vespa.http.HttpURL.Path; import ai.vespa.http.HttpURL.Query; +import com.yahoo.time.TimeBudget; import org.apache.hc.client5.http.config.RequestConfig; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; @@ -19,6 +20,7 @@ import java.io.InputStream; import java.io.UncheckedIOException; import java.net.URI; import java.time.Duration; +import java.time.Instant; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -100,6 +102,12 @@ public interface HttpClient extends Closeable { /** Overrides the default socket read timeout of the request. {@code Duration.ZERO} gives infinite timeout. */ RequestBuilder timeout(Duration timeout); + /** + * Pseudo-deadline for the request, including retries. + * Pseudo- because it only ensures request timeouts are low enough to honour the deadline, but nothing else. + */ + RequestBuilder deadline(TimeBudget deadline); + /** Overrides the default request config of the request. */ RequestBuilder config(RequestConfig config); diff --git a/http-client/src/main/java/ai/vespa/hosted/client/MockHttpClient.java b/http-client/src/main/java/ai/vespa/hosted/client/MockHttpClient.java index 09f618f70cb..6c2a882f990 100644 --- a/http-client/src/main/java/ai/vespa/hosted/client/MockHttpClient.java +++ b/http-client/src/main/java/ai/vespa/hosted/client/MockHttpClient.java @@ -1,6 +1,7 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package ai.vespa.hosted.client; +import ai.vespa.http.HttpURL; import org.apache.hc.client5.http.protocol.HttpClientContext; import org.apache.hc.core5.http.ClassicHttpRequest; import org.apache.hc.core5.http.ClassicHttpResponse; @@ -8,11 +9,17 @@ import org.apache.hc.core5.http.ContentType; import org.apache.hc.core5.http.io.entity.HttpEntities; import org.apache.hc.core5.http.message.BasicClassicHttpResponse; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.net.URISyntaxException; +import java.nio.charset.StandardCharsets; import java.util.ArrayDeque; import java.util.Deque; +import java.util.function.BiFunction; import java.util.function.Function; +import static java.nio.charset.StandardCharsets.UTF_8; + /** * @author jonmv */ @@ -39,7 +46,7 @@ public class MockHttpClient extends AbstractHttpClient { expectations.add(expectation); } - public void expect(int status, Function<ClassicHttpRequest, String> mapper) { + public void expect(Function<ClassicHttpRequest, String> mapper, int status) { expect(request -> { BasicClassicHttpResponse response = new BasicClassicHttpResponse(status); response.setEntity(HttpEntities.create(mapper.apply(request), ContentType.APPLICATION_JSON)); @@ -47,6 +54,22 @@ public class MockHttpClient extends AbstractHttpClient { }); } + public void expect(BiFunction<HttpURL, String, String> mapper, int status) { + expect(request -> { + try { + BasicClassicHttpResponse response = new BasicClassicHttpResponse(status); + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + request.getEntity().writeTo(buffer); + response.setEntity(HttpEntities.create(mapper.apply(HttpURL.from(request.getUri()), buffer.toString(UTF_8)), + ContentType.APPLICATION_JSON)); + return response; + } + catch (URISyntaxException e) { + throw new IllegalArgumentException(e); + } + }); + } + @FunctionalInterface public interface Expectation { diff --git a/http-client/src/test/java/ai/vespa/hosted/client/ApacheHttpClientTest.java b/http-client/src/test/java/ai/vespa/hosted/client/ApacheHttpClientTest.java index 647da68ab07..271b8a11a90 100644 --- a/http-client/src/test/java/ai/vespa/hosted/client/ApacheHttpClientTest.java +++ b/http-client/src/test/java/ai/vespa/hosted/client/ApacheHttpClientTest.java @@ -4,6 +4,9 @@ package ai.vespa.hosted.client; import ai.vespa.hosted.client.HttpClient.HostStrategy; import ai.vespa.hosted.client.HttpClient.ResponseException; import com.github.tomakehurst.wiremock.http.Fault; +import com.yahoo.concurrent.UncheckedTimeoutException; +import com.yahoo.test.ManualClock; +import com.yahoo.time.TimeBudget; import org.apache.hc.client5.http.impl.classic.HttpClients; import org.apache.hc.core5.http.Method; import org.junit.jupiter.api.AfterEach; @@ -14,6 +17,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; import java.io.IOException; import java.io.UncheckedIOException; import java.net.URI; +import java.time.Duration; import java.util.List; import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; @@ -101,6 +105,19 @@ class ApacheHttpClientTest { server.verify(1, getRequestedFor(urlEqualTo("/"))); server.verify(1, anyRequestedFor(anyUrl())); server.resetRequests(); + + // Timeout results in UncheckedTimeoutException + ManualClock clock = new ManualClock(); + TimeBudget budget = TimeBudget.fromNow(clock, Duration.ofSeconds(1)); + clock.advance(Duration.ofSeconds(1)); + UncheckedTimeoutException timeout = assertThrows(UncheckedTimeoutException.class, + () -> client.send(HostStrategy.repeating(URI.create("http://localhost:" + server.port()), 2), + Method.GET) + .deadline(budget) + .discard()); + assertEquals("Time since start PT1S exceeds timeout PT1S", timeout.getMessage()); + server.verify(0, anyRequestedFor(anyUrl())); + server.resetRequests(); } @AfterEach diff --git a/jaxrs_client_utils/OWNERS b/jaxrs_client_utils/OWNERS deleted file mode 100644 index e131dacde49..00000000000 --- a/jaxrs_client_utils/OWNERS +++ /dev/null @@ -1 +0,0 @@ -hakonhall diff --git a/jaxrs_client_utils/README b/jaxrs_client_utils/README deleted file mode 100644 index 038a4830a74..00000000000 --- a/jaxrs_client_utils/README +++ /dev/null @@ -1,2 +0,0 @@ -Utilities for client side JAX-RS. - diff --git a/jaxrs_client_utils/README.md b/jaxrs_client_utils/README.md deleted file mode 100644 index 3e6d80d75b5..00000000000 --- a/jaxrs_client_utils/README.md +++ /dev/null @@ -1,3 +0,0 @@ -<!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> -# Utilities for client-side JAX-RS -Code to make it simpler to connect to JAX-RS services (aka REST APIs). diff --git a/jaxrs_client_utils/pom.xml b/jaxrs_client_utils/pom.xml deleted file mode 100644 index 13f2237e088..00000000000 --- a/jaxrs_client_utils/pom.xml +++ /dev/null @@ -1,129 +0,0 @@ -<?xml version="1.0"?> -<!-- Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. --> -<project xmlns="http://maven.apache.org/POM/4.0.0" - xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" - xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 - http://maven.apache.org/xsd/maven-4.0.0.xsd"> - <modelVersion>4.0.0</modelVersion> - <parent> - <groupId>com.yahoo.vespa</groupId> - <artifactId>parent</artifactId> - <version>7-SNAPSHOT</version> - <relativePath>../parent/pom.xml</relativePath> - </parent> - <artifactId>jaxrs_client_utils</artifactId> - <version>7-SNAPSHOT</version> - <packaging>container-plugin</packaging> - <name>${project.artifactId}</name> - <dependencies> - <!-- provided --> - <dependency> - <groupId>com.fasterxml.jackson.jaxrs</groupId> - <artifactId>jackson-jaxrs-json-provider</artifactId> - <scope>provided</scope> - </dependency> - <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>vespajlib</artifactId> - <version>${project.version}</version> - <scope>provided</scope> - </dependency> - <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>annotations</artifactId> - <version>${project.version}</version> - <scope>provided</scope> - </dependency> - <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>application-model</artifactId> - <version>${project.version}</version> - <scope>provided</scope> - </dependency> - <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>security-utils</artifactId> - <version>${project.version}</version> - <scope>provided</scope> - </dependency> - <dependency> - <groupId>javax.ws.rs</groupId> - <artifactId>javax.ws.rs-api</artifactId> - <version>2.0</version> - <scope>provided</scope> - </dependency> - <dependency> - <groupId>org.glassfish.jersey.core</groupId> - <artifactId>jersey-client</artifactId> - <scope>provided</scope> - </dependency> - <dependency> - <groupId>org.glassfish.jersey.ext</groupId> - <artifactId>jersey-proxy-client</artifactId> - <scope>provided</scope> - </dependency> - - <!-- test --> - <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>jaxrs_utils</artifactId> - <version>${project.version}</version> - <scope>test</scope> - </dependency> - <dependency> - <groupId>com.yahoo.vespa</groupId> - <artifactId>defaults</artifactId> - <version>${project.version}</version> - <scope>test</scope> - </dependency> - <dependency> - <groupId>org.glassfish.jersey.test-framework.providers</groupId> - <artifactId>jersey-test-framework-provider-grizzly2</artifactId> - <version>${jersey2.version}</version> - <scope>test</scope> - </dependency> - <dependency> - <groupId>junit</groupId> - <artifactId>junit</artifactId> - <scope>test</scope> - </dependency> - <dependency> - <groupId>org.mockito</groupId> - <artifactId>mockito-core</artifactId> - <scope>test</scope> - </dependency> - <dependency> - <groupId>com.sun.activation</groupId> - <artifactId>javax.activation</artifactId> - <scope>test</scope> - </dependency> - <dependency> - <groupId>javax.xml.bind</groupId> - <artifactId>jaxb-api</artifactId> - <scope>test</scope> - </dependency> - </dependencies> - <build> - <plugins> - <plugin> - <groupId>com.yahoo.vespa</groupId> - <artifactId>bundle-plugin</artifactId> - <extensions>true</extensions> - </plugin> - <plugin> - <groupId>org.apache.maven.plugins</groupId> - <artifactId>maven-compiler-plugin</artifactId> - </plugin> - <plugin> - <groupId>org.apache.maven.plugins</groupId> - <artifactId>maven-surefire-plugin</artifactId> - <configuration> - <!-- Illegal reflective access by HttpPatchTest via JerseyInvocation --> - <argLine> - --add-opens=java.base/java.net=ALL-UNNAMED - </argLine> - </configuration> - </plugin> - </plugins> - </build> -</project> diff --git a/jaxrs_client_utils/src/main/java/ai/vespa/util/http/VespaClientBuilderFactory.java b/jaxrs_client_utils/src/main/java/ai/vespa/util/http/VespaClientBuilderFactory.java deleted file mode 100644 index 1cf5304facd..00000000000 --- a/jaxrs_client_utils/src/main/java/ai/vespa/util/http/VespaClientBuilderFactory.java +++ /dev/null @@ -1,106 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package ai.vespa.util.http; - -import com.yahoo.security.tls.MixedMode; -import com.yahoo.security.tls.TlsContext; -import com.yahoo.security.tls.TransportSecurityUtils; - -import javax.ws.rs.client.ClientBuilder; -import javax.ws.rs.client.ClientRequestContext; -import javax.ws.rs.client.ClientRequestFilter; -import javax.ws.rs.core.UriBuilder; -import java.net.URI; -import java.util.List; -import java.util.Optional; -import java.util.concurrent.CopyOnWriteArrayList; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.logging.Filter; -import java.util.logging.Level; -import java.util.logging.Logger; - -import static java.util.logging.Level.CONFIG; - -/** - * Factory for JAX-RS http client builder for internal Vespa communications over http/https. - * - * Notes: - * - hostname verification is not enabled - CN/SAN verification is assumed to be handled by the underlying x509 trust manager. - * - ssl context or hostname verifier must not be overridden by the caller - * - * @deprecated Use Apache httpclient based client factory instead (VespaHttpClientBuilder). - * @author bjorncs - */ -@Deprecated(forRemoval = true) -public class VespaClientBuilderFactory implements AutoCloseable { - - private static final Logger log = Logger.getLogger(VespaClientBuilderFactory.class.getName()); - - // Keep instances of the Jersey loggers to block the JVM from GCing them in case we initialize the loggers before - // their owner classes (e.g ExecutorProviders) are loaded. - private static final List<Logger> externalJerseyLoggers = new CopyOnWriteArrayList<>(); - - static { - // CONFIG log message are logged repeatedly from these classes. - disableConfigLogging("org.glassfish.jersey.client.internal.HttpUrlConnector"); - disableConfigLogging("org.glassfish.jersey.process.internal.ExecutorProviders"); - disableConfigLogging("athenz.shade.zts.org.glassfish.jersey.client.internal.HttpUrlConnector"); - disableConfigLogging("athenz.shade.zts.org.glassfish.jersey.process.internal.ExecutorProviders"); - } - - // This method will hook a filter into the Jersey logger removing unwanted config messages. - private static void disableConfigLogging(String className) { - @SuppressWarnings("LoggerInitializedWithForeignClass") - Logger logger = Logger.getLogger(className); - Optional<Filter> currentFilter = Optional.ofNullable(logger.getFilter()); - Filter filter = logRecord -> - !logRecord.getLevel().equals(CONFIG) - && currentFilter.map(f -> f.isLoggable(logRecord)).orElse(true); // Honour existing filter if exists - logger.setFilter(filter); - externalJerseyLoggers.add(logger); - } - - - private final TlsContext tlsContext = TransportSecurityUtils.getSystemTlsContext().orElse(null); - private final MixedMode mixedMode = TransportSecurityUtils.getInsecureMixedMode(); - private final AtomicBoolean closed = new AtomicBoolean(false); - - public ClientBuilder newBuilder() { - if (closed.get()) throw new IllegalStateException("Client already closed"); - ClientBuilder builder = ClientBuilder.newBuilder(); - setSslConfiguration(builder); - return builder; - } - - private void setSslConfiguration(ClientBuilder builder) { - if (tlsContext != null) { - builder.sslContext(tlsContext.context()); - builder.hostnameVerifier((hostname, sslSession) -> true); // disable hostname verification - if (mixedMode != MixedMode.PLAINTEXT_CLIENT_MIXED_SERVER) { - builder.register(new UriRewritingRequestFilter()); - } - } - } - - @Override - public void close() { - if (closed.getAndSet(true)) throw new IllegalStateException("Client already closed"); - } - - static class UriRewritingRequestFilter implements ClientRequestFilter { - @Override - public void filter(ClientRequestContext requestContext) { - requestContext.setUri(rewriteUri(requestContext.getUri())); - } - - private static URI rewriteUri(URI originalUri) { - if (!originalUri.getScheme().equals("http")) { - return originalUri; - } - int port = originalUri.getPort(); - int rewrittenPort = port != -1 ? port : 80; - URI rewrittenUri = UriBuilder.fromUri(originalUri).scheme("https").port(rewrittenPort).build(); - log.log(Level.FINE, () -> String.format("Uri rewritten from '%s' to '%s'", originalUri, rewrittenUri)); - return rewrittenUri; - } - } -} diff --git a/jaxrs_client_utils/src/main/java/ai/vespa/util/http/package-info.java b/jaxrs_client_utils/src/main/java/ai/vespa/util/http/package-info.java deleted file mode 100644 index 439c98b5594..00000000000 --- a/jaxrs_client_utils/src/main/java/ai/vespa/util/http/package-info.java +++ /dev/null @@ -1,8 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -/** - * @author bjorncs - */ -@ExportPackage -package ai.vespa.util.http; - -import com.yahoo.osgi.annotation.ExportPackage;
\ No newline at end of file diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsClientFactory.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsClientFactory.java deleted file mode 100644 index f6e044a497a..00000000000 --- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsClientFactory.java +++ /dev/null @@ -1,57 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.jaxrs.client; - -import com.yahoo.vespa.applicationmodel.HostName; - -import java.net.URI; -import java.time.Duration; - -/** - * Interface for creating a JAX-RS client API instance for a single server endpoint. - * - * @author bakksjo - */ -public interface JaxRsClientFactory { - class Params<T> { - private final Class<T> apiClass; - private final URI uri; - - private Duration connectTimeout = Duration.ofSeconds(30); - private Duration readTimeout = Duration.ofSeconds(30); - - public Params(Class<T> apiClass, URI uri) { - this.apiClass = apiClass; - this.uri = uri; - } - - public Class<T> apiClass() { - return apiClass; - } - - public URI uri() { - return uri; - } - - public void setConnectTimeout(Duration timeout) { - this.connectTimeout = timeout; - } - - public Duration connectTimeout() { - return connectTimeout; - } - - public void setReadTimeout(Duration timeout) { - readTimeout = timeout; - } - - public Duration readTimeout() { - return readTimeout; - } - } - - default <T> T createClient(Params<T> params) { - return createClient(params.apiClass, new HostName(params.uri.getHost()), params.uri.getPort(), params.uri.getPath(), params.uri.getScheme()); - } - - <T> T createClient(Class<T> apiClass, HostName hostName, int port, String pathPrefix, String scheme); -} diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategy.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategy.java deleted file mode 100644 index 5ecd001c0fe..00000000000 --- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategy.java +++ /dev/null @@ -1,18 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.jaxrs.client; - -import java.io.IOException; -import java.util.function.Function; - -/** - * This interface allows different strategies for accessing server-side JAX-RS APIs programmatically. - * - * @author bakksjo - */ -public interface JaxRsStrategy<T> { - <R> R apply(final Function<T, R> function) throws IOException; - - default <R> R apply(final Function<T, R> function, JaxRsTimeouts timeouts) throws IOException { - return apply(function); - } -} diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategyFactory.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategyFactory.java deleted file mode 100644 index ffcf1c5d707..00000000000 --- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsStrategyFactory.java +++ /dev/null @@ -1,91 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.jaxrs.client; - -import com.yahoo.vespa.applicationmodel.HostName; - -import java.util.Collection; -import java.util.Iterator; -import java.util.List; -import java.util.Objects; -import java.util.Set; -import java.util.concurrent.ThreadLocalRandom; - -/** - * The idea behind this class is twofold: - * - * <ol> - * <li> - * It can provide alternative strategies for communicating with a JAX-RS-based server API. - * </li> - * <li> - * It can make it simpler to work with hosts that serve multiple APIs. Example: - * <pre>{@code - * final JaxRsStrategyFactory apiFactory = new JaxRsStrategyFactory(hostNames, port, clientFactory); - * // No need to repeat the hostNames etc here: - * apiFactory.apiWithRetries(FooApi.class).apply(FooApi::fooMethod); - * apiFactory.apiWithRetries(BarApi.class).apply(BarApi::barMethod); - * apiFactory.apiWithRetries(BazongaApi.class).apply(BazongaApi::bazinga); - * }</pre> - * </li> - * </ol> - * - * @author bakksjo - */ -public class JaxRsStrategyFactory { - private final Set<HostName> hostNames; - private int port; - private final String scheme; - private final JaxRsClientFactory jaxRsClientFactory; - - // TODO: We might need to support per-host port specification. - public JaxRsStrategyFactory( - final Set<HostName> hostNames, - final int port, - final JaxRsClientFactory jaxRsClientFactory, - String scheme) { - if (hostNames.isEmpty()) { - throw new IllegalArgumentException("hostNames argument must not be empty"); - } - Objects.requireNonNull(jaxRsClientFactory, "jaxRsClientFactory argument may not be null"); - this.hostNames = hostNames; - this.port = port; - this.jaxRsClientFactory = jaxRsClientFactory; - this.scheme = scheme; - } - - public <T> RetryingJaxRsStrategy<T> apiWithRetries(final Class<T> apiClass, final String pathPrefix) { - Objects.requireNonNull(apiClass, "apiClass argument may not be null"); - Objects.requireNonNull(pathPrefix, "pathPrefix argument may not be null"); - return new RetryingJaxRsStrategy<T>(hostNames, port, jaxRsClientFactory, apiClass, pathPrefix, scheme); - } - - public <T> JaxRsStrategy<T> apiNoRetries(final Class<T> apiClass, final String pathPrefix) { - Objects.requireNonNull(apiClass, "apiClass argument may not be null"); - Objects.requireNonNull(pathPrefix, "pathPrefix argument may not be null"); - final HostName hostName = getRandom(hostNames); - return new NoRetryJaxRsStrategy<T>(hostName, port, jaxRsClientFactory, apiClass, pathPrefix, scheme); - } - - private static <T> T getRandom(final Collection<? extends T> collection) { - int index = ThreadLocalRandom.current().nextInt(collection.size()); - return getIndex(collection, index); - } - - private static <T> T getIndex(final Collection<? extends T> collection, final int index) { - if (index >= collection.size() || index < 0) { - throw new IndexOutOfBoundsException( - "Attempt to get element #" + index + " from collection with " + collection.size() + " elements"); - } - - if (collection instanceof List) { - final List<? extends T> list = (List<? extends T>) collection; - return list.get(index); - } - - final Iterator<? extends T> iterator = collection.iterator(); - for (int i = 0; i < index; i++) { - iterator.next(); - } - return iterator.next(); - } -} diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsTimeouts.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsTimeouts.java deleted file mode 100644 index bd498dc02df..00000000000 --- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JaxRsTimeouts.java +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.jaxrs.client; - -import java.time.Duration; - -/** - * @author hakonhall - */ -public interface JaxRsTimeouts { - /** - * The connect timeout, which must be at least 1ms. Called once per real REST call. - * - * @throws com.yahoo.concurrent.UncheckedTimeoutException on timeout. - */ - Duration getConnectTimeoutOrThrow(); - - /** - * The read timeout, which must be at least 1ms. Called once per real REST call. - * - * @throws com.yahoo.concurrent.UncheckedTimeoutException on timeout. - */ - Duration getReadTimeoutOrThrow(); -} diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JerseyJaxRsClientFactory.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JerseyJaxRsClientFactory.java deleted file mode 100644 index 80eb449591e..00000000000 --- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/JerseyJaxRsClientFactory.java +++ /dev/null @@ -1,68 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.jaxrs.client; - -import com.yahoo.vespa.applicationmodel.HostName; -import org.glassfish.jersey.client.ClientProperties; -import org.glassfish.jersey.client.HttpUrlConnectorProvider; -import org.glassfish.jersey.client.proxy.WebResourceFactory; - -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLContext; -import javax.ws.rs.client.Client; -import javax.ws.rs.client.ClientBuilder; -import javax.ws.rs.client.ClientRequestFilter; -import javax.ws.rs.client.WebTarget; -import javax.ws.rs.core.HttpHeaders; -import javax.ws.rs.core.UriBuilder; -import java.util.Collections; - -/** - * Factory for creating Jersey clients from a JAX-RS resource interface. - * - * @author Oyvind Bakksjo - */ -public class JerseyJaxRsClientFactory implements JaxRsClientFactory { - - // Client is a heavy-weight object with a finalizer so we create only one and re-use it - private final Client client; - - public JerseyJaxRsClientFactory() { - this(null, null, null); - } - - public JerseyJaxRsClientFactory(SSLContext sslContext, HostnameVerifier hostnameVerifier, String userAgent) { - /* - * Configure client with some workarounds for HTTP/JAX-RS/Jersey issues. See: - * https://jersey.java.net/apidocs/latest/jersey/org/glassfish/jersey/client/ClientProperties.html#SUPPRESS_HTTP_COMPLIANCE_VALIDATION - * https://jersey.java.net/apidocs/latest/jersey/org/glassfish/jersey/client/HttpUrlConnectorProvider.html#SET_METHOD_WORKAROUND - */ - ClientBuilder builder = ClientBuilder.newBuilder() - .property(ClientProperties.SUPPRESS_HTTP_COMPLIANCE_VALIDATION, true) // Allow empty PUT. TODO: Fix API. - .property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true) // Allow e.g. PATCH method. - .property(ClientProperties.FOLLOW_REDIRECTS, true); - if (sslContext != null) { - builder.sslContext(sslContext); - } - if (hostnameVerifier != null) { - builder.hostnameVerifier(hostnameVerifier); - } - if (userAgent != null) { - builder.register((ClientRequestFilter) context -> context.getHeaders().put(HttpHeaders.USER_AGENT, Collections.singletonList(userAgent))); - } - this.client = builder.build(); - } - - @Override - public <T> T createClient(Params<T> params) { - WebTarget target = client.target(params.uri()); - target.property(ClientProperties.CONNECT_TIMEOUT, (int) params.connectTimeout().toMillis()); - target.property(ClientProperties.READ_TIMEOUT, (int) params.readTimeout().toMillis()); - return WebResourceFactory.newResource(params.apiClass(), target); - } - - @Override - public <T> T createClient(Class<T> apiClass, HostName hostName, int port, String pathPrefix, String scheme) { - UriBuilder uriBuilder = UriBuilder.fromPath(pathPrefix).host(hostName.s()).port(port).scheme(scheme); - return createClient(new Params<>(apiClass, uriBuilder.build())); - } -} diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/LegacyJaxRsTimeouts.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/LegacyJaxRsTimeouts.java deleted file mode 100644 index ced3650d38c..00000000000 --- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/LegacyJaxRsTimeouts.java +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.jaxrs.client; - -import java.time.Duration; - -/** - * Legacy defaults for timeouts. - * - * Clients should instead define their own JaxRsTimeouts tailored to their use-case. - * - * @author hakonhall - */ -// Immutable -public class LegacyJaxRsTimeouts implements JaxRsTimeouts { - private static final Duration CONNECT_TIMEOUT = Duration.ofSeconds(30); - private static final Duration READ_TIMEOUT = Duration.ofSeconds(30); - - @Override - public Duration getConnectTimeoutOrThrow() { - return CONNECT_TIMEOUT; - } - - @Override - public Duration getReadTimeoutOrThrow() { - return READ_TIMEOUT; - } -} diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/LocalPassThroughJaxRsStrategy.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/LocalPassThroughJaxRsStrategy.java deleted file mode 100644 index 9f9d0d4e1bc..00000000000 --- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/LocalPassThroughJaxRsStrategy.java +++ /dev/null @@ -1,23 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.jaxrs.client; - -import java.io.IOException; -import java.util.function.Function; - -/** - * A {@link JaxRsStrategy} that does not use the network, only forwards calls to a local object. - * - * @author bakksjo - */ -public class LocalPassThroughJaxRsStrategy<T> implements JaxRsStrategy<T> { - private final T api; - - public LocalPassThroughJaxRsStrategy(final T api) { - this.api = api; - } - - @Override - public <R> R apply(final Function<T, R> function) throws IOException { - return function.apply(api); - } -} diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/NoRetryJaxRsStrategy.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/NoRetryJaxRsStrategy.java deleted file mode 100644 index 31f03396c5a..00000000000 --- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/NoRetryJaxRsStrategy.java +++ /dev/null @@ -1,52 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.jaxrs.client; - -import com.yahoo.vespa.applicationmodel.HostName; - -import javax.ws.rs.ProcessingException; -import java.io.IOException; -import java.util.Objects; -import java.util.function.Function; - -/** - * A {@link JaxRsStrategy} that will try API calls once against a single server, giving up immediately on failure. - * - * @author bakksjo - */ -public class NoRetryJaxRsStrategy<T> implements JaxRsStrategy<T> { - private final HostName hostName; - private final int port; - private final JaxRsClientFactory jaxRsClientFactory; - private final Class<T> apiClass; - private final String scheme; - private String pathPrefix; - - public NoRetryJaxRsStrategy( - final HostName hostName, - final int port, - final JaxRsClientFactory jaxRsClientFactory, - final Class<T> apiClass, - final String pathPrefix, - String scheme) { - Objects.requireNonNull(hostName, "hostName argument may not be null"); - Objects.requireNonNull(jaxRsClientFactory, "jaxRsClientFactory argument may not be null"); - Objects.requireNonNull(apiClass, "apiClass argument may not be null"); - Objects.requireNonNull(pathPrefix, "pathPrefix argument may not be null"); - this.hostName = hostName; - this.port = port; - this.jaxRsClientFactory = jaxRsClientFactory; - this.apiClass = apiClass; - this.pathPrefix = pathPrefix; - this.scheme = scheme; - } - - @Override - public <R> R apply(final Function<T, R> function) throws IOException { - final T jaxRsClient = jaxRsClientFactory.createClient(apiClass, hostName, port, pathPrefix, scheme); - try { - return function.apply(jaxRsClient); - } catch (ProcessingException e) { - throw new IOException("Communication with REST server failed", e); - } - } -} diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategy.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategy.java deleted file mode 100644 index a185aa9cf5c..00000000000 --- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategy.java +++ /dev/null @@ -1,107 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.jaxrs.client; - -import com.yahoo.vespa.applicationmodel.HostName; - -import javax.ws.rs.ProcessingException; -import javax.ws.rs.ServiceUnavailableException; -import javax.ws.rs.core.UriBuilder; -import java.io.IOException; -import java.net.URI; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.Objects; -import java.util.Set; -import java.util.function.Function; -import java.util.logging.Level; -import java.util.logging.Logger; - -/** - * A {@link JaxRsStrategy} that will retry on failures, looping twice over all available server hosts before giving up. - * - * @author bakksjo - */ -public class RetryingJaxRsStrategy<T> implements JaxRsStrategy<T> { - private static final Logger logger = Logger.getLogger(RetryingJaxRsStrategy.class.getName()); - - private final List<HostName> hostNames; - private final int port; - private final JaxRsClientFactory jaxRsClientFactory; - private final Class<T> apiClass; - private final String pathPrefix; - private final String scheme; - - private int maxIterations = 2; - - public RetryingJaxRsStrategy( - final Set<HostName> hostNames, - final int port, - final JaxRsClientFactory jaxRsClientFactory, - final Class<T> apiClass, - final String pathPrefix, - String scheme) { - if (hostNames.isEmpty()) { - throw new IllegalArgumentException("hostNames argument must not be empty"); - } - Objects.requireNonNull(jaxRsClientFactory, "jaxRsClientFactory argument may not be null"); - Objects.requireNonNull(apiClass, "apiClass argument may not be null"); - Objects.requireNonNull(pathPrefix, "pathPrefix argument may not be null"); - this.hostNames = new ArrayList<>(hostNames); - Collections.shuffle(this.hostNames); - this.port = port; - this.jaxRsClientFactory = jaxRsClientFactory; - this.apiClass = apiClass; - this.pathPrefix = pathPrefix; - this.scheme = scheme; - } - - /** - * The the max number of times the hostnames should be iterated over, before giving up. - * - * <p>By default, maxIterations is 2. - */ - public RetryingJaxRsStrategy<T> setMaxIterations(int maxIterations) { - this.maxIterations = maxIterations; - return this; - } - - @Override - public <R> R apply(final Function<T, R> function) throws IOException { - return apply(function, new LegacyJaxRsTimeouts()); - } - - - @Override - public <R> R apply(final Function<T, R> function, JaxRsTimeouts timeouts) throws IOException { - RuntimeException sampleException = null; - - for (int i = 0; i < maxIterations; ++i) { - for (final HostName hostName : hostNames) { - URI uri = UriBuilder.fromPath(pathPrefix).port(port).scheme(scheme).host(hostName.s()).build(); - JaxRsClientFactory.Params<T> params = new JaxRsClientFactory.Params<>(apiClass, uri); - params.setConnectTimeout(timeouts.getConnectTimeoutOrThrow()); - params.setReadTimeout(timeouts.getReadTimeoutOrThrow()); - final T jaxRsClient = jaxRsClientFactory.createClient(params); - try { - return function.apply(jaxRsClient); - } catch (ProcessingException | ServiceUnavailableException e) { - // E.g. java.net.SocketTimeoutException thrown on read timeout is wrapped as a ProcessingException, - // while ServiceUnavailableException is a WebApplicationException - String message = "Failed REST API call to " + uri + " (in retry loop):"; - sampleException = new RuntimeException(message, e); - logger.log(Level.INFO, message + e.getMessage()); - } - } - } - - final String message = String.format( - "Giving up invoking REST API after %d tries against hosts %s.%s", - maxIterations, - hostNames, - sampleException == null ? "" : ", sample error: " + sampleException.getMessage()); - - assert sampleException != null; - throw new IOException(message, sampleException); - } -} diff --git a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/VespaJerseyJaxRsClientFactory.java b/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/VespaJerseyJaxRsClientFactory.java deleted file mode 100644 index 5ea390dc290..00000000000 --- a/jaxrs_client_utils/src/main/java/com/yahoo/vespa/jaxrs/client/VespaJerseyJaxRsClientFactory.java +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.jaxrs.client; - -import ai.vespa.util.http.VespaClientBuilderFactory; -import com.fasterxml.jackson.jaxrs.json.JacksonJsonProvider; -import com.yahoo.vespa.applicationmodel.HostName; -import org.glassfish.jersey.client.ClientProperties; -import org.glassfish.jersey.client.HttpUrlConnectorProvider; -import org.glassfish.jersey.client.proxy.WebResourceFactory; - -import javax.ws.rs.client.Client; -import javax.ws.rs.client.ClientRequestFilter; -import javax.ws.rs.client.WebTarget; -import javax.ws.rs.core.HttpHeaders; -import javax.ws.rs.core.UriBuilder; -import java.util.List; - -/** - * Factory for creating Jersey based Vespa clients from a JAX-RS resource interface. - * - * @deprecated Use Apache httpclient based client factory instead (VespaHttpClientBuilder). - * @author bjorncs - */ -@Deprecated(forRemoval = true) -public class VespaJerseyJaxRsClientFactory implements JaxRsClientFactory, AutoCloseable { - - @SuppressWarnings("removal") - private final VespaClientBuilderFactory clientBuilder = new VespaClientBuilderFactory(); - // Client is a heavy-weight object with a finalizer so we create only one and re-use it - private final Client client; - - public VespaJerseyJaxRsClientFactory(String userAgent) { - this.client = clientBuilder.newBuilder() - .property(ClientProperties.SUPPRESS_HTTP_COMPLIANCE_VALIDATION, true) // Allow empty PUT - .property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true) // Allow e.g. PATCH method. - .property(ClientProperties.FOLLOW_REDIRECTS, true) - .register(JacksonJsonProvider.class) - .register((ClientRequestFilter) context -> context.getHeaders().put(HttpHeaders.USER_AGENT, List.of(userAgent))) - .build(); - } - - @Override - public <T> T createClient(Params<T> params) { - WebTarget target = client.target(params.uri()); - target.property(ClientProperties.CONNECT_TIMEOUT, (int) params.connectTimeout().toMillis()); - target.property(ClientProperties.READ_TIMEOUT, (int) params.readTimeout().toMillis()); - return WebResourceFactory.newResource(params.apiClass(), target); - } - - @Override - public <T> T createClient(Class<T> apiClass, HostName hostName, int port, String pathPrefix, String scheme) { - UriBuilder uriBuilder = UriBuilder.fromPath(pathPrefix).host(hostName.s()).port(port).scheme(scheme); - return createClient(new Params<>(apiClass, uriBuilder.build())); - } - - @Override - public void close() { - clientBuilder.close(); - } -} diff --git a/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/HttpPatchTest.java b/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/HttpPatchTest.java deleted file mode 100644 index 19bd441e29d..00000000000 --- a/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/HttpPatchTest.java +++ /dev/null @@ -1,107 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.jaxrs.client; - -import com.yahoo.vespa.applicationmodel.HostName; -import com.yahoo.vespa.jaxrs.annotation.PATCH; -import org.glassfish.jersey.client.ClientConfig; -import org.glassfish.jersey.client.HttpUrlConnectorProvider; -import org.glassfish.jersey.test.JerseyTest; -import org.junit.Test; - -import javax.ws.rs.GET; -import javax.ws.rs.Path; -import javax.ws.rs.client.Entity; -import javax.ws.rs.core.Application; -import javax.ws.rs.core.Response; -import java.net.URI; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; -import java.util.concurrent.CompletableFuture; -import java.util.concurrent.TimeUnit; - -import static org.junit.Assert.assertEquals; - -/** - * @author bakksjo - */ -public class HttpPatchTest extends JerseyTest { - private final TestResource testResourceSingleton = new TestResource(); - - @Override - protected Application configure() { - return new Application() { - @Override - public Set<Class<?>> getClasses() { - return Collections.emptySet(); - } - - @Override - public Set<Object> getSingletons() { - return new HashSet<>(Arrays.asList(testResourceSingleton)); - } - }; - } - - @Override - protected void configureClient(final ClientConfig config) { - config.getConfiguration().property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true); - } - - private static final String REQUEST_BODY = "Hello there"; - - @Test - public void clientPatchRequest() throws Exception { - final Response response = target(TestResourceApi.PATH) - .request() - .method("PATCH", Entity.text(REQUEST_BODY)); - assertEquals(REQUEST_BODY, testResourceSingleton.invocation.get(60, TimeUnit.SECONDS)); - assertEquals(REQUEST_BODY, response.readEntity(String.class)); - } - - @Test - public void clientPatchRequestUsingProxyClass() throws Exception { - final URI targetUri = target(TestResourceApi.PATH).getUri(); - final HostName apiHost = new HostName(targetUri.getHost()); - final int apiPort = targetUri.getPort(); - final String apiPath = targetUri.getPath(); - - final JaxRsClientFactory jaxRsClientFactory = new JerseyJaxRsClientFactory(); - final JaxRsStrategyFactory factory = new JaxRsStrategyFactory( - Collections.singleton(apiHost), apiPort, jaxRsClientFactory, "http"); - final JaxRsStrategy<TestResourceApi> client = factory.apiNoRetries(TestResourceApi.class, apiPath); - - final String responseBody; - responseBody = client.apply(api -> api.doPatch(REQUEST_BODY)); - - assertEquals(REQUEST_BODY, testResourceSingleton.invocation.get(60, TimeUnit.SECONDS)); - assertEquals(REQUEST_BODY, responseBody); - } - - public interface TestResourceApi { - String PATH = "test"; - - @GET - String getHello(); - - @PATCH - String doPatch(final String body); - } - - @Path(TestResourceApi.PATH) - public static class TestResource implements TestResourceApi { - public final CompletableFuture<String> invocation = new CompletableFuture<>(); - - @GET - public String getHello() { - return "Hello World!"; - } - - @PATCH - public String doPatch(final String body) { - invocation.complete(body); - return body; - } - } -} diff --git a/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/NoRetryJaxRsStrategyTest.java b/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/NoRetryJaxRsStrategyTest.java deleted file mode 100644 index bd9425119c8..00000000000 --- a/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/NoRetryJaxRsStrategyTest.java +++ /dev/null @@ -1,74 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.jaxrs.client; - -import com.yahoo.vespa.applicationmodel.HostName; -import com.yahoo.vespa.defaults.Defaults; -import org.junit.Before; -import org.junit.Test; - -import javax.ws.rs.GET; -import javax.ws.rs.Path; -import javax.ws.rs.ProcessingException; -import java.io.IOException; -import static org.junit.Assert.fail; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyInt; -import static org.mockito.ArgumentMatchers.anyString; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -public class NoRetryJaxRsStrategyTest { - private static final String API_PATH = "/foo/bar"; - - @Path(API_PATH) - private interface TestJaxRsApi { - @GET - @Path("/foo/bar") - String doSomething(); - } - - private static final HostName SERVER_HOST = new HostName("host-1"); - private static final int REST_PORT = Defaults.getDefaults().vespaWebServicePort(); - - private final JaxRsClientFactory jaxRsClientFactory = mock(JaxRsClientFactory.class); - private final TestJaxRsApi mockApi = mock(TestJaxRsApi.class); - private final JaxRsStrategy<TestJaxRsApi> jaxRsStrategy = new NoRetryJaxRsStrategy<>( - SERVER_HOST, REST_PORT, jaxRsClientFactory, TestJaxRsApi.class, API_PATH, "http"); - - @Before - public void setup() { - when(jaxRsClientFactory.createClient(eq(TestJaxRsApi.class), any(HostName.class), anyInt(), anyString(), anyString())) - .thenReturn(mockApi); - } - - @Test - public void noRetryIfNoFailure() throws Exception { - jaxRsStrategy.apply(TestJaxRsApi::doSomething); - - verify(mockApi, times(1)).doSomething(); - - verify(jaxRsClientFactory, times(1)) - .createClient(eq(TestJaxRsApi.class), eq(SERVER_HOST), eq(REST_PORT), eq(API_PATH), eq("http")); - } - - @Test - public void testNoRetryAfterFailure() throws Exception { - // Make the first call fail. - when(mockApi.doSomething()) - .thenThrow(new ProcessingException("Fake timeout induced by test")) - .thenReturn("a response"); - - try { - jaxRsStrategy.apply(TestJaxRsApi::doSomething); - fail("The above statement should throw"); - } catch (IOException e) { - // As expected. - } - - // Check that there was no second attempt. - verify(mockApi, times(1)).doSomething(); - } -} diff --git a/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategyTest.java b/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategyTest.java deleted file mode 100644 index 55a316e2607..00000000000 --- a/jaxrs_client_utils/src/test/java/com/yahoo/vespa/jaxrs/client/RetryingJaxRsStrategyTest.java +++ /dev/null @@ -1,154 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.jaxrs.client; - -import com.yahoo.vespa.applicationmodel.HostName; -import com.yahoo.vespa.defaults.Defaults; -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.ArgumentCaptor; -import org.mockito.Captor; -import org.mockito.junit.MockitoJUnitRunner; -import org.mockito.stubbing.OngoingStubbing; - -import javax.ws.rs.GET; -import javax.ws.rs.Path; -import javax.ws.rs.ProcessingException; -import java.io.IOException; -import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; -import java.util.stream.Collectors; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.atLeast; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -@RunWith(MockitoJUnitRunner.class) -public class RetryingJaxRsStrategyTest { - private static final String API_PATH = "/"; - - @Captor - ArgumentCaptor<JaxRsClientFactory.Params<TestJaxRsApi>> paramsCaptor; - - @Path(API_PATH) - private interface TestJaxRsApi { - @GET - @Path("/foo/bar") - String doSomething(); - } - - private static final Set<HostName> SERVER_HOSTS = new HashSet<>(Arrays.asList( - new HostName("host-1"), - new HostName("host-2"), - new HostName("host-3"))); - private static final int REST_PORT = Defaults.getDefaults().vespaWebServicePort(); - - private final JaxRsClientFactory jaxRsClientFactory = mock(JaxRsClientFactory.class); - private final TestJaxRsApi mockApi = mock(TestJaxRsApi.class); - private final RetryingJaxRsStrategy<TestJaxRsApi> jaxRsStrategy = new RetryingJaxRsStrategy<>( - SERVER_HOSTS, REST_PORT, jaxRsClientFactory, TestJaxRsApi.class, API_PATH, "http"); - - @Before - public void setup() { - when(jaxRsClientFactory.createClient(any())).thenReturn(mockApi); - } - - @Test - public void noRetryIfNoFailure() throws Exception { - jaxRsStrategy.apply(TestJaxRsApi::doSomething); - - verify(mockApi, times(1)).doSomething(); - - verify(jaxRsClientFactory, times(1)).createClient(paramsCaptor.capture()); - JaxRsClientFactory.Params<TestJaxRsApi> params = paramsCaptor.getValue(); - assertEquals(REST_PORT, params.uri().getPort()); - assertEquals(API_PATH, params.uri().getPath()); - assertEquals("http", params.uri().getScheme()); - assertTrue(SERVER_HOSTS.contains(new HostName(params.uri().getHost()))); - } - - @Test - public void testRetryAfterSingleFailure() throws Exception { - // Make the first attempt fail. - when(mockApi.doSomething()) - .thenThrow(new ProcessingException("Fake timeout induced by test")) - .thenReturn("a response"); - - jaxRsStrategy.apply(TestJaxRsApi::doSomething); - - // Check that there was a second attempt. - verify(mockApi, times(2)).doSomething(); - } - - @Test - public void testRetryUsesAllAvailableServers() throws Exception { - when(mockApi.doSomething()) - .thenThrow(new ProcessingException("Fake timeout 1 induced by test")) - .thenThrow(new ProcessingException("Fake timeout 2 induced by test")) - .thenReturn("a response"); - - jaxRsStrategy.apply(TestJaxRsApi::doSomething); - - verify(mockApi, times(3)).doSomething(); - verifyAllServersContacted(jaxRsClientFactory); - } - - @Test - public void testRetryLoopsOverAvailableServers() throws Exception { - when(mockApi.doSomething()) - .thenThrow(new ProcessingException("Fake socket timeout 1 induced by test")) - .thenThrow(new ProcessingException("Fake socket timeout 2 induced by test")) - .thenThrow(new ProcessingException("Fake socket timeout 3 induced by test")) - .thenThrow(new ProcessingException("Fake socket timeout 4 induced by test")) - .thenReturn("a response"); - - jaxRsStrategy.apply(TestJaxRsApi::doSomething); - - verify(mockApi, times(5)).doSomething(); - verifyAllServersContacted(jaxRsClientFactory); - } - - @Test - public void testRetryGivesUpAfterOneLoopOverAvailableServers() { - jaxRsStrategy.setMaxIterations(1); - testRetryGivesUpAfterXIterations(1); - } - - @Test - public void testRetryGivesUpAfterTwoLoopsOverAvailableServers() { - testRetryGivesUpAfterXIterations(2); - } - - private void testRetryGivesUpAfterXIterations(int iterations) { - OngoingStubbing<String> stub = when(mockApi.doSomething()); - for (int i = 0; i < iterations; ++i) { - stub = stub - .thenThrow(new ProcessingException("Fake timeout 1 iteration " + i)) - .thenThrow(new ProcessingException("Fake timeout 2 iteration " + i)) - .thenThrow(new ProcessingException("Fake timeout 3 iteration " + i)); - } - - try { - jaxRsStrategy.apply(TestJaxRsApi::doSomething); - fail("Exception should be thrown from above statement"); - } catch (IOException e) { - // As expected. - } - - verify(mockApi, times(iterations * 3)).doSomething(); - verifyAllServersContacted(jaxRsClientFactory); - } - - private void verifyAllServersContacted(final JaxRsClientFactory jaxRsClientFactory) { - verify(jaxRsClientFactory, atLeast(SERVER_HOSTS.size())).createClient(paramsCaptor.capture()); - final Set<JaxRsClientFactory.Params<TestJaxRsApi>> actualServerHostsContacted = new HashSet<>(paramsCaptor.getAllValues()); - assertEquals(actualServerHostsContacted.stream().map(x -> new HostName(x.uri().getHost())).collect(Collectors.toSet()), SERVER_HOSTS); - } -} diff --git a/orchestrator/pom.xml b/orchestrator/pom.xml index 839342574d5..3052c6f81cd 100644 --- a/orchestrator/pom.xml +++ b/orchestrator/pom.xml @@ -72,6 +72,23 @@ </dependency> <dependency> <groupId>com.yahoo.vespa</groupId> + <artifactId>http-client</artifactId> + <version>${project.version}</version> + <scope>compile</scope> + </dependency> + <dependency> + <groupId>org.apache.httpcomponents.client5</groupId> + <artifactId>httpclient5</artifactId> + <scope>provided</scope> + <exclusions> + <exclusion> + <groupId>org.slf4j</groupId> + <artifactId>slf4j-api</artifactId> + </exclusion> + </exclusions> + </dependency> + <dependency> + <groupId>com.yahoo.vespa</groupId> <artifactId>jaxrs_client_utils</artifactId> <version>${project.version}</version> <scope>compile</scope> diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/ApplicationStateChangeDeniedException.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/ApplicationStateChangeDeniedException.java index dfe2b2ca344..cf2e923d018 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/ApplicationStateChangeDeniedException.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/ApplicationStateChangeDeniedException.java @@ -9,11 +9,8 @@ package com.yahoo.vespa.orchestrator; */ public class ApplicationStateChangeDeniedException extends Exception { - final String reason; - public ApplicationStateChangeDeniedException(String reason) { - super(); - this.reason = reason; + super(reason); } } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java index ddaec86b340..db18a71c805 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/OrchestratorImpl.java @@ -17,7 +17,6 @@ import com.yahoo.vespa.orchestrator.config.OrchestratorConfig; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClient; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory; import com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState; -import com.yahoo.vespa.orchestrator.controller.ClusterControllerStateResponse; import com.yahoo.vespa.orchestrator.model.ApplicationApi; import com.yahoo.vespa.orchestrator.model.ApplicationApiFactory; import com.yahoo.vespa.orchestrator.model.NodeGroup; @@ -37,7 +36,6 @@ import com.yahoo.vespa.orchestrator.status.StatusService; import com.yahoo.vespa.service.monitor.ServiceMonitor; import com.yahoo.yolean.Exceptions; -import java.io.IOException; import java.time.Clock; import java.util.HashMap; import java.util.List; @@ -427,10 +425,7 @@ public class OrchestratorImpl implements Orchestrator { ClusterControllerClient client = clusterControllerClientFactory.createClient(clusterControllers, cluster.clusterId().s()); for (ServiceInstance service : cluster.serviceInstances()) { try { - ClusterControllerStateResponse response = client.setNodeState(context, - VespaModelUtil.getStorageNodeIndex(service.configId()), - MAINTENANCE); - if ( ! response.wasModified) + if ( ! client.setNodeState(context, service.hostName(), VespaModelUtil.getStorageNodeIndex(service.configId()), MAINTENANCE)) return false; } catch (Exception e) { @@ -449,7 +444,7 @@ public class OrchestratorImpl implements Orchestrator { private void setClusterStateInController(OrchestratorContext context, ApplicationInstance application, ClusterControllerNodeState state) - throws ApplicationStateChangeDeniedException, ApplicationIdNotFoundException { + throws ApplicationStateChangeDeniedException { // Get all content clusters for this application Set<ClusterId> contentClusterIds = application.serviceClusters().stream() .filter(VespaModelUtil::isContent) @@ -459,23 +454,8 @@ public class OrchestratorImpl implements Orchestrator { // For all content clusters set in maintenance for (ClusterId clusterId : contentClusterIds) { List<HostName> clusterControllers = VespaModelUtil.getClusterControllerInstancesInOrder(application, clusterId); - ClusterControllerClient client = clusterControllerClientFactory.createClient( - clusterControllers, - clusterId.s()); - try { - ClusterControllerStateResponse response = client.setApplicationState(context, state); - if (!response.wasModified) { - String msg = String.format("Fail to set application %s, cluster name %s to cluster state %s due to: %s", - application.applicationInstanceId(), clusterId, state, response.reason); - throw new ApplicationStateChangeDeniedException(msg); - } - } catch (IOException e) { - throw new ApplicationStateChangeDeniedException(e.getMessage()); - } catch (UncheckedTimeoutException e) { - throw new ApplicationStateChangeDeniedException( - "Timed out while waiting for cluster controllers " + clusterControllers + - " with cluster ID " + clusterId.s() + ": " + e.getMessage()); - } + ClusterControllerClient client = clusterControllerClientFactory.createClient(clusterControllers, clusterId.s()); + client.setApplicationState(context, application.applicationInstanceId(), state); } } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java index fd62f2b4b70..2c31b475b21 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClient.java @@ -1,10 +1,11 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.orchestrator.controller; -import com.yahoo.concurrent.UncheckedTimeoutException; +import com.yahoo.vespa.applicationmodel.ApplicationInstanceId; +import com.yahoo.vespa.applicationmodel.HostName; +import com.yahoo.vespa.orchestrator.ApplicationStateChangeDeniedException; import com.yahoo.vespa.orchestrator.OrchestratorContext; - -import java.io.IOException; +import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; /** * @author bakksjo @@ -14,17 +15,18 @@ public interface ClusterControllerClient { /** * Requests that a cluster controller sets the requested node to the requested state. * - * @throws IOException if there was a problem communicating with the cluster controller - * @throws UncheckedTimeoutException if operation times out + * @return false is this was a probe operation, and permission would be denied. + * @throws HostStateChangeDeniedException if operation fails, or is otherwise disallowed. */ - ClusterControllerStateResponse setNodeState(OrchestratorContext context, int storageNodeIndex, ClusterControllerNodeState wantedState) throws IOException; + boolean setNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, + ClusterControllerNodeState wantedState) throws HostStateChangeDeniedException; /** * Requests that a cluster controller sets all nodes in the cluster to the requested state. * - * @throws IOException if there was a problem communicating with the cluster controller - * @throws UncheckedTimeoutException if operation times out + * @throws ApplicationStateChangeDeniedException if operation fails, or is disallowed. */ - ClusterControllerStateResponse setApplicationState(OrchestratorContext context, ClusterControllerNodeState wantedState) throws IOException; + void setApplicationState(OrchestratorContext context, ApplicationInstanceId applicationId, + ClusterControllerNodeState wantedState) throws ApplicationStateChangeDeniedException; } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java index f4929a4f09e..211b9f9ff0a 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImpl.java @@ -1,92 +1,176 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.orchestrator.controller; +import ai.vespa.hosted.client.HttpClient; +import ai.vespa.hosted.client.HttpClient.HostStrategy; +import ai.vespa.hosted.client.HttpClient.ResponseException; +import ai.vespa.hosted.client.HttpClient.ResponseVerifier; +import ai.vespa.http.DomainName; +import ai.vespa.http.HttpURL; +import ai.vespa.http.HttpURL.Path; +import ai.vespa.http.HttpURL.Query; +import ai.vespa.http.HttpURL.Scheme; import com.yahoo.concurrent.UncheckedTimeoutException; -import com.yahoo.vespa.jaxrs.client.JaxRsStrategy; +import com.yahoo.slime.Cursor; +import com.yahoo.slime.Inspector; +import com.yahoo.slime.Slime; +import com.yahoo.slime.SlimeUtils; +import com.yahoo.vespa.applicationmodel.ApplicationInstanceId; +import com.yahoo.vespa.applicationmodel.HostName; +import com.yahoo.vespa.orchestrator.ApplicationStateChangeDeniedException; import com.yahoo.vespa.orchestrator.OrchestratorContext; +import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; +import com.yahoo.vespa.orchestrator.policy.HostedVespaPolicy; +import com.yahoo.yolean.Exceptions; +import org.apache.hc.core5.http.ClassicHttpRequest; +import org.apache.hc.core5.http.Method; import java.io.IOException; +import java.io.UncheckedIOException; +import java.net.URI; +import java.util.Iterator; +import java.util.List; + +import static java.nio.charset.StandardCharsets.UTF_8; /** * Default implementation of the ClusterControllerClient. * * @author smorgrav */ -public class ClusterControllerClientImpl implements ClusterControllerClient{ +public class ClusterControllerClientImpl implements ClusterControllerClient { + + enum Condition { + FORCE, SAFE; + } public static final String REQUEST_REASON = "Orchestrator"; - private final JaxRsStrategy<ClusterControllerJaxRsApi> clusterControllerApi; + private final HttpClient client; + private final List<HostName> hosts; private final String clusterName; - public ClusterControllerClientImpl(JaxRsStrategy<ClusterControllerJaxRsApi> clusterControllerApi, - String clusterName) { + ClusterControllerClientImpl(HttpClient client, List<HostName> hosts, String clusterName) { this.clusterName = clusterName; - this.clusterControllerApi = clusterControllerApi; + this.hosts = hosts; + this.client = client; } - /** - * Requests that a cluster controller sets the requested node to the requested state. - * - * @throws IOException if there was a problem communicating with the cluster controller - */ @Override - public ClusterControllerStateResponse setNodeState(OrchestratorContext context, - int storageNodeIndex, - ClusterControllerNodeState wantedState) throws IOException { - ClusterControllerStateRequest.State state = new ClusterControllerStateRequest.State(wantedState, REQUEST_REASON); - ClusterControllerStateRequest stateRequest = new ClusterControllerStateRequest( - state, - ClusterControllerStateRequest.Condition.SAFE, - context.isProbe() ? true : null); - ClusterControllerClientTimeouts timeouts = context.getClusterControllerTimeouts(); - + public boolean setNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, ClusterControllerNodeState wantedState) { try { - return clusterControllerApi.apply(api -> api.setNodeState( - clusterName, - storageNodeIndex, - timeouts.getServerTimeoutOrThrow().toMillis() / 1000.0f, - stateRequest), - timeouts); - } catch (IOException | UncheckedTimeoutException e) { - String message = String.format( - "Giving up setting %s for storage node with index %d in cluster %s: %s", - stateRequest, - storageNodeIndex, - clusterName, - e.getMessage()); + Inspector response = client.send(strategyWithTimeout(hosts, context.getClusterControllerTimeouts()), Method.POST) + .at("cluster", "v2", clusterName, "storage", Integer.toString(storageNodeIndex)) + .deadline(context.getClusterControllerTimeouts().readBudget()) + .body(stateChangeRequestBytes(wantedState, Condition.SAFE, context.isProbe())) + .throwing(retryOnRedirect) + .read(SlimeUtils::jsonToSlime).get(); + if ( ! response.field("wasModified").asBool()) { + if (context.isProbe()) + return false; - throw new IOException(message, e); + throw new HostStateChangeDeniedException(host, + HostedVespaPolicy.SET_NODE_STATE_CONSTRAINT, + "Failed to set state to " + wantedState + + " in cluster controller: " + response.field("reason").asString()); + } + return true; + } + catch (ResponseException e) { + throw new HostStateChangeDeniedException(host, + HostedVespaPolicy.SET_NODE_STATE_CONSTRAINT, + "Failed setting node " + storageNodeIndex + " in cluster " + + clusterName + " to state " + wantedState + ": " + e.getMessage()); + } + catch (UncheckedIOException e) { + throw new HostStateChangeDeniedException(host, + HostedVespaPolicy.CLUSTER_CONTROLLER_AVAILABLE_CONSTRAINT, + String.format("Giving up setting %s for storage node with index %d in cluster %s: %s", + wantedState, + storageNodeIndex, + clusterName, + e.getMessage()), + e.getCause()); + } + catch (UncheckedTimeoutException e) { + throw new HostStateChangeDeniedException(host, + HostedVespaPolicy.DEADLINE_CONSTRAINT, + "Timeout while waiting for setNodeState(" + storageNodeIndex + ", " + wantedState + + ") against " + hosts + ": " + e.getMessage(), + e); } } - /** - * Requests that a cluster controller sets all nodes in the cluster to the requested state. - * - * @throws IOException if there was a problem communicating with the cluster controller - */ @Override - public ClusterControllerStateResponse setApplicationState( - OrchestratorContext context, - ClusterControllerNodeState wantedState) throws IOException { - ClusterControllerStateRequest.State state = new ClusterControllerStateRequest.State(wantedState, REQUEST_REASON); - ClusterControllerStateRequest stateRequest = new ClusterControllerStateRequest( - state, ClusterControllerStateRequest.Condition.FORCE, null); - ClusterControllerClientTimeouts timeouts = context.getClusterControllerTimeouts(); - + public void setApplicationState(OrchestratorContext context, ApplicationInstanceId applicationId, + ClusterControllerNodeState wantedState) throws ApplicationStateChangeDeniedException { try { - return clusterControllerApi.apply(api -> api.setClusterState( - clusterName, - timeouts.getServerTimeoutOrThrow().toMillis() / 1000.0f, - stateRequest), - timeouts); - } catch (IOException | UncheckedTimeoutException e) { - final String message = String.format( - "Giving up setting %s for cluster %s", - stateRequest, - clusterName); - - throw new IOException(message, e); + Inspector response = client.send(strategyWithTimeout(hosts, context.getClusterControllerTimeouts()), Method.POST) + .at("cluster", "v2", clusterName) + .deadline(context.getClusterControllerTimeouts().readBudget()) + .body(stateChangeRequestBytes(wantedState, Condition.FORCE, false)) + .throwing(retryOnRedirect) + .read(SlimeUtils::jsonToSlime).get(); + if ( ! response.field("wasModified").asBool()) { + throw new ApplicationStateChangeDeniedException("Failed to set application " + applicationId + ", cluster name " + + clusterName + " to cluster state " + wantedState + " due to: " + + response.field("reason").asString()); + } + } + catch (ResponseException e) { + throw new ApplicationStateChangeDeniedException("Failed to set application " + applicationId + " cluster name " + + clusterName + " to cluster state " + wantedState + " due to: " + e.getMessage()); } + catch (UncheckedIOException e) { + throw new ApplicationStateChangeDeniedException("Failed communicating with cluster controllers " + hosts + + " with cluster ID " + clusterName + ": " + e.getCause().getMessage()); + } + catch (UncheckedTimeoutException e) { + throw new ApplicationStateChangeDeniedException("Timed out while waiting for cluster controllers " + hosts + + " with cluster ID " + clusterName + ": " + e.getMessage()); + } + } + + static byte[] stateChangeRequestBytes(ClusterControllerNodeState wantedState, Condition condition, boolean isProbe) { + Cursor root = new Slime().setObject(); + Cursor stateObject = root.setObject("user"); + stateObject.setString("reason", REQUEST_REASON); + stateObject.setString("state", wantedState.getWireName()); + root.setString("condition", condition.name()); + if (isProbe) root.setBool("probe", true); + return Exceptions.uncheck(() -> SlimeUtils.toJsonBytes(root)); + } + + /** ᕙ༼◕_◕༽ᕤ hack to vary query parameters with retries ᕙ༼◕_◕༽ᕤ */ + static HostStrategy strategyWithTimeout(List<HostName> hosts, ClusterControllerClientTimeouts timeouts) { + return () -> new Iterator<>() { + final Iterator<HostName> wrapped = hosts.iterator(); + @Override public boolean hasNext() { + return wrapped.hasNext(); + } + @Override public URI next() { + return HttpURL.create(Scheme.http, + DomainName.of(wrapped.next().s()), + 19050, + Path.empty(), + Query.empty().set("timeout", Double.toString(timeouts.getServerTimeoutOrThrow().toMillis() * 1e-3))) + .asURI(); + } + }; } + + static final ResponseVerifier retryOnRedirect = new ResponseVerifier() { + @Override + public boolean shouldRetry(int statusCode) { // Need to try the other servers when we get a redirect. + return statusCode < 400 || statusCode == 503; + } + @Override + public RuntimeException toException(int statusCode, byte[] body, ClassicHttpRequest request) { + Inspector root = SlimeUtils.jsonToSlime(body).get(); + String detail = root.field("message").valid() ? root.field("message").asString() + : new String(body, UTF_8); + return new ResponseException("got status code " + statusCode + " for " + request + (detail.isBlank() ? "" : ": " + detail)); + } + }; + } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeouts.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeouts.java index cdae68a0d06..c54b6aae4fe 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeouts.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeouts.java @@ -3,23 +3,23 @@ package com.yahoo.vespa.orchestrator.controller; import com.yahoo.concurrent.UncheckedTimeoutException; import com.yahoo.time.TimeBudget; -import com.yahoo.vespa.jaxrs.client.JaxRsTimeouts; import java.time.Duration; +import java.util.Optional; /** * Calculates various timeouts associated with a REST call from the Orchestrator to the Cluster Controller. * * <p>Timeout handling of HTTP messaging is fundamentally flawed in various Java implementations. * We would like to specify a max time for the whole operation (connect, send request, and receive response). - * Jersey JAX-RS implementation and the Apache HTTP client library provides a way to set the connect timeout C + * The Apache HTTP client library provides a way to set the connect timeout C * and read timeout R. So if the operation takes NR reads, and the writes takes TW time, * the theoretical max time is: T = C + R * NR + TW. With both NR and TW unknown, there's no way to * set a proper C and R.</p> * * @author hakonhall */ -public class ClusterControllerClientTimeouts implements JaxRsTimeouts { +public class ClusterControllerClientTimeouts { static final Duration CONNECT_TIMEOUT = Duration.ofMillis(100); // Time reserved to guarantee that even though the server application honors a server timeout S, // some time will pass before the server sees the timeout, and after it has returned. @@ -36,40 +36,28 @@ public class ClusterControllerClientTimeouts implements JaxRsTimeouts { * A logical call to CC may in fact call the CC several times, if the first ones are down and/or not * the master. * - * @param timeBudget The time budget for a single logical call to the the Cluster Controller. + * @param timeBudget The time budget for a single logical call to the Cluster Controller. */ public ClusterControllerClientTimeouts(TimeBudget timeBudget) { this.timeBudget = timeBudget; } - @Override - public Duration getConnectTimeoutOrThrow() { - return CONNECT_TIMEOUT; - } - - @Override - public Duration getReadTimeoutOrThrow() { - Duration timeLeft = timeBudget.timeLeft().get(); - - // timeLeft = CONNECT_TIMEOUT + readTimeout - Duration readTimeout = timeLeft.minus(CONNECT_TIMEOUT); - - if (readTimeout.toMillis() <= 0) { + public Duration getServerTimeoutOrThrow() { + // readTimeout = DOWNSTREAM_OVERHEAD + serverTimeout + TimeBudget serverBudget = readBudget().withReserved(DOWNSTREAM_OVERHEAD); + if (serverBudget.timeLeft().get().compareTo(MIN_SERVER_TIMEOUT) < 0) throw new UncheckedTimeoutException("Timed out after " + timeBudget.originalTimeout().get()); - } - return readTimeout; + return serverBudget.timeLeft().get(); } - public Duration getServerTimeoutOrThrow() { - // readTimeout = DOWNSTREAM_OVERHEAD + serverTimeout - Duration serverTimeout = getReadTimeoutOrThrow().minus(DOWNSTREAM_OVERHEAD); - - if (serverTimeout.toMillis() < MIN_SERVER_TIMEOUT.toMillis()) { - throw new UncheckedTimeoutException("Timed out after " + timeBudget.originalTimeout().get()); - } + public Duration connectTimeout() { + return CONNECT_TIMEOUT; + } - return serverTimeout; + public TimeBudget readBudget() { + // timeLeft = CONNECT_TIMEOUT + readTimeout + return timeBudget.withReserved(CONNECT_TIMEOUT); } } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerJaxRsApi.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerJaxRsApi.java deleted file mode 100644 index 01e3cc6718a..00000000000 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerJaxRsApi.java +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.orchestrator.controller; - -import javax.ws.rs.Consumes; -import javax.ws.rs.POST; -import javax.ws.rs.Path; -import javax.ws.rs.PathParam; -import javax.ws.rs.Produces; -import javax.ws.rs.QueryParam; -import javax.ws.rs.core.MediaType; - -/** - * @author hakonhall - */ -public interface ClusterControllerJaxRsApi { - - @POST - @Path("/cluster/v2/{clusterName}/storage/{storageNodeIndex}") - @Consumes(MediaType.APPLICATION_JSON) - @Produces(MediaType.APPLICATION_JSON) - ClusterControllerStateResponse setNodeState( - @PathParam("clusterName") String clusterName, - @PathParam("storageNodeIndex") int storageNodeIndex, - @QueryParam("timeout") Float timeoutSeconds, - ClusterControllerStateRequest request); - - @POST - @Path("/cluster/v2/{clusterName}") - @Consumes(MediaType.APPLICATION_JSON) - @Produces(MediaType.APPLICATION_JSON) - ClusterControllerStateResponse setClusterState( - @PathParam("clusterName") String clusterName, - @QueryParam("timeout") Float timeoutSeconds, - ClusterControllerStateRequest request); - -} diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateErrorResponse.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateErrorResponse.java deleted file mode 100644 index b8613073228..00000000000 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateErrorResponse.java +++ /dev/null @@ -1,20 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.orchestrator.controller; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonProperty; - -/** - * Error response from cluster controller. - * - * @author hakonhall - */ -public class ClusterControllerStateErrorResponse { - @JsonProperty("message") - public final String message; - - @JsonCreator - public ClusterControllerStateErrorResponse(@JsonProperty("message") String message) { - this.message = message; - } -} diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateRequest.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateRequest.java deleted file mode 100644 index e08788cb238..00000000000 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateRequest.java +++ /dev/null @@ -1,103 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.orchestrator.controller; - -import com.fasterxml.jackson.annotation.JsonInclude; -import com.fasterxml.jackson.annotation.JsonProperty; - -import javax.annotation.concurrent.Immutable; -import java.util.Collections; -import java.util.Map; -import java.util.Objects; - -/** - * @author hakonhall - */ -@Immutable -@JsonInclude(JsonInclude.Include.NON_NULL) -public class ClusterControllerStateRequest { - - @JsonProperty("state") - public final Map<String, State> state; - - @JsonProperty("condition") - public final Condition condition; - - @JsonProperty("probe") - public final Boolean probe; - - public ClusterControllerStateRequest(State currentState, Condition condition, Boolean probe) { - Map<String, State> state = Collections.singletonMap("user", currentState); - this.state = Collections.unmodifiableMap(state); - this.condition = condition; - this.probe = probe; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - ClusterControllerStateRequest that = (ClusterControllerStateRequest) o; - return Objects.equals(state, that.state) && - condition == that.condition && - Objects.equals(probe, that.probe); - } - - @Override - public int hashCode() { - return Objects.hash(state, condition, probe); - } - - @Override - public String toString() { - return "NodeStateRequest {" - + " condition=" + condition - + " state=" + state - + " }"; - } - - public static class State { - @JsonProperty("state") - public final ClusterControllerNodeState state; - - /** - * The reason the client is making the request to set the node state. - * Useful for logging in the Cluster Controller. - */ - @JsonProperty("reason") - public final String reason; - - public State(ClusterControllerNodeState state, String reason) { - this.state = state; - this.reason = reason; - } - - @Override - public boolean equals(Object object) { - if (!(object instanceof State)) { - return false; - } - - State that = (State) object; - return this.state.equals(that.state) && - this.reason.equals(that.reason); - } - - @Override - public int hashCode() { - int hash = 1; - hash = 17 * hash + state.hashCode(); - hash = 13 * hash + reason.hashCode(); - return hash; - } - - @Override - public String toString() { - return "reason: " + reason + ", state: " + state; - } - } - - public enum Condition { - FORCE, SAFE; - } - -} diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateResponse.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateResponse.java deleted file mode 100644 index 2d7ba3afa7d..00000000000 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerStateResponse.java +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.orchestrator.controller; - -import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonProperty; - -/** - * The response returned by the cluster controller's set-node-state APIs. - * - * @author hakonhall - */ -public class ClusterControllerStateResponse { - - @JsonProperty("wasModified") - public final boolean wasModified; - - @JsonProperty("reason") - public final String reason; - - @JsonCreator - public ClusterControllerStateResponse(@JsonProperty("wasModified") boolean wasModified, - @JsonProperty("reason") String reason) { - this.wasModified = wasModified; - this.reason = reason; - } - -} diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java index c4132d490dc..c98cefc6c86 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactory.java @@ -1,59 +1,60 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.orchestrator.controller; +import ai.vespa.hosted.client.AbstractHttpClient; +import ai.vespa.hosted.client.HttpClient; +import ai.vespa.util.http.hc5.VespaHttpClientBuilder; import com.google.inject.Inject; import com.yahoo.component.AbstractComponent; import com.yahoo.vespa.applicationmodel.HostName; -import com.yahoo.vespa.jaxrs.client.JaxRsStrategy; -import com.yahoo.vespa.jaxrs.client.JaxRsStrategyFactory; -import com.yahoo.vespa.jaxrs.client.VespaJerseyJaxRsClientFactory; -import java.util.HashSet; +import java.io.IOException; import java.util.List; +import java.util.logging.Level; +import java.util.logging.Logger; /** * @author bakksjo */ -@SuppressWarnings("removal") // VespaJerseyJaxRsClientFactory public class RetryingClusterControllerClientFactory extends AbstractComponent implements ClusterControllerClientFactory { - // TODO: Figure this port out dynamically. - public static final int HARDCODED_CLUSTERCONTROLLER_PORT = 19050; - public static final String CLUSTERCONTROLLER_API_PATH = "/"; - public static final String CLUSTERCONTROLLER_SCHEME = "http"; + private static Logger log = Logger.getLogger(RetryingClusterControllerClientFactory.class.getName()); - private final VespaJerseyJaxRsClientFactory jaxRsClientFactory; + private final HttpClient client; @Inject public RetryingClusterControllerClientFactory() { - this(new VespaJerseyJaxRsClientFactory("orchestrator-cluster-controller-client")); + this(AbstractHttpClient.wrapping(VespaHttpClientBuilder.create() + .setUserAgent("orchestrator-cluster-controller-client") + .build())); } - RetryingClusterControllerClientFactory(VespaJerseyJaxRsClientFactory jaxRsClientFactory) { - this.jaxRsClientFactory = jaxRsClientFactory; + RetryingClusterControllerClientFactory(HttpClient client) { + this.client = client; } @Override public ClusterControllerClient createClient(List<HostName> clusterControllers, String clusterName) { - JaxRsStrategy<ClusterControllerJaxRsApi> jaxRsApi = - new JaxRsStrategyFactory( - new HashSet<>(clusterControllers), - HARDCODED_CLUSTERCONTROLLER_PORT, - jaxRsClientFactory, - CLUSTERCONTROLLER_SCHEME) - .apiWithRetries(ClusterControllerJaxRsApi.class, CLUSTERCONTROLLER_API_PATH) - // Use max iteration 1: The JaxRsStrategyFactory will try host 1, 2, then 3: - // - If host 1 responds, it will redirect to master if necessary. Otherwise - // - If host 2 responds, it will redirect to master if necessary. Otherwise - // - If host 3 responds, it may redirect to master if necessary (if they're up - // after all), but more likely there's no quorum and this will fail too. - // If there's only 1 CC, we'll try that one twice. - .setMaxIterations(clusterControllers.size() > 1 ? 1 : 2); - return new ClusterControllerClientImpl(jaxRsApi, clusterName); + List<HostName> hosts = clusterControllers.size() == 1 + // If there's only 1 CC, we'll try that one twice. + ? List.of(clusterControllers.get(0), clusterControllers.get(0)) + // Otherwise, try each host once: + // * if host 1 responds, it will redirect to master if necessary; otherwise + // * if host 2 responds, it will redirect to master if necessary; otherwise + // * if host 3 responds, it may redirect to master if necessary (if they're up + // after all), but more likely there's no quorum and this will fail too. + : List.copyOf(clusterControllers); + return new ClusterControllerClientImpl(client, hosts, clusterName); } @Override public void deconstruct() { - jaxRsClientFactory.close(); + try { + client.close(); + } + catch (IOException e) { + log.log(Level.WARNING, "failed shutting down", e); + } } + } diff --git a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java index d393117d57c..f3e3fd0e674 100644 --- a/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java +++ b/orchestrator/src/main/java/com/yahoo/vespa/orchestrator/model/StorageNodeImpl.java @@ -1,7 +1,6 @@ // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.orchestrator.model; -import com.yahoo.concurrent.UncheckedTimeoutException; import com.yahoo.vespa.applicationmodel.ApplicationInstance; import com.yahoo.vespa.applicationmodel.ClusterId; import com.yahoo.vespa.applicationmodel.ConfigId; @@ -11,14 +10,8 @@ import com.yahoo.vespa.orchestrator.OrchestratorContext; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClient; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory; import com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState; -import com.yahoo.vespa.orchestrator.controller.ClusterControllerStateErrorResponse; -import com.yahoo.vespa.orchestrator.controller.ClusterControllerStateResponse; import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; -import com.yahoo.vespa.orchestrator.policy.HostedVespaPolicy; -import javax.ws.rs.WebApplicationException; -import javax.ws.rs.core.Response; -import java.io.IOException; import java.util.List; import java.util.Objects; import java.util.logging.Level; @@ -69,41 +62,7 @@ public class StorageNodeImpl implements StorageNode { ", node index " + nodeIndex + ", node state " + wantedNodeState); - ClusterControllerStateResponse response; - try { - response = client.setNodeState(context, nodeIndex, wantedNodeState); - } catch (IOException e) { - throw new HostStateChangeDeniedException( - hostName(), - HostedVespaPolicy.CLUSTER_CONTROLLER_AVAILABLE_CONSTRAINT, - "Failed to communicate with cluster controllers " + clusterControllers + ": " + e, - e); - } catch (WebApplicationException e) { - Response webResponse = e.getResponse(); - // Response may contain detail message - ClusterControllerStateErrorResponse errorResponse = webResponse.readEntity(ClusterControllerStateErrorResponse.class); - String detail = errorResponse.message == null ? "" : ": " + errorResponse.message; - throw new HostStateChangeDeniedException( - hostName(), - HostedVespaPolicy.SET_NODE_STATE_CONSTRAINT, - "Failure from cluster controllers " + clusterControllers + " when setting node " + nodeIndex + - " in cluster " + clusterId + " to state " + wantedNodeState + detail, - e); - } catch (UncheckedTimeoutException e) { - throw new HostStateChangeDeniedException( - hostName(), - HostedVespaPolicy.DEADLINE_CONSTRAINT, - "Timeout while waiting for setNodeState(" + nodeIndex + ", " + wantedNodeState + - ") against " + clusterControllers + ": " + e.getMessage(), - e); - } - - if ( ! response.wasModified) { - throw new HostStateChangeDeniedException( - hostName(), - HostedVespaPolicy.SET_NODE_STATE_CONSTRAINT, - "Failed to set state to " + wantedNodeState + " in cluster controller: " + response.reason); - } + client.setNodeState(context, storageService.hostName(), nodeIndex, wantedNodeState); } @Override diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java index eba8d3d4d66..6e8e1eb0ccb 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/OrchestratorImplTest.java @@ -23,7 +23,6 @@ import com.yahoo.vespa.orchestrator.controller.ClusterControllerClient; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactory; import com.yahoo.vespa.orchestrator.controller.ClusterControllerClientFactoryMock; import com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState; -import com.yahoo.vespa.orchestrator.controller.ClusterControllerStateResponse; import com.yahoo.vespa.orchestrator.model.ApplicationApiFactory; import com.yahoo.vespa.orchestrator.model.NodeGroup; import com.yahoo.vespa.orchestrator.policy.BatchHostStateChangeDeniedException; @@ -460,18 +459,16 @@ public class OrchestratorImplTest { applicationApiFactory, flagSource); - ClusterControllerStateResponse accepted = new ClusterControllerStateResponse(true, "OK"); - ClusterControllerStateResponse denied = new ClusterControllerStateResponse(false, "NO"); - when(fooClient.setNodeState(any(), eq(1), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(accepted); - when(fooClient.setNodeState(any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(accepted); - when(barClient.setNodeState(any(), eq(0), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(accepted); - when(barClient.setNodeState(any(), eq(3), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(accepted); + when(fooClient.setNodeState(any(), any(), eq(1), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); + when(fooClient.setNodeState(any(), any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); + when(barClient.setNodeState(any(), any(), eq(0), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); + when(barClient.setNodeState(any(), any(), eq(3), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(true); assertTrue(orchestrator.isQuiescent(id)); - when(fooClient.setNodeState(any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(denied); + when(fooClient.setNodeState(any(), any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenReturn(false); assertFalse(orchestrator.isQuiescent(id)); - when(fooClient.setNodeState(any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenThrow(new RuntimeException()); + when(fooClient.setNodeState(any(), any(), eq(2), eq(ClusterControllerNodeState.MAINTENANCE))).thenThrow(new RuntimeException()); assertFalse(orchestrator.isQuiescent(id)); } diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java index fcd4c8404a3..27928618eeb 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientFactoryMock.java @@ -2,15 +2,16 @@ package com.yahoo.vespa.orchestrator.controller; import com.yahoo.vespa.applicationmodel.ApplicationInstance; +import com.yahoo.vespa.applicationmodel.ApplicationInstanceId; import com.yahoo.vespa.applicationmodel.ClusterId; import com.yahoo.vespa.applicationmodel.HostName; +import com.yahoo.vespa.orchestrator.ApplicationStateChangeDeniedException; import com.yahoo.vespa.orchestrator.DummyServiceMonitor; import com.yahoo.vespa.orchestrator.OrchestratorContext; import com.yahoo.vespa.orchestrator.model.VespaModelUtil; +import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; -import java.io.IOException; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; @@ -22,13 +23,14 @@ import java.util.Set; * @author smorgrav */ public class ClusterControllerClientFactoryMock implements ClusterControllerClientFactory { + Map<String, ClusterControllerNodeState> nodes = new HashMap<>(); public boolean isInMaintenance(ApplicationInstance appInstance, HostName hostName) { try { ClusterId clusterName = VespaModelUtil.getContentClusterName(appInstance, hostName); int storageNodeIndex = VespaModelUtil.getStorageNodeIndex(appInstance, hostName); - String globalMapKey = clusterName.s() + storageNodeIndex; + String globalMapKey = clusterName + "/" + storageNodeIndex; return nodes.getOrDefault(globalMapKey, ClusterControllerNodeState.UP) == ClusterControllerNodeState.MAINTENANCE; } catch (Exception e) { //Catch all - meant to catch cases where the node is not part of a storage cluster @@ -42,7 +44,7 @@ public class ClusterControllerClientFactoryMock implements ClusterControllerClie for (HostName host : hosts) { ClusterId clusterName = VespaModelUtil.getContentClusterName(app, host); int storageNodeIndex = VespaModelUtil.getStorageNodeIndex(app, host); - String globalMapKey = clusterName.s() + storageNodeIndex; + String globalMapKey = clusterName + "/" + storageNodeIndex; nodes.put(globalMapKey, ClusterControllerNodeState.UP); } } @@ -51,23 +53,16 @@ public class ClusterControllerClientFactoryMock implements ClusterControllerClie @Override public ClusterControllerClient createClient(List<HostName> clusterControllers, String clusterName) { return new ClusterControllerClient() { - - @Override - public ClusterControllerStateResponse setNodeState(OrchestratorContext context, int storageNodeIndex, ClusterControllerNodeState wantedState) throws IOException { - nodes.put(clusterName + storageNodeIndex, wantedState); - return new ClusterControllerStateResponse(true, "Yes"); + @Override public boolean setNodeState(OrchestratorContext context, HostName host, int storageNodeIndex, + ClusterControllerNodeState wantedState) throws HostStateChangeDeniedException { + nodes.put(clusterName + "/" + storageNodeIndex, wantedState); + return true; } - - @Override - public ClusterControllerStateResponse setApplicationState(OrchestratorContext context, ClusterControllerNodeState wantedState) throws IOException { - Set<String> keyCopy = new HashSet<>(nodes.keySet()); - for (String s : keyCopy) { - if (s.startsWith(clusterName)) { - nodes.put(s, wantedState); - } - } - return new ClusterControllerStateResponse(true, "It works"); + @Override public void setApplicationState(OrchestratorContext context, ApplicationInstanceId applicationId, + ClusterControllerNodeState wantedState) throws ApplicationStateChangeDeniedException { + nodes.replaceAll((key, state) -> key.startsWith(clusterName + "/") ? wantedState : state); } }; } + } diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImplTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImplTest.java new file mode 100644 index 00000000000..49bd0ebbd97 --- /dev/null +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientImplTest.java @@ -0,0 +1,158 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +package com.yahoo.vespa.orchestrator.controller; + +import ai.vespa.hosted.client.MockHttpClient; +import com.yahoo.test.ManualClock; +import com.yahoo.vespa.applicationmodel.ApplicationInstanceId; +import com.yahoo.vespa.applicationmodel.HostName; +import com.yahoo.vespa.orchestrator.ApplicationStateChangeDeniedException; +import com.yahoo.vespa.orchestrator.OrchestratorContext; +import com.yahoo.vespa.orchestrator.policy.HostStateChangeDeniedException; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import java.io.IOException; +import java.time.Clock; +import java.time.Duration; +import java.util.List; + +import static com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState.DOWN; +import static com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState.MAINTENANCE; +import static com.yahoo.vespa.orchestrator.controller.ClusterControllerNodeState.UP; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; + +public class ClusterControllerClientImplTest { + + final HostName host = new HostName("node"); + final ManualClock clock = new ManualClock(); + final List<HostName> clusterControllers = List.of(new HostName("host1"), new HostName("host2"), new HostName("host3")); + + MockHttpClient wire; + RetryingClusterControllerClientFactory factory; + ClusterControllerClient client; + + @Before + public void setup() { + wire = new MockHttpClient(); + factory = new RetryingClusterControllerClientFactory(wire); + client = factory.createClient(clusterControllers, "cc"); + } + + @After + public void teardown() { + factory.deconstruct(); + } + + @Test + public void verifySetNodeState() { + OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock); + wire.expect((url, body) -> { + assertEquals("http://host1:19050/cluster/v2/cc/storage/2?timeout=9.6", + url.asURI().toString()); + assertEquals("{\"user\":{\"reason\":\"Orchestrator\",\"state\":\"down\"},\"condition\":\"SAFE\"}", + body); + return "{ \"wasModified\": true }"; + }, + 200); + assertTrue(client.setNodeState(context, host, 2, DOWN)); + + clock.advance(Duration.ofSeconds(9)); + wire.expect((url, body) -> { + assertEquals("http://host1:19050/cluster/v2/cc/storage/1?timeout=0.6", + url.asURI().toString()); + assertEquals("{\"user\":{\"reason\":\"Orchestrator\",\"state\":\"down\"},\"condition\":\"SAFE\"}", + body); + return "{ \"wasModified\": false, \"reason\": \"because\" }"; + }, + 200); + assertEquals("Changing the state of node would violate controller-set-node-state: Failed to set state to DOWN in cluster controller: because", + assertThrows(HostStateChangeDeniedException.class, + () -> client.setNodeState(context, host, 1, DOWN)) + .getMessage()); + } + + @Test + public void verifyProbeNodeState() { + wire.expect((url, body) -> { + assertEquals("http://host1:19050/cluster/v2/cc/storage/2?timeout=59.6", + url.asURI().toString()); + assertEquals("{\"user\":{\"reason\":\"Orchestrator\",\"state\":\"maintenance\"},\"condition\":\"SAFE\",\"probe\":true}", + body); + return "{ \"wasModified\": false, \"reason\": \"no reason\" }"; + }, + 200); + assertFalse(client.setNodeState(OrchestratorContext.createContextForBatchProbe(clock), host, 2, MAINTENANCE)); + } + + @Test + public void verifySetApplicationState() { + wire.expect((url, body) -> { + assertEquals("http://host1:19050/cluster/v2/cc?timeout=299.6", + url.asURI().toString()); + assertEquals("{\"user\":{\"reason\":\"Orchestrator\",\"state\":\"up\"},\"condition\":\"FORCE\"}", + body); + return "{ \"message\": \":<\" }"; + }, + 500); + assertEquals("Failed to set application app cluster name cc to cluster state UP due to: " + + "got status code 500 for POST http://host1:19050/cluster/v2/cc?timeout=299.6: :<", + assertThrows(ApplicationStateChangeDeniedException.class, + () -> client.setApplicationState(OrchestratorContext.createContextForAdminOp(clock), + new ApplicationInstanceId("app"), + UP)) + .getMessage()); + } + + @Test + public void verifyRetriesUntilTimeout() { + OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock); + // IOException + wire.expect(request -> { + clock.advance(Duration.ofSeconds(6)); + throw new IOException("Oh no!"); + }); + // Redirect + wire.expect((url, body) -> { + assertEquals("http://host2:19050/cluster/v2/cc/storage/2?timeout=3.6", + url.asURI().toString()); + clock.advance(Duration.ofSeconds(4)); + return ""; + }, 302); + // Timeout + assertEquals("Changing the state of node would violate deadline: Timeout while waiting for setNodeState(2, UP) " + + "against [host1, host2, host3]: Timed out after PT10S", + assertThrows(HostStateChangeDeniedException.class, + () -> client.setNodeState(context, host, 2, UP)) + .getMessage()); + } + + @Test + public void testRetriesUntilExhaustion() { + OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock); + for (int i = 0; i < clusterControllers.size(); i++) { + int j = i + 1; + wire.expect((url, body) -> { + assertEquals("http://host" + j + ":19050/cluster/v2/cc/storage/2?timeout=9.6", + url.asURI().toString()); + return ""; + }, + 503); + } + // All retries exhausted + assertEquals("Changing the state of node would violate controller-set-node-state: Failed setting node 2 in cluster cc to state UP: " + + "got status code 503 for POST http://host1:19050/cluster/v2/cc/storage/2?timeout=9.6", + assertThrows(HostStateChangeDeniedException.class, + () -> client.setNodeState(context, host, 2, UP)) + .getMessage()); + } + +} diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java deleted file mode 100644 index 655b7921612..00000000000 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTest.java +++ /dev/null @@ -1,62 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.orchestrator.controller; - -import com.yahoo.vespa.jaxrs.client.JaxRsStrategy; -import com.yahoo.vespa.jaxrs.client.LocalPassThroughJaxRsStrategy; -import com.yahoo.vespa.orchestrator.OrchestratorContext; -import org.junit.Before; -import org.junit.Test; - -import java.io.IOException; -import java.time.Duration; - -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -public class ClusterControllerClientTest { - private static final String CLUSTER_NAME = "clusterName"; - private static final int STORAGE_NODE_INDEX = 0; - - private final ClusterControllerJaxRsApi clusterControllerApi = mock(ClusterControllerJaxRsApi.class); - private final JaxRsStrategy<ClusterControllerJaxRsApi> strategyMock = new LocalPassThroughJaxRsStrategy<>(clusterControllerApi); - private final ClusterControllerClient clusterControllerClient = new ClusterControllerClientImpl(strategyMock, CLUSTER_NAME); - private final ClusterControllerNodeState wantedState = ClusterControllerNodeState.MAINTENANCE; - private final OrchestratorContext context = mock(OrchestratorContext.class); - private final ClusterControllerClientTimeouts timeouts = mock(ClusterControllerClientTimeouts.class); - private final ClusterControllerStateRequest.State state = new ClusterControllerStateRequest.State(wantedState, ClusterControllerClientImpl.REQUEST_REASON); - - @Before - public void setUp() { - when(context.getClusterControllerTimeouts()).thenReturn(timeouts); - when(context.isProbe()).thenReturn(false); - when(timeouts.getServerTimeoutOrThrow()).thenReturn(Duration.ofSeconds(1)); - } - - @Test - public void correctParametersArePassedThrough() throws IOException { - setNodeStateAndVerify(null); - } - - @Test - public void probingIsCorrectlyPassedThrough() throws IOException { - when(context.isProbe()).thenReturn(true); - setNodeStateAndVerify(true); - } - - private void setNodeStateAndVerify(Boolean expectedProbe) throws IOException { - clusterControllerClient.setNodeState(context, STORAGE_NODE_INDEX, wantedState); - - final ClusterControllerStateRequest expectedNodeStateRequest = new ClusterControllerStateRequest( - state, ClusterControllerStateRequest.Condition.SAFE, expectedProbe); - - verify(clusterControllerApi, times(1)) - .setNodeState( - eq(CLUSTER_NAME), - eq(STORAGE_NODE_INDEX), - eq(1.0f), - eq(expectedNodeStateRequest)); - } -} diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeoutsTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeoutsTest.java index 6f22ff74ad8..9a34ddc7d64 100644 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeoutsTest.java +++ b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/ClusterControllerClientTimeoutsTest.java @@ -17,6 +17,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; public class ClusterControllerClientTimeoutsTest { + // The minimum time that allows for a single RPC to CC. private static final Duration MINIMUM_TIME_LEFT = CONNECT_TIMEOUT .plus(DOWNSTREAM_OVERHEAD) @@ -44,27 +45,26 @@ public class ClusterControllerClientTimeoutsTest { @Test public void makesManyRequestsWithShortProcessingTime() { - assertEquals(Duration.ofMillis(100), timeouts.getConnectTimeoutOrThrow()); - assertEquals(Duration.ofMillis(2900), timeouts.getReadTimeoutOrThrow()); + assertEquals(Duration.ofMillis(100), timeouts.connectTimeout()); + assertEquals(Duration.ofMillis(2900), timeouts.readBudget().timeLeftOrThrow().get()); assertEquals(Duration.ofMillis(2600), timeouts.getServerTimeoutOrThrow()); clock.advance(Duration.ofMillis(100)); - assertEquals(Duration.ofMillis(100), timeouts.getConnectTimeoutOrThrow()); - assertEquals(Duration.ofMillis(2800), timeouts.getReadTimeoutOrThrow()); + assertEquals(Duration.ofMillis(100), timeouts.connectTimeout()); + assertEquals(Duration.ofMillis(2800), timeouts.readBudget().timeLeftOrThrow().get()); assertEquals(Duration.ofMillis(2500), timeouts.getServerTimeoutOrThrow()); clock.advance(Duration.ofMillis(100)); - assertEquals(Duration.ofMillis(100), timeouts.getConnectTimeoutOrThrow()); - assertEquals(Duration.ofMillis(2700), timeouts.getReadTimeoutOrThrow()); + assertEquals(Duration.ofMillis(100), timeouts.connectTimeout()); + assertEquals(Duration.ofMillis(2700), timeouts.readBudget().timeLeftOrThrow().get()); assertEquals(Duration.ofMillis(2400), timeouts.getServerTimeoutOrThrow()); } @Test public void alreadyTimedOut() { clock.advance(Duration.ofSeconds(4)); - try { timeouts.getServerTimeoutOrThrow(); fail(); @@ -89,4 +89,5 @@ public class ClusterControllerClientTimeoutsTest { clock.advance(originalTimeout.minus(MINIMUM_TIME_LEFT)); timeouts.getServerTimeoutOrThrow(); } + } diff --git a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactoryTest.java b/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactoryTest.java deleted file mode 100644 index ab54eb48a1a..00000000000 --- a/orchestrator/src/test/java/com/yahoo/vespa/orchestrator/controller/RetryingClusterControllerClientFactoryTest.java +++ /dev/null @@ -1,56 +0,0 @@ -// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -package com.yahoo.vespa.orchestrator.controller; - -import com.yahoo.test.ManualClock; -import com.yahoo.vespa.applicationmodel.HostName; -import com.yahoo.vespa.jaxrs.client.VespaJerseyJaxRsClientFactory; -import com.yahoo.vespa.orchestrator.OrchestratorContext; -import org.junit.Test; -import org.mockito.ArgumentCaptor; - -import java.io.IOException; -import java.time.Clock; -import java.util.Arrays; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import static org.junit.Assert.assertEquals; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -public class RetryingClusterControllerClientFactoryTest { - private final Clock clock = new ManualClock(); - - @Test - @SuppressWarnings("removal") // VespaJerseyJaxRsClientFactory - public void verifyJerseyCallForSetNodeState() throws IOException { - VespaJerseyJaxRsClientFactory clientFactory = mock(VespaJerseyJaxRsClientFactory.class); - ClusterControllerJaxRsApi api = mock(ClusterControllerJaxRsApi.class); - when(clientFactory.createClient(any())).thenReturn(api); - RetryingClusterControllerClientFactory factory = new RetryingClusterControllerClientFactory(clientFactory); - String clusterName = "clustername"; - HostName host1 = new HostName("host1"); - HostName host2 = new HostName("host2"); - HostName host3 = new HostName("host3"); - List<HostName> clusterControllers = Arrays.asList(host1, host2, host3); - ClusterControllerClient client = factory.createClient(clusterControllers, clusterName); - OrchestratorContext context = OrchestratorContext.createContextForSingleAppOp(clock); - int storageNode = 2; - ClusterControllerNodeState wantedState = ClusterControllerNodeState.MAINTENANCE; - client.setNodeState(context, storageNode, wantedState); - - ArgumentCaptor<ClusterControllerStateRequest> requestCaptor = ArgumentCaptor.forClass(ClusterControllerStateRequest.class); - - verify(api, times(1)).setNodeState(eq(clusterName), eq(storageNode), eq(9.6f), requestCaptor.capture()); - ClusterControllerStateRequest request = requestCaptor.getValue(); - assertEquals(ClusterControllerStateRequest.Condition.SAFE, request.condition); - Map<String, Object> expectedState = new HashMap<>(); - expectedState.put("user", new ClusterControllerStateRequest.State(wantedState, "Orchestrator")); - assertEquals(expectedState, request.state); - } -} @@ -86,7 +86,6 @@ <module>http-utils</module> <module>indexinglanguage</module> <!--<module>integration/intellij</module>--> - <module>jaxrs_client_utils</module> <module>jaxrs_utils</module> <module>jdisc-cloud-aws</module> <module>jdisc-security-filters</module> diff --git a/vespajlib/src/main/java/com/yahoo/time/TimeBudget.java b/vespajlib/src/main/java/com/yahoo/time/TimeBudget.java index 64cf9dd522c..7e6c015bca8 100644 --- a/vespajlib/src/main/java/com/yahoo/time/TimeBudget.java +++ b/vespajlib/src/main/java/com/yahoo/time/TimeBudget.java @@ -60,7 +60,7 @@ public class TimeBudget { Duration passed = timePassed(); Duration left = timeout.minus(passed); if (left.toMillis() <= 0) { - throw new UncheckedTimeoutException("Time since start " + passed + " exceeds timeout " + this.timeout); + throw new UncheckedTimeoutException("Time since start " + passed + " exceeds timeout " + timeout); } return left; @@ -75,8 +75,7 @@ public class TimeBudget { /** Returns the time left as a new TimeBudget. */ public TimeBudget timeLeftAsTimeBudget() { Instant now = clock.instant(); - Optional<Instant> deadline = deadline(); - return new TimeBudget(clock, now, deadline.map(d -> Duration.between(now, d))); + return new TimeBudget(clock, now, deadline().map(d -> Duration.between(now, d))); } /** Returns a new TimeBudget with the same clock and start, but with this deadline. */ @@ -84,6 +83,11 @@ public class TimeBudget { return new TimeBudget(clock, start, Optional.of(Duration.between(start, deadline))); } + /** Returns a new TimeBudget with the given duration chopped off, reserved for something else. */ + public TimeBudget withReserved(Duration chunk) { + return timeout.isEmpty() ? this : new TimeBudget(clock, start, Optional.of(timeout.get().minus(chunk))); + } + private static Duration nonNegativeBetween(Instant start, Instant end) { return makeNonNegative(Duration.between(start, end)); } |