From 82f302b84e17da9ad632e11c01d34c3b9f1e596e Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Mon, 3 Jun 2019 08:17:32 +0100 Subject: [PATCH 1/7] Fix client thread leak after origin reload. Add global event loop for StyxHttpClient. Add clues to ExpiringConnectionSpec. --- .../com/hotels/styx/client/Connection.java | 7 +- .../hotels/styx/client/StyxHttpClient.java | 5 +- .../ExpiringConnectionFactory.java | 8 +- .../NettyConnectionFactory.java | 61 ++++--- .../com/hotels/styx/StyxPipelineFactory.java | 7 +- .../main/java/com/hotels/styx/StyxServer.java | 3 +- .../styx/proxy/BackendServicesRouter.java | 26 +-- .../styx/routing/StaticPipelineFactory.java | 21 ++- .../routing/handlers/BackendServiceProxy.java | 10 +- .../styx/routing/handlers/ProxyToBackend.java | 4 +- .../styx/startup/StyxServerComponents.java | 48 ++++++ .../styx/proxy/BackendServicesRouterTest.java | 29 ++-- .../routing/StaticPipelineBuilderTest.java | 9 +- ...ileBasedOriginsFileChangeMonitorSpec.scala | 135 --------------- .../styx/client/ExpiringConnectionSpec.scala | 9 +- .../hotels/styx/proxy/OriginsReloadSpec.scala | 104 ------------ system-tests/ft-suite/pom.xml | 6 + .../FileBasedOriginsFileChangeMonitorSpec.kt | 130 ++++++++++++++ .../styx/resiliency/OriginResourcesSpec.kt | 160 ++++++++++++++++++ .../hotels/styx/support/StyxServerProvider.kt | 139 +++++++++++++++ 20 files changed, 602 insertions(+), 319 deletions(-) delete mode 100644 system-tests/e2e-suite/src/test/scala/com/hotels/styx/admin/FileBasedOriginsFileChangeMonitorSpec.scala delete mode 100644 system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/OriginsReloadSpec.scala create mode 100644 system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/FileBasedOriginsFileChangeMonitorSpec.kt create mode 100644 system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt create mode 100644 system-tests/ft-suite/src/test/kotlin/com/hotels/styx/support/StyxServerProvider.kt diff --git a/components/client/src/main/java/com/hotels/styx/client/Connection.java b/components/client/src/main/java/com/hotels/styx/client/Connection.java index a8dda473bd..38face57df 100644 --- a/components/client/src/main/java/com/hotels/styx/client/Connection.java +++ b/components/client/src/main/java/com/hotels/styx/client/Connection.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -23,6 +23,7 @@ import java.io.Closeable; import java.util.EventListener; +import java.util.concurrent.CompletableFuture; /** * A connection to an origin. @@ -41,6 +42,10 @@ interface Factory { * @return the newly created connection */ Mono createConnection(Origin origin, ConnectionSettings connectionSettings); + + default CompletableFuture close() { + return CompletableFuture.completedFuture(null); + } } /** diff --git a/components/client/src/main/java/com/hotels/styx/client/StyxHttpClient.java b/components/client/src/main/java/com/hotels/styx/client/StyxHttpClient.java index 1a50022606..730957b196 100644 --- a/components/client/src/main/java/com/hotels/styx/client/StyxHttpClient.java +++ b/components/client/src/main/java/com/hotels/styx/client/StyxHttpClient.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -26,6 +26,7 @@ import com.hotels.styx.api.extension.service.TlsSettings; import com.hotels.styx.client.netty.connectionpool.HttpRequestOperation; import com.hotels.styx.client.netty.connectionpool.NettyConnectionFactory; +import com.hotels.styx.client.netty.eventloop.PlatformAwareClientEventLoopGroupFactory; import com.hotels.styx.client.ssl.SslContextFactory; import io.netty.handler.ssl.SslContext; import reactor.core.publisher.Mono; @@ -339,7 +340,7 @@ Builder copy() { */ public StyxHttpClient build() { NettyConnectionFactory connectionFactory = new NettyConnectionFactory.Builder() - .name(threadName) + .eventLoopGroupFactory(new PlatformAwareClientEventLoopGroupFactory(threadName, 1)) .httpConfig(newHttpConfigBuilder().setMaxHeadersSize(maxHeaderSize).build()) .tlsSettings(tlsSettings) .httpRequestOperationFactory(request -> new HttpRequestOperation( diff --git a/components/client/src/main/java/com/hotels/styx/client/connectionpool/ExpiringConnectionFactory.java b/components/client/src/main/java/com/hotels/styx/client/connectionpool/ExpiringConnectionFactory.java index 6bcf702d57..d8658950e5 100644 --- a/components/client/src/main/java/com/hotels/styx/client/connectionpool/ExpiringConnectionFactory.java +++ b/components/client/src/main/java/com/hotels/styx/client/connectionpool/ExpiringConnectionFactory.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -21,6 +21,7 @@ import com.hotels.styx.client.ConnectionSettings; import reactor.core.publisher.Mono; +import java.util.concurrent.CompletableFuture; import java.util.function.Supplier; import static java.util.Objects.requireNonNull; @@ -47,6 +48,11 @@ public Mono createConnection(Origin origin, ConnectionSettings conne .map(this::decorate); } + @Override + public CompletableFuture close() { + return connectionFactory.close(); + } + private Connection decorate(Connection conn) { return new ExpiringConnection(conn, connectionExpirationSeconds, SYSTEM_TICKER); } diff --git a/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnectionFactory.java b/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnectionFactory.java index 691b7e04a9..75d48e60f0 100644 --- a/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnectionFactory.java +++ b/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnectionFactory.java @@ -23,9 +23,9 @@ import com.hotels.styx.client.ConnectionSettings; import com.hotels.styx.client.HttpConfig; import com.hotels.styx.client.HttpRequestOperationFactory; +import com.hotels.styx.client.netty.ClientEventLoopFactory; import com.hotels.styx.client.netty.eventloop.PlatformAwareClientEventLoopGroupFactory; import com.hotels.styx.client.ssl.SslContextFactory; -import com.hotels.styx.common.CompletableFutures; import io.netty.bootstrap.Bootstrap; import io.netty.buffer.PooledByteBufAllocator; import io.netty.channel.Channel; @@ -40,16 +40,24 @@ import static com.hotels.styx.client.HttpConfig.defaultHttpConfig; import static com.hotels.styx.client.HttpRequestOperationFactory.Builder.httpRequestOperationFactoryBuilder; +import static com.hotels.styx.common.CompletableFutures.fromNettyFuture; import static io.netty.channel.ChannelOption.ALLOCATOR; import static io.netty.channel.ChannelOption.CONNECT_TIMEOUT_MILLIS; import static io.netty.channel.ChannelOption.SO_KEEPALIVE; import static io.netty.channel.ChannelOption.TCP_NODELAY; import static java.util.Objects.requireNonNull; +import static java.util.concurrent.CompletableFuture.completedFuture; /** * A connection factory that creates connections using netty. */ public class NettyConnectionFactory implements Connection.Factory { + private static final PlatformAwareClientEventLoopGroupFactory FACTORY = new PlatformAwareClientEventLoopGroupFactory( + "Styx-Client-Global", + 0); + private static final Class GLOBAL_CLIENT_EVENT_LOOP_CLASS = FACTORY.clientSocketChannelClass(); + private static final EventLoopGroup GLOBAL_CLIENT_EVENT_LOOP = FACTORY.newClientWorkerEventLoopGroup(); + private final HttpConfig httpConfig; private final SslContext sslContext; private final boolean sendSni; @@ -60,14 +68,16 @@ public class NettyConnectionFactory implements Connection.Factory { private Class clientSocketChannelClass; private NettyConnectionFactory(Builder builder) { - PlatformAwareClientEventLoopGroupFactory eventLoopGroupFactory = new PlatformAwareClientEventLoopGroupFactory( - builder.name, - builder.clientWorkerThreadsCount - ); - this.eventLoopGroup = eventLoopGroupFactory.newClientWorkerEventLoopGroup(); + this.clientSocketChannelClass = builder.eventLoopGroupFactory != null + ? builder.eventLoopGroupFactory.clientSocketChannelClass() + : GLOBAL_CLIENT_EVENT_LOOP_CLASS; + + this.eventLoopGroup = builder.eventLoopGroupFactory != null + ? builder.eventLoopGroupFactory.newClientWorkerEventLoopGroup() + : GLOBAL_CLIENT_EVENT_LOOP; + this.httpConfig = requireNonNull(builder.httpConfig); this.sslContext = builder.tlsSettings == null ? null : SslContextFactory.get(builder.tlsSettings); - this.clientSocketChannelClass = eventLoopGroupFactory.clientSocketChannelClass(); this.httpRequestOperationFactory = requireNonNull(builder.httpRequestOperationFactory); this.sendSni = builder.tlsSettings != null && builder.tlsSettings.sendSni(); this.sniHost = builder.tlsSettings != null ? builder.tlsSettings.sniHost() : Optional.empty(); @@ -93,6 +103,15 @@ public Mono createConnection(Origin origin, ConnectionSettings conne }); } + @Override + public CompletableFuture close() { + if (eventLoopGroup == GLOBAL_CLIENT_EVENT_LOOP) { + return completedFuture(null); + } + + return fromNettyFuture(eventLoopGroup.shutdownGracefully()); + } + private ChannelFuture openConnection(Origin origin, ConnectionSettings connectionSettings) { bootstrap(connectionSettings); return bootstrap.connect(origin.host(), origin.port()); @@ -114,10 +133,6 @@ private synchronized void bootstrap(ConnectionSettings connectionSettings) { } } - public CompletableFuture close() { - return CompletableFutures.fromNettyFuture(eventLoopGroup.shutdownGracefully()); - } - private class Initializer extends ChannelInitializer { @Override protected void initChannel(Channel ch) { @@ -129,30 +144,12 @@ protected void initChannel(Channel ch) { */ public static final class Builder { private HttpRequestOperationFactory httpRequestOperationFactory = httpRequestOperationFactoryBuilder().build(); - private String name = "Styx-Client"; - private int clientWorkerThreadsCount = 1; private HttpConfig httpConfig = defaultHttpConfig(); private TlsSettings tlsSettings; + private ClientEventLoopFactory eventLoopGroupFactory; - /** - * Sets the name. - * - * @param name name - * @return this builder - */ - public Builder name(String name) { - this.name = requireNonNull(name); - return this; - } - - /** - * Sets number of client worker threads. - * - * @param clientWorkerThreadsCount number of client worker threads - * @return this builder - */ - public Builder clientWorkerThreadsCount(int clientWorkerThreadsCount) { - this.clientWorkerThreadsCount = clientWorkerThreadsCount; + public Builder eventLoopGroupFactory(ClientEventLoopFactory eventLoopGroupFactory) { + this.eventLoopGroupFactory = eventLoopGroupFactory; return this; } diff --git a/components/proxy/src/main/java/com/hotels/styx/StyxPipelineFactory.java b/components/proxy/src/main/java/com/hotels/styx/StyxPipelineFactory.java index 06ccfa6ed2..2bc99aaf44 100644 --- a/components/proxy/src/main/java/com/hotels/styx/StyxPipelineFactory.java +++ b/components/proxy/src/main/java/com/hotels/styx/StyxPipelineFactory.java @@ -21,6 +21,7 @@ import com.hotels.styx.api.extension.service.BackendService; import com.hotels.styx.api.extension.service.spi.Registry; import com.hotels.styx.api.extension.service.spi.StyxService; +import com.hotels.styx.client.netty.ClientEventLoopFactory; import com.hotels.styx.proxy.plugin.NamedPlugin; import com.hotels.styx.routing.HttpPipelineFactory; import com.hotels.styx.routing.RoutingObject; @@ -51,18 +52,20 @@ public final class StyxPipelineFactory implements PipelineFactory { private final Environment environment; private final Map services; private final List plugins; + private final ClientEventLoopFactory eventLoopGroupFactory; public StyxPipelineFactory( StyxObjectStore routeDb, RoutingObjectFactory routingObjectFactory, Environment environment, Map services, - List plugins) { + List plugins, ClientEventLoopFactory eventLoopGroupFactory) { this.routeDb = requireNonNull(routeDb); this.routingObjectFactory = requireNonNull(routingObjectFactory); this.environment = requireNonNull(environment); this.services = requireNonNull(services); this.plugins = requireNonNull(plugins); + this.eventLoopGroupFactory = requireNonNull(eventLoopGroupFactory); } @Override @@ -87,7 +90,7 @@ private RoutingObject configuredPipeline(RoutingObjectFactory routingObjectFacto }) .orElseGet(() -> { Registry backendServicesRegistry = (Registry) services.get("backendServiceRegistry"); - return new StaticPipelineFactory(environment, backendServicesRegistry, plugins, requestTracking); + return new StaticPipelineFactory(environment, backendServicesRegistry, plugins, eventLoopGroupFactory, requestTracking); }); return pipelineBuilder.build(); diff --git a/components/proxy/src/main/java/com/hotels/styx/StyxServer.java b/components/proxy/src/main/java/com/hotels/styx/StyxServer.java index 542b5d1c66..886a04f21a 100644 --- a/components/proxy/src/main/java/com/hotels/styx/StyxServer.java +++ b/components/proxy/src/main/java/com/hotels/styx/StyxServer.java @@ -150,7 +150,8 @@ public StyxServer(StyxServerComponents components, Stopwatch stopwatch) { components.routingObjectFactory(), components.environment(), components.services(), - components.plugins())); + components.plugins(), + components.eventLoopGroupFactory())); this.proxyServer = proxyServerSetUp.createProxyServer(components); this.adminServer = createAdminServer(components); diff --git a/components/proxy/src/main/java/com/hotels/styx/proxy/BackendServicesRouter.java b/components/proxy/src/main/java/com/hotels/styx/proxy/BackendServicesRouter.java index bf45904689..194fb74a30 100644 --- a/components/proxy/src/main/java/com/hotels/styx/proxy/BackendServicesRouter.java +++ b/components/proxy/src/main/java/com/hotels/styx/proxy/BackendServicesRouter.java @@ -26,6 +26,7 @@ import com.hotels.styx.api.extension.service.BackendService; import com.hotels.styx.api.extension.service.ConnectionPoolSettings; import com.hotels.styx.api.extension.service.HealthCheckConfig; +import com.hotels.styx.api.extension.service.TlsSettings; import com.hotels.styx.api.extension.service.spi.Registry; import com.hotels.styx.client.BackendServiceClient; import com.hotels.styx.client.Connection; @@ -42,6 +43,7 @@ import com.hotels.styx.client.healthcheck.OriginHealthStatusMonitor; import com.hotels.styx.client.healthcheck.OriginHealthStatusMonitorFactory; import com.hotels.styx.client.healthcheck.UrlRequestHealthCheck; +import com.hotels.styx.client.netty.ClientEventLoopFactory; import com.hotels.styx.client.netty.connectionpool.NettyConnectionFactory; import com.hotels.styx.server.HttpRouter; import org.slf4j.Logger; @@ -67,16 +69,17 @@ public class BackendServicesRouter implements HttpRouter, Registry.ChangeListene private final BackendServiceClientFactory clientFactory; private final Environment environment; + private final ClientEventLoopFactory eventLoopGroupFactory; private final ConcurrentMap routes; - private final int clientWorkerThreadsCount; - public BackendServicesRouter(BackendServiceClientFactory clientFactory, Environment environment) { + public BackendServicesRouter(BackendServiceClientFactory clientFactory, Environment environment, ClientEventLoopFactory eventLoopGroupFactory) { this.clientFactory = requireNonNull(clientFactory); - this.environment = environment; + this.environment = requireNonNull(environment); + this.eventLoopGroupFactory = requireNonNull(eventLoopGroupFactory); + this.routes = new ConcurrentSkipListMap<>( comparingInt(String::length).reversed() .thenComparing(naturalOrder())); - this.clientWorkerThreadsCount = environment.styxConfig().proxyServerConfig().clientWorkerThreadsCount(); } ConcurrentMap routes() { @@ -115,7 +118,9 @@ public void onChange(Registry.Changes changes) { ConnectionPoolSettings poolSettings = backendService.connectionPoolConfig(); Connection.Factory connectionFactory = connectionFactory( - backendService, + eventLoopGroupFactory, + backendService.responseTimeoutMillis(), + backendService.tlsSettings().orElse(null), requestLoggingEnabled, longFormat, originStatsFactory, @@ -179,25 +184,26 @@ private StyxHttpClient healthCheckClient(BackendService backendService) { } private Connection.Factory connectionFactory( - BackendService backendService, + ClientEventLoopFactory eventLoopGroupFactory, + int responseTimeoutMillis, + TlsSettings tlsSettings, boolean requestLoggingEnabled, boolean longFormat, OriginStatsFactory originStatsFactory, long connectionExpiration) { Connection.Factory factory = new NettyConnectionFactory.Builder() - .name("Styx") + .eventLoopGroupFactory(eventLoopGroupFactory) .httpRequestOperationFactory( httpRequestOperationFactoryBuilder() .flowControlEnabled(true) .originStatsFactory(originStatsFactory) - .responseTimeoutMillis(backendService.responseTimeoutMillis()) + .responseTimeoutMillis(responseTimeoutMillis) .requestLoggingEnabled(requestLoggingEnabled) .longFormat(longFormat) .build() ) - .clientWorkerThreadsCount(clientWorkerThreadsCount) - .tlsSettings(backendService.tlsSettings().orElse(null)) + .tlsSettings(tlsSettings) .build(); if (connectionExpiration > 0) { diff --git a/components/proxy/src/main/java/com/hotels/styx/routing/StaticPipelineFactory.java b/components/proxy/src/main/java/com/hotels/styx/routing/StaticPipelineFactory.java index 98fcaee123..64c22805ae 100644 --- a/components/proxy/src/main/java/com/hotels/styx/routing/StaticPipelineFactory.java +++ b/components/proxy/src/main/java/com/hotels/styx/routing/StaticPipelineFactory.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ import com.hotels.styx.Environment; import com.hotels.styx.api.extension.service.BackendService; import com.hotels.styx.api.extension.service.spi.Registry; +import com.hotels.styx.client.netty.ClientEventLoopFactory; import com.hotels.styx.proxy.BackendServiceClientFactory; import com.hotels.styx.proxy.BackendServicesRouter; import com.hotels.styx.proxy.InterceptorPipelineBuilder; @@ -26,6 +27,8 @@ import com.hotels.styx.proxy.StyxBackendServiceClientFactory; import com.hotels.styx.proxy.plugin.NamedPlugin; +import static java.util.Objects.requireNonNull; + /** * Builds a static "backwards compatibility" pipeline which is just a sequence of plugins * followed by backend service proxy. @@ -35,6 +38,7 @@ public class StaticPipelineFactory implements HttpPipelineFactory { private final Environment environment; private final Registry registry; private final Iterable plugins; + private final ClientEventLoopFactory eventLoopGroupFactory; private final boolean trackRequests; @VisibleForTesting @@ -42,19 +46,22 @@ public class StaticPipelineFactory implements HttpPipelineFactory { Environment environment, Registry registry, Iterable plugins, + ClientEventLoopFactory eventLoopGroupFactory, boolean trackRequests) { - this.clientFactory = clientFactory; - this.environment = environment; - this.registry = registry; - this.plugins = plugins; + this.clientFactory = requireNonNull(clientFactory); + this.environment = requireNonNull(environment); + this.registry = requireNonNull(registry); + this.plugins = requireNonNull(plugins); this.trackRequests = trackRequests; + this.eventLoopGroupFactory = requireNonNull(eventLoopGroupFactory); } public StaticPipelineFactory(Environment environment, Registry registry, Iterable plugins, + ClientEventLoopFactory eventLoopGroupFactory, boolean trackRequests) { - this(createClientFactory(environment), environment, registry, plugins, trackRequests); + this(createClientFactory(environment), environment, registry, plugins, eventLoopGroupFactory, trackRequests); } private static BackendServiceClientFactory createClientFactory(Environment environment) { @@ -63,7 +70,7 @@ private static BackendServiceClientFactory createClientFactory(Environment envir @Override public RoutingObject build() { - BackendServicesRouter backendServicesRouter = new BackendServicesRouter(clientFactory, environment); + BackendServicesRouter backendServicesRouter = new BackendServicesRouter(clientFactory, environment, eventLoopGroupFactory); registry.addListener(backendServicesRouter); RouteHandlerAdapter router = new RouteHandlerAdapter(backendServicesRouter); diff --git a/components/proxy/src/main/java/com/hotels/styx/routing/handlers/BackendServiceProxy.java b/components/proxy/src/main/java/com/hotels/styx/routing/handlers/BackendServiceProxy.java index 3c19a8ddd3..a94f218fe2 100644 --- a/components/proxy/src/main/java/com/hotels/styx/routing/handlers/BackendServiceProxy.java +++ b/components/proxy/src/main/java/com/hotels/styx/routing/handlers/BackendServiceProxy.java @@ -23,6 +23,7 @@ import com.hotels.styx.api.LiveHttpResponse; import com.hotels.styx.api.extension.service.BackendService; import com.hotels.styx.api.extension.service.spi.Registry; +import com.hotels.styx.client.netty.eventloop.PlatformAwareClientEventLoopGroupFactory; import com.hotels.styx.infrastructure.configuration.yaml.JsonNodeConfig; import com.hotels.styx.proxy.BackendServiceClientFactory; import com.hotels.styx.proxy.BackendServicesRouter; @@ -47,8 +48,11 @@ public class BackendServiceProxy implements RoutingObject { private final RouteHandlerAdapter handler; - private BackendServiceProxy(BackendServiceClientFactory serviceClientFactory, Registry registry, Environment environment) { - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment); + private BackendServiceProxy( + BackendServiceClientFactory serviceClientFactory, + Registry registry, Environment environment, + PlatformAwareClientEventLoopGroupFactory eventLoopGroupFactory) { + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, eventLoopGroupFactory); registry.addListener(router); handler = new RouteHandlerAdapter(router); } @@ -96,7 +100,7 @@ public RoutingObject build(List parents, Context context, RoutingObjectD join(".", append(parents, "backendProvider")), provider)); } - return new BackendServiceProxy(serviceClientFactory, registry, environment); + return new BackendServiceProxy(serviceClientFactory, registry, environment, new PlatformAwareClientEventLoopGroupFactory("BackendServiceProxy", 0)); } } diff --git a/components/proxy/src/main/java/com/hotels/styx/routing/handlers/ProxyToBackend.java b/components/proxy/src/main/java/com/hotels/styx/routing/handlers/ProxyToBackend.java index 2b674a4351..1e136523e5 100644 --- a/components/proxy/src/main/java/com/hotels/styx/routing/handlers/ProxyToBackend.java +++ b/components/proxy/src/main/java/com/hotels/styx/routing/handlers/ProxyToBackend.java @@ -32,6 +32,7 @@ import com.hotels.styx.client.connectionpool.ExpiringConnectionFactory; import com.hotels.styx.client.connectionpool.SimpleConnectionPoolFactory; import com.hotels.styx.client.netty.connectionpool.NettyConnectionFactory; +import com.hotels.styx.client.netty.eventloop.PlatformAwareClientEventLoopGroupFactory; import com.hotels.styx.config.schema.Schema; import com.hotels.styx.infrastructure.configuration.yaml.JsonNodeConfig; import com.hotels.styx.proxy.BackendServiceClientFactory; @@ -95,7 +96,7 @@ static RoutingObject build(List parents, Context context, RoutingObjectD OriginStatsFactory originStatsFactory = new CachingOriginStatsFactory(context.environment().metricRegistry()); Connection.Factory connectionFactory = new NettyConnectionFactory.Builder() - .name("Styx") + .eventLoopGroupFactory(new PlatformAwareClientEventLoopGroupFactory("Styx", clientWorkerThreadsCount)) .httpRequestOperationFactory( httpRequestOperationFactoryBuilder() .flowControlEnabled(true) @@ -104,7 +105,6 @@ static RoutingObject build(List parents, Context context, RoutingObjectD .responseTimeoutMillis(backendService.responseTimeoutMillis()) .longFormat(longFormat) .build()) - .clientWorkerThreadsCount(clientWorkerThreadsCount) .tlsSettings(backendService.tlsSettings().orElse(null)) .build(); diff --git a/components/proxy/src/main/java/com/hotels/styx/startup/StyxServerComponents.java b/components/proxy/src/main/java/com/hotels/styx/startup/StyxServerComponents.java index a69c9d8a1c..cde2257b16 100644 --- a/components/proxy/src/main/java/com/hotels/styx/startup/StyxServerComponents.java +++ b/components/proxy/src/main/java/com/hotels/styx/startup/StyxServerComponents.java @@ -28,6 +28,9 @@ import com.hotels.styx.api.extension.service.spi.StyxService; import com.hotels.styx.api.metrics.codahale.CodaHaleMetricRegistry; import com.hotels.styx.api.plugins.spi.Plugin; +import com.hotels.styx.client.netty.ClientEventLoopFactory; +import com.hotels.styx.client.netty.eventloop.PlatformAwareClientEventLoopGroupFactory; +import com.hotels.styx.common.Pair; import com.hotels.styx.infrastructure.configuration.yaml.JsonNodeConfig; import com.hotels.styx.proxy.plugin.NamedPlugin; import com.hotels.styx.routing.RoutingObject; @@ -38,12 +41,16 @@ import com.hotels.styx.routing.db.StyxObjectStore; import com.hotels.styx.routing.handlers.RouteRefLookup.RouteDbRefLookup; import com.hotels.styx.startup.extensions.ConfiguredPluginFactory; +import io.netty.channel.EventLoopGroup; +import io.netty.channel.socket.SocketChannel; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.atomic.AtomicReference; import static com.hotels.styx.Version.readVersionFrom; +import static com.hotels.styx.common.Pair.pair; import static com.hotels.styx.infrastructure.logging.LOGBackConfigurer.initLogging; import static com.hotels.styx.routing.config.RoutingObjectFactory.BUILTIN_HANDLER_FACTORIES; import static com.hotels.styx.startup.ServicesLoader.SERVICES_FROM_CONFIG; @@ -62,6 +69,7 @@ public class StyxServerComponents { private final List plugins; private final StyxObjectStore routeObjectStore = new StyxObjectStore<>(); private final RoutingObjectFactory routingObjectFactory; + private final ClientEventLoopFactory eventLoopGroupFactory; private StyxServerComponents(Builder builder) { StyxConfig styxConfig = requireNonNull(builder.styxConfig); @@ -69,6 +77,10 @@ private StyxServerComponents(Builder builder) { this.environment = newEnvironment(styxConfig, builder.metricRegistry); builder.loggingSetUp.setUp(environment); + this.eventLoopGroupFactory = new MemoizedEventLoopFactory(new PlatformAwareClientEventLoopGroupFactory( + "Styx", + environment.configuration().proxyServerConfig().clientWorkerThreadsCount())); + // TODO In further refactoring, we will probably want this loading to happen outside of this constructor call, // so that it doesn't delay the admin server from starting up this.plugins = builder.configuredPluginFactories == null @@ -135,6 +147,10 @@ public RoutingObjectFactory routingObjectFactory() { return this.routingObjectFactory; } + public ClientEventLoopFactory eventLoopGroupFactory() { + return this.eventLoopGroupFactory; + } + private static Environment newEnvironment(StyxConfig styxConfig, MetricRegistry metricRegistry) { return new Environment.Builder() .configuration(styxConfig) @@ -250,4 +266,36 @@ static LoggingSetUp from(String logConfigLocation) { return environment -> setUpLogging(logConfigLocation); } } + + static class MemoizedEventLoopFactory implements ClientEventLoopFactory { + private final AtomicReference>> value = new AtomicReference<>(); + private final ClientEventLoopFactory delegate; + + private MemoizedEventLoopFactory(ClientEventLoopFactory delegate) { + this.delegate = requireNonNull(delegate); + } + + @Override + public EventLoopGroup newClientWorkerEventLoopGroup() { + return getOrCreate().key(); + } + + @Override + public Class clientSocketChannelClass() { + return getOrCreate().value(); + } + + private synchronized Pair> getOrCreate() { + if (value.get() == null) { + + EventLoopGroup eventLoop = delegate.newClientWorkerEventLoopGroup(); + Class socketChannelClass = delegate.clientSocketChannelClass(); + + value.set(pair(eventLoop, socketChannelClass)); + } + + return value.get(); + } + } + } diff --git a/components/proxy/src/test/java/com/hotels/styx/proxy/BackendServicesRouterTest.java b/components/proxy/src/test/java/com/hotels/styx/proxy/BackendServicesRouterTest.java index 2c2420ef3d..5995142979 100644 --- a/components/proxy/src/test/java/com/hotels/styx/proxy/BackendServicesRouterTest.java +++ b/components/proxy/src/test/java/com/hotels/styx/proxy/BackendServicesRouterTest.java @@ -26,6 +26,7 @@ import com.hotels.styx.client.BackendServiceClient; import com.hotels.styx.client.OriginStatsFactory; import com.hotels.styx.client.OriginsInventory; +import com.hotels.styx.client.netty.eventloop.PlatformAwareClientEventLoopGroupFactory; import com.hotels.styx.server.HttpInterceptorContext; import org.mockito.ArgumentCaptor; import org.testng.annotations.BeforeMethod; @@ -63,6 +64,8 @@ public class BackendServicesRouterTest { private Environment environment; + private PlatformAwareClientEventLoopGroupFactory factory = mock(PlatformAwareClientEventLoopGroupFactory.class); + @BeforeMethod public void before() { environment = new Environment.Builder().build(); @@ -75,7 +78,7 @@ public void registersAllRoutes() { appB().newCopy().path("/badheaders").build(), appB().newCopy().id("appB-03").path("/cookies").build()); - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); router.onChange(changes); assertThat(router.routes().keySet(), contains("/badheaders", "/cookies", "/headers")); @@ -87,7 +90,7 @@ public void selectsServiceBasedOnPath() throws Exception { appA().newCopy().path("/").build(), appB().newCopy().path("/appB/hotel/details.html").build()); - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); router.onChange(changes); LiveHttpRequest request = get("/appB/hotel/details.html").build(); @@ -102,7 +105,7 @@ public void selectsApplicationBasedOnPathIfAppsAreProvidedInOppositeOrder() thro appB().newCopy().path("/appB/hotel/details.html").build(), appA().newCopy().path("/").build()); - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); router.onChange(changes); LiveHttpRequest request = get("/appB/hotel/details.html").build(); @@ -118,7 +121,7 @@ public void selectsUsingSingleSlashPath() throws Exception { appA().newCopy().path("/").build(), appB().newCopy().path("/appB/hotel/details.html").build()); - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); router.onChange(changes); LiveHttpRequest request = get("/").build(); @@ -133,7 +136,7 @@ public void selectsUsingSingleSlashPathIfAppsAreProvidedInOppositeOrder() throws appB().newCopy().path("/appB/hotel/details.html").build(), appA().newCopy().path("/").build()); - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); router.onChange(changes); LiveHttpRequest request = get("/").build(); @@ -148,7 +151,7 @@ public void selectsUsingPathWithNoSubsequentCharacters() throws Exception { appA().newCopy().path("/").build(), appB().newCopy().path("/appB/").build()); - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); router.onChange(changes); LiveHttpRequest request = get("/appB/").build(); @@ -159,7 +162,7 @@ public void selectsUsingPathWithNoSubsequentCharacters() throws Exception { @Test public void doesNotMatchRequestIfFinalSlashIsMissing() { - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); router.onChange(added(appB().newCopy().path("/appB/hotel/details.html").build())); LiveHttpRequest request = get("/ba/").build(); @@ -171,7 +174,7 @@ public void doesNotMatchRequestIfFinalSlashIsMissing() { @Test public void throwsExceptionWhenNoApplicationMatches() { - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); router.onChange(added(appB().newCopy().path("/appB/hotel/details.html").build())); LiveHttpRequest request = get("/qwertyuiop").build(); @@ -180,7 +183,7 @@ public void throwsExceptionWhenNoApplicationMatches() { @Test public void removesExistingServicesBeforeAddingNewOnes() throws Exception { - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); router.onChange(added(appB())); router.onChange(new Registry.Changes.Builder() @@ -195,7 +198,7 @@ public void removesExistingServicesBeforeAddingNewOnes() throws Exception { @Test public void updatesRoutesOnBackendServicesChange() throws Exception { - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); LiveHttpRequest request = get("/appB/").build(); @@ -224,7 +227,7 @@ public void closesClientWhenBackendServicesAreUpdated() { .thenReturn(firstClient) .thenReturn(secondClient); - BackendServicesRouter router = new BackendServicesRouter(clientFactory, environment); + BackendServicesRouter router = new BackendServicesRouter(clientFactory, environment, factory); BackendService bookingApp = appB(); router.onChange(added(bookingApp)); @@ -252,7 +255,7 @@ public void closesClientWhenBackendServicesAreRemoved() { .thenReturn(firstClient) .thenReturn(secondClient); - BackendServicesRouter router = new BackendServicesRouter(clientFactory, environment); + BackendServicesRouter router = new BackendServicesRouter(clientFactory, environment, factory); BackendService bookingApp = appB(); router.onChange(added(bookingApp)); @@ -273,7 +276,7 @@ public void deregistersAndReregistersMetricsAppropriately() { .metricRegistry(metrics) .build(); BackendServicesRouter router = new BackendServicesRouter( - new StyxBackendServiceClientFactory(environment), environment); + new StyxBackendServiceClientFactory(environment), environment, factory); router.onChange(added(backendService(APP_B, "/appB/", 9094, "appB-01", 9095, "appB-02"))); diff --git a/components/proxy/src/test/java/com/hotels/styx/routing/StaticPipelineBuilderTest.java b/components/proxy/src/test/java/com/hotels/styx/routing/StaticPipelineBuilderTest.java index 9789ff50cf..d27fce3de6 100644 --- a/components/proxy/src/test/java/com/hotels/styx/routing/StaticPipelineBuilderTest.java +++ b/components/proxy/src/test/java/com/hotels/styx/routing/StaticPipelineBuilderTest.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -23,6 +23,7 @@ import com.hotels.styx.api.extension.service.spi.AbstractRegistry; import com.hotels.styx.api.extension.service.spi.Registry; import com.hotels.styx.api.plugins.spi.Plugin; +import com.hotels.styx.client.netty.eventloop.PlatformAwareClientEventLoopGroupFactory; import com.hotels.styx.proxy.BackendServiceClientFactory; import com.hotels.styx.proxy.plugin.NamedPlugin; import com.hotels.styx.server.HttpInterceptorContext; @@ -49,6 +50,7 @@ public class StaticPipelineBuilderTest { private Environment environment; private BackendServiceClientFactory clientFactory; private Registry registry; + private PlatformAwareClientEventLoopGroupFactory factory = new PlatformAwareClientEventLoopGroupFactory("Styx", 0); @BeforeMethod @@ -61,8 +63,7 @@ public void staticPipelineBuilderTest() { @Test public void buildsInterceptorPipelineForBackendServices() throws Exception { - - HttpHandler handler = new StaticPipelineFactory(clientFactory, environment, registry, ImmutableList.of(), false).build(); + HttpHandler handler = new StaticPipelineFactory(clientFactory, environment, registry, ImmutableList.of(), factory,false).build(); LiveHttpResponse response = Mono.from(handler.handle(get("/foo").build(), HttpInterceptorContext.create())).block(); assertThat(response.status(), is(OK)); } @@ -74,7 +75,7 @@ public void appliesPluginsInOrderTheyAreConfigured() throws Exception { interceptor("Test-B", appendResponseHeader("X-From-Plugin", "B")) ); - HttpHandler handler = new StaticPipelineFactory(clientFactory, environment, registry, plugins, false).build(); + HttpHandler handler = new StaticPipelineFactory(clientFactory, environment, registry, plugins, factory,false).build(); LiveHttpResponse response = Mono.from(handler.handle(get("/foo").build(), HttpInterceptorContext.create())).block(); assertThat(response.status(), is(OK)); diff --git a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/admin/FileBasedOriginsFileChangeMonitorSpec.scala b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/admin/FileBasedOriginsFileChangeMonitorSpec.scala deleted file mode 100644 index db64e2bfe8..0000000000 --- a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/admin/FileBasedOriginsFileChangeMonitorSpec.scala +++ /dev/null @@ -1,135 +0,0 @@ -/* - Copyright (C) 2013-2018 Expedia Inc. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ -/** - * Copyright (C) 2013-2018 Expedia Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package com.hotels.styx.admin - -import java.io.{ByteArrayInputStream, File} -import java.nio.charset.StandardCharsets.UTF_8 -import java.nio.file.Files.{copy, delete} -import java.nio.file.Path -import java.nio.file.StandardCopyOption.REPLACE_EXISTING - -import com.google.common.io.Files.createTempDir -import com.hotels.styx.api.HttpRequest.get -import com.hotels.styx.api.HttpResponseStatus -import com.hotels.styx.support.backends.FakeHttpServer -import com.hotels.styx.support.configuration._ -import com.hotels.styx.{StyxClientSupplier, StyxServerSupport} -import org.scalatest.concurrent.Eventually -import org.scalatest.{BeforeAndAfterAll, ConfigMap, FunSpec, Matchers} - -class FileBasedOriginsFileChangeMonitorSpec extends FunSpec - with StyxServerSupport - with StyxClientSupplier - with Matchers - with BeforeAndAfterAll - with Eventually { - - val tempDir = createTempDir() - val styxOriginsFile = new File(tempDir, "origins.yml").toPath - - val origin = FakeHttpServer.HttpStartupConfig(appId = "app", originId="app").start() - - val configTemplate = - """--- - |- id: "%s" - | path: "%s" - | connectionPool: - | maxConnectionsPerHost: 45 - | maxPendingConnectionsPerHost: 15 - | socketTimeoutMillis: 120000 - | connectTimeoutMillis: 1000 - | pendingConnectionTimeoutMillis: 8000 - | healthCheck: - | uri: "/version.txt" - | intervalMillis: 5000 - | responseTimeoutMillis: 60000 - | origins: - | - { id: "app1", host: "localhost:%d" } - |""".stripMargin - - private val yamlConfig: String = - s"""--- - |proxy: - | connectors: - | http: - | port: 0 - | - |admin: - | connectors: - | http: - | port: 0 - | - |services: - | factories: - | backendServiceRegistry: - | class: "com.hotels.styx.proxy.backends.file.FileBackedBackendServicesRegistry$$Factory" - | config: - | originsFile: "${styxOriginsFile.toString.replace("\\","/")}" - | monitor: - | enabled: true - """.stripMargin - - writeConfig(styxOriginsFile, configTemplate.format("appv1", "/app01/", origin.port())) - - val styxServer = StyxYamlConfig(yamlConfig = yamlConfig).startServer() - val reqToApp01 = get(s"http://localhost:${styxServer.proxyHttpAddress().getPort}/app01/x").build() - val reqToApp02 = get(s"http://localhost:${styxServer.proxyHttpAddress().getPort}/app02/x").build() - - - override protected def beforeAll() { - styxServer.isRunning should be(true) - } - - it("Automatically detects changes in origins file.") { - decodedRequest(reqToApp01).status() should be (HttpResponseStatus.OK) - decodedRequest(reqToApp02).status() should be (HttpResponseStatus.BAD_GATEWAY) - - writeConfig(styxOriginsFile, configTemplate.format("appv2", "/app02/", origin.port())) - - Thread.sleep(2000) - - decodedRequest(reqToApp01).status() should be (HttpResponseStatus.BAD_GATEWAY) - decodedRequest(reqToApp02).status() should be (HttpResponseStatus.OK) - } - - def writeConfig(path: Path, text: String): Unit = { - println(s"Updating origins configuration to $path") - println(text) - copy(new ByteArrayInputStream(text.getBytes(UTF_8)), path, REPLACE_EXISTING) - } - - override protected def afterAll(): Unit = { - styxServer.stopAsync().awaitTerminated() - delete(styxOriginsFile) - delete(tempDir.toPath) - origin.stop() - } -} diff --git a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/client/ExpiringConnectionSpec.scala b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/client/ExpiringConnectionSpec.scala index 37e02774f9..121def49e9 100644 --- a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/client/ExpiringConnectionSpec.scala +++ b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/client/ExpiringConnectionSpec.scala @@ -89,8 +89,13 @@ class ExpiringConnectionSpec extends FunSpec assertThat(response2.status(), is(OK)) eventually(timeout(2.seconds)) { - styxServer.metricsSnapshot.gauge(s"origins.appOne.generic-app-01.connectionspool.available-connections").get should be(1) - styxServer.metricsSnapshot.gauge(s"origins.appOne.generic-app-01.connectionspool.connections-terminated").get should be(1) + withClue("A connection should be available in pool") { + styxServer.metricsSnapshot.gauge(s"origins.appOne.generic-app-01.connectionspool.available-connections").get should be(1) + } + + withClue("A previous connection should have been terminated") { + styxServer.metricsSnapshot.gauge(s"origins.appOne.generic-app-01.connectionspool.connections-terminated").get should be(1) + } } } diff --git a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/OriginsReloadSpec.scala b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/OriginsReloadSpec.scala deleted file mode 100644 index 39ed7a5b00..0000000000 --- a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/OriginsReloadSpec.scala +++ /dev/null @@ -1,104 +0,0 @@ -/* - Copyright (C) 2013-2018 Expedia Inc. - - Licensed under the Apache License, Version 2.0 (the "License"); - you may not use this file except in compliance with the License. - You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - - Unless required by applicable law or agreed to in writing, software - distributed under the License is distributed on an "AS IS" BASIS, - WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - See the License for the specific language governing permissions and - limitations under the License. - */ -package com.hotels.styx.proxy - -import java.nio.charset.StandardCharsets.UTF_8 - -import com.hotels.styx.api.HttpRequest -import com.hotels.styx.api.HttpResponseStatus.{METHOD_NOT_ALLOWED, OK} -import com.hotels.styx.support.backends.FakeHttpServer -import com.hotels.styx.support.configuration.{HttpBackend, Origins} -import com.hotels.styx.{DefaultStyxConfiguration, StyxClientSupplier, StyxProxySpec} -import org.scalatest.FunSpec -import org.scalatest.concurrent.Eventually - -import scala.concurrent.duration._ - -class OriginsReloadSpec extends FunSpec - with StyxProxySpec - with DefaultStyxConfiguration - with StyxClientSupplier - with Eventually { - - val backend1 = FakeHttpServer.HttpStartupConfig(appId = "appOne", originId = "appOne-01").start() - val backend2 = FakeHttpServer.HttpStartupConfig(appId = "appOne", originId = "appOne-02").start() - val backend3 = FakeHttpServer.HttpStartupConfig(appId = "appOne", originId = "appOne-03").start() - - override protected def beforeAll(): Unit = { - super.beforeAll() - - styxServer.setBackends( - "/foobar" -> HttpBackend("appOne", Origins(backend1, backend2)) - ) - } - - override protected def afterAll(): Unit = { - backend1.stop() - backend2.stop() - backend3.stop() - super.afterAll() - } - - describe("Reload Origins Endpoint") { - it("should return 200 for POST /admin/tasks/origins/reload") { - val response = post(styxServer.adminURL("/admin/tasks/origins/reload")) - - assert(response.status() == OK) - } - - it("should return 405 for GET /admin/tasks/origins/reload") { - val response = get(styxServer.adminURL("/admin/tasks/origins/reload")) - - assert(response.status() == METHOD_NOT_ALLOWED) - } - - it("should reflect origin addition in origins status page") { - styxServer.setBackends( - "/foobar" -> HttpBackend("appOne", Origins(backend1, backend2, backend3)) - ) - - eventually(timeout(1.seconds)) { - val response = get(styxServer.adminURL("/admin/origins/status")) - - response.bodyAs(UTF_8) should include("localhost:" + backend1.port()) - response.bodyAs(UTF_8) should include("localhost:" + backend2.port()) - response.bodyAs(UTF_8) should include("localhost:" + backend3.port()) - } - } - - it("should reflect origin removal in origins status page") { - styxServer.setBackends( - "/foobar" -> HttpBackend("appOne", Origins(backend1)) - ) - - eventually(timeout(1.seconds)) { - val response = get(styxServer.adminURL("/admin/origins/status")) - - response.bodyAs(UTF_8) should include("localhost:" + backend1.port()) - response.bodyAs(UTF_8) should not include ("localhost:" + backend2.port()) - response.bodyAs(UTF_8) should not include ("localhost:" + backend3.port()) - } - } - } - - private def post(url: String, content: String = "") = { - decodedRequest(HttpRequest.post(url).body(content, UTF_8).build()) - } - - private def get(url: String) = { - decodedRequest(HttpRequest.get(url).build()) - } -} diff --git a/system-tests/ft-suite/pom.xml b/system-tests/ft-suite/pom.xml index a1b8af82bc..3998fc7eaf 100644 --- a/system-tests/ft-suite/pom.xml +++ b/system-tests/ft-suite/pom.xml @@ -59,6 +59,12 @@ jackson-annotations ${jackson.version} + + com.hotels.styx + styx-e2e-testsupport + 1.0-SNAPSHOT + test + diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/FileBasedOriginsFileChangeMonitorSpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/FileBasedOriginsFileChangeMonitorSpec.kt new file mode 100644 index 0000000000..be9705c9d8 --- /dev/null +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/FileBasedOriginsFileChangeMonitorSpec.kt @@ -0,0 +1,130 @@ +/* + Copyright (C) 2013-2019 Expedia Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package com.hotels.styx.admin + +import com.github.tomakehurst.wiremock.client.WireMock +import com.hotels.styx.api.HttpHeaderNames.HOST +import com.hotels.styx.api.HttpRequest +import com.hotels.styx.api.HttpResponseStatus.BAD_GATEWAY +import com.hotels.styx.api.HttpResponseStatus.OK +import com.hotels.styx.client.StyxHttpClient +import com.hotels.styx.server.HttpConnectorConfig +import com.hotels.styx.servers.MockOriginServer +import com.hotels.styx.support.StyxServerProvider +import com.hotels.styx.support.proxyHttpHostHeader +import com.hotels.styx.support.wait +import io.kotlintest.Spec +import io.kotlintest.shouldBe +import io.kotlintest.specs.StringSpec +import java.io.ByteArrayInputStream +import java.io.File +import java.nio.charset.StandardCharsets.UTF_8 +import java.nio.file.Files.copy +import java.nio.file.Path +import java.nio.file.StandardCopyOption.REPLACE_EXISTING + +class FileBasedOriginsFileChangeMonitorSpec: StringSpec() { + + init { + "Automatically detects changes in origins config file." { + val reqToApp01 = HttpRequest.get("/app01/x") + .header(HOST, styxServer().proxyHttpHostHeader()) + .build() + + val reqToApp02 = HttpRequest.get("/app02/x") + .header(HOST, styxServer().proxyHttpHostHeader()) + .build() + + client.send(reqToApp01).wait().status() shouldBe OK + client.send(reqToApp02).wait().status() shouldBe BAD_GATEWAY + + writeConfig(styxOriginsFile, configTemplate.format("appv2", "/app02/", mockServer.port())) + Thread.sleep(2000) + + client.send(reqToApp01).wait().status() shouldBe BAD_GATEWAY + client.send(reqToApp02).wait().status() shouldBe OK + } + } + + val tempDir = createTempDir() + val styxOriginsFile = File(tempDir, "origins.yml").toPath() + + private val configTemplate = """ + --- + - id: "%s" + path: "%s" + connectionPool: + maxConnectionsPerHost: 45 + maxPendingConnectionsPerHost: 15 + socketTimeoutMillis: 120000 + connectTimeoutMillis: 1000 + pendingConnectionTimeoutMillis: 8000 + healthCheck: + uri: "/version.txt" + intervalMillis: 5000 + responseTimeoutMillis: 60000 + origins: + - { id: "app1", host: "localhost:%d" } + """.trimIndent() + + val styxServer = StyxServerProvider(""" + --- + proxy: + connectors: + http: + port: 0 + + admin: + connectors: + http: + port: 0 + + services: + factories: + backendServiceRegistry: + class: "com.hotels.styx.proxy.backends.file.FileBackedBackendServicesRegistry${'$'}Factory" + config: + originsFile: "${styxOriginsFile.toString().replace("\\","/")}" + monitor: + enabled: true + """.trimIndent()) + + val mockServer = MockOriginServer.create("", "", 0, HttpConnectorConfig(0)) + .start() + .stub(WireMock.get(WireMock.urlMatching("/.*")), WireMock.aResponse() + .withStatus(200) + .withBody("mock-server-01")) + + override fun beforeSpec(spec: Spec) { + writeConfig(styxOriginsFile, configTemplate.format("appv1", "/app01/", mockServer.port())) + styxServer.restart() + } + + override fun afterSpec(spec: Spec) { + styxServer.stop() + mockServer.stop() + client.shutdown() + } +} + +val client: StyxHttpClient = StyxHttpClient.Builder().build() + +fun writeConfig(path: Path, text: String): Unit { + println("Updating origins configuration to $path") + println(text) + text.toByteArray(UTF_8) + copy(ByteArrayInputStream(text.toByteArray(UTF_8)), path, REPLACE_EXISTING) +} diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt new file mode 100644 index 0000000000..78b888eb15 --- /dev/null +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt @@ -0,0 +1,160 @@ +/* + Copyright (C) 2013-2019 Expedia Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package com.hotels.styx.resiliency + +import com.github.tomakehurst.wiremock.client.WireMock +import com.hotels.styx.api.HttpHeaderNames.HOST +import com.hotels.styx.api.HttpRequest.get +import com.hotels.styx.api.HttpResponseStatus.OK +import com.hotels.styx.client.StyxHttpClient +import com.hotels.styx.server.HttpConnectorConfig +import com.hotels.styx.servers.MockOriginServer +import com.hotels.styx.support.StyxServerProvider +import com.hotels.styx.support.proxyHttpHostHeader +import com.hotels.styx.support.wait +import io.kotlintest.Spec +import io.kotlintest.shouldBe +import io.kotlintest.specs.StringSpec +import java.io.ByteArrayInputStream +import java.io.File +import java.nio.charset.StandardCharsets.UTF_8 +import java.nio.file.Files.copy +import java.nio.file.Path +import java.nio.file.StandardCopyOption.REPLACE_EXISTING + +class OriginResourcesSpec : StringSpec() { + + val mockServer = MockOriginServer.create("", "", 0, HttpConnectorConfig(0)) + .start() + .stub(WireMock.get(WireMock.urlMatching("/.*")), WireMock.aResponse() + .withStatus(200) + .withBody("mock-server-01")) + + + init { + "Cleans up threads after use" { + "---" + (1..100) + .map { appDeclaration("aaa-$it") } + .joinToString("\n") + .let { writeConfig(styxOriginsFile, it) } + + // Wait for styx to pick up the configuration + Thread.sleep(2000) + + (1..100).forEach { + client.send(get("/aaa-$it").header(HOST, styxServer().proxyHttpHostHeader()).build()) + .wait(debug = false) + .status() shouldBe OK + } + + val threadCountBefore = threadCount("Styx-Client-Worker") + + "---" + (1..100) + .map { appDeclaration("bbb-$it") } + .joinToString("\n") + .let { writeConfig(styxOriginsFile, it) } + + // Wait for styx to pick up the new configuration + Thread.sleep(2000) + + (1..100).forEach { + client.send(get("/bbb-$it").header(HOST, styxServer().proxyHttpHostHeader()).build()) + .wait(debug = false) + .status() shouldBe OK + } + + val threadCountAfter = threadCount("Styx-Client-Worker") + + threadCountBefore shouldBe threadCountAfter + } + } + + val tempDir = createTempDir() + val styxOriginsFile = File(tempDir, "origins.yml").toPath() + + private val configTemplate = """ + --- + - id: "%s" + path: "%s" + connectionPool: + maxConnectionsPerHost: 45 + maxPendingConnectionsPerHost: 15 + socketTimeoutMillis: 120000 + connectTimeoutMillis: 1000 + pendingConnectionTimeoutMillis: 8000 + healthCheck: + uri: "/version.txt" + intervalMillis: 5000 + responseTimeoutMillis: 60000 + origins: + - { id: "app1", host: "localhost:%d" } + """.trimIndent() + + val styxServer = StyxServerProvider(""" + --- + proxy: + connectors: + http: + port: 0 + + admin: + connectors: + http: + port: 0 + + services: + factories: + backendServiceRegistry: + class: "com.hotels.styx.proxy.backends.file.FileBackedBackendServicesRegistry${'$'}Factory" + config: + originsFile: "${styxOriginsFile.toString().replace("\\", "/")}" + monitor: + enabled: true + """.trimIndent()) + + + override fun beforeSpec(spec: Spec) { + writeConfig(styxOriginsFile, configTemplate.format("appv1", "/app01/", mockServer.port())) + styxServer.restart() + } + + override fun afterSpec(spec: Spec) { + styxServer.stop() + mockServer.stop() + client.shutdown() + } + + fun appDeclaration(prefix: String) = """ + - id: "$prefix" + path: "/$prefix" + origins: + - { id: "$prefix", host: "localhost:${mockServer.port()}" } + """.trimIndent() + + fun threadCount(namePattern: String) = Thread.getAllStackTraces().keys + .map { it.name } + .filter { it.contains(namePattern) } + .count() + +} + +val client: StyxHttpClient = StyxHttpClient.Builder().build() + +fun writeConfig(path: Path, text: String): Unit { + text.toByteArray(UTF_8) + copy(ByteArrayInputStream(text.toByteArray(UTF_8)), path, REPLACE_EXISTING) +} + diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/support/StyxServerProvider.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/support/StyxServerProvider.kt new file mode 100644 index 0000000000..316e177d73 --- /dev/null +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/support/StyxServerProvider.kt @@ -0,0 +1,139 @@ +/* + Copyright (C) 2013-2019 Expedia Inc. + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ +package com.hotels.styx.support + +import com.fasterxml.jackson.core.JsonFactory +import com.fasterxml.jackson.core.type.TypeReference +import com.fasterxml.jackson.databind.ObjectMapper +import com.hotels.styx.StyxConfig +import com.hotels.styx.StyxServer +import com.hotels.styx.api.HttpHeaderNames +import com.hotels.styx.api.HttpRequest +import com.hotels.styx.api.HttpResponse +import com.hotels.styx.api.HttpResponseStatus +import com.hotels.styx.client.StyxHttpClient +import com.hotels.styx.startup.StyxServerComponents +import reactor.core.publisher.toMono +import java.nio.charset.StandardCharsets +import java.util.HashMap +import java.util.concurrent.CompletableFuture +import java.util.concurrent.atomic.AtomicReference + +class StyxServerProvider(val initialConfig: String) { + val serverRef: AtomicReference = AtomicReference() + + operator fun invoke() = get() + + fun get(): StyxServer { + if (!started()) { + restart() + } + + return serverRef.get()!! + } + + fun started() = (serverRef.get() == null) || serverRef.get()!!.isRunning + + fun restart(): StyxServerProvider { + if (started()) { + stop() + } + + val newServer = StyxServer(StyxServerComponents.Builder() + .styxConfig(StyxConfig.fromYaml(initialConfig)) + .build()) + newServer.startAsync()?.awaitRunning() + + serverRef.set(newServer) + return this + } + + fun stop(): Unit { + val oldServer = serverRef.get() + if (oldServer?.isRunning ?: false) { + oldServer!!.stopAsync().awaitTerminated() + } + } +} + +fun CompletableFuture.wait(debug: Boolean = true) = this.toMono() + .doOnNext { + if (debug) { + println("${it.status()} - ${it.headers()} - ${it.bodyAs(StandardCharsets.UTF_8)}") + } + } + .block() + +fun StyxServer.adminHostHeader() = "${this.adminHttpAddress().hostName}:${this.adminHttpAddress().port}" +fun StyxServer.proxyHttpHostHeader() = "localhost:${this.proxyHttpAddress().port}" +fun StyxServer.proxyHttpsHostHeader() = "localhost:${this.proxyHttpsAddress().port}" + +//@Throws(IOException::class) +private fun decodeToMap(body: String): Map { + val factory = JsonFactory() + val mapper = ObjectMapper(factory) + val typeRef = object : TypeReference>() { + + } + return mapper.readValue(body, typeRef) +} + +fun flattenMetricsMap(metricsText: String) = decodeToMap(metricsText) + .filter { it.value is Map<*, *> } + .flatMap { (it.value as Map<*, *>).entries } + .map { it.key to it.value } + .filter { it.second != null } + .toMap() + +fun StyxServer.metrics(): Map> { + val metricsText = StyxHttpClient.Builder().build() + .send(HttpRequest.get("/admin/metrics") + .header(HttpHeaderNames.HOST, this.adminHostHeader()) + .build()) + .wait() + .bodyAs(StandardCharsets.UTF_8) + + return flattenMetricsMap(metricsText) as Map> +} + +fun StyxServer.newRoutingObject(name: String, routingObject: String): HttpResponseStatus { + val response = StyxHttpClient.Builder().build() + .send(HttpRequest.put("/admin/routing/objects/$name") + .header(HttpHeaderNames.HOST, this.adminHostHeader()) + .body(routingObject, StandardCharsets.UTF_8) + .build()) + .wait() + + if (response?.status() != HttpResponseStatus.CREATED) { + println("Object $name was not created. Response from server: ${response?.status()} - '${response?.bodyAs(StandardCharsets.UTF_8)}'") + } + + return response?.status() ?: HttpResponseStatus.statusWithCode(666) +} + +fun StyxServer.removeRoutingObject(name: String): HttpResponseStatus { + val response = StyxHttpClient.Builder().build() + .send(HttpRequest.delete("/admin/routing/objects/$name") + .header(HttpHeaderNames.HOST, this.adminHostHeader()) + .build()) + .wait() + + if (response?.status() != HttpResponseStatus.OK) { + println("Object $name was not removed. Response from server: ${response?.status()} - '${response?.bodyAs(StandardCharsets.UTF_8)}'") + } + + return response?.status() ?: HttpResponseStatus.statusWithCode(666) +} \ No newline at end of file From 371c24b0b5c93678e7f592fbe870fbf58d7ac064 Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Mon, 3 Jun 2019 09:42:43 +0100 Subject: [PATCH 2/7] Fix a test. --- .../com/hotels/styx/resiliency/OriginResourcesSpec.kt | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt index 78b888eb15..f5edcdcba6 100644 --- a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt @@ -36,6 +36,7 @@ import java.nio.file.Path import java.nio.file.StandardCopyOption.REPLACE_EXISTING class OriginResourcesSpec : StringSpec() { + val count = 20 val mockServer = MockOriginServer.create("", "", 0, HttpConnectorConfig(0)) .start() @@ -46,7 +47,7 @@ class OriginResourcesSpec : StringSpec() { init { "Cleans up threads after use" { - "---" + (1..100) + "---" + (1..count) .map { appDeclaration("aaa-$it") } .joinToString("\n") .let { writeConfig(styxOriginsFile, it) } @@ -54,7 +55,7 @@ class OriginResourcesSpec : StringSpec() { // Wait for styx to pick up the configuration Thread.sleep(2000) - (1..100).forEach { + (1..count).forEach { client.send(get("/aaa-$it").header(HOST, styxServer().proxyHttpHostHeader()).build()) .wait(debug = false) .status() shouldBe OK @@ -62,7 +63,7 @@ class OriginResourcesSpec : StringSpec() { val threadCountBefore = threadCount("Styx-Client-Worker") - "---" + (1..100) + "---" + (1..count) .map { appDeclaration("bbb-$it") } .joinToString("\n") .let { writeConfig(styxOriginsFile, it) } @@ -70,7 +71,7 @@ class OriginResourcesSpec : StringSpec() { // Wait for styx to pick up the new configuration Thread.sleep(2000) - (1..100).forEach { + (1..count).forEach { client.send(get("/bbb-$it").header(HOST, styxServer().proxyHttpHostHeader()).build()) .wait(debug = false) .status() shouldBe OK From a6ad3c31931775af35d99df40f58d5ded20add8a Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Mon, 3 Jun 2019 16:23:36 +0100 Subject: [PATCH 3/7] * Remove `Connection.Factory.close()` method. * Inject Netty eventLoopGroup and socketChannelClass from StyxServer down to NettyConnection Factory. * Tidy up. --- .../com/hotels/styx/client/Connection.java | 4 -- .../hotels/styx/client/StyxHttpClient.java | 24 --------- .../ExpiringConnectionFactory.java | 6 --- .../NettyConnectionFactory.java | 29 +++------- .../styx/client/StyxHttpClientTest.java | 25 +-------- .../com/hotels/styx/StyxPipelineFactory.java | 16 ++++-- .../main/java/com/hotels/styx/StyxServer.java | 3 +- .../styx/proxy/BackendServicesRouter.java | 23 ++++---- .../styx/routing/StaticPipelineFactory.java | 19 ++++--- .../routing/handlers/BackendServiceProxy.java | 10 ++-- .../styx/routing/handlers/ProxyToBackend.java | 4 +- .../styx/startup/StyxServerComponents.java | 54 +++++-------------- .../styx/proxy/BackendServicesRouterTest.java | 40 +++++++++----- .../routing/StaticPipelineBuilderTest.java | 11 ++-- .../com/hotels/styx/metrics/StyxMetrics.java | 3 +- .../com/hotels/styx/StyxClientSupplier.scala | 4 +- .../styx/proxy/BigFileDownloadSpec.scala | 4 +- .../com/hotels/styx/proxy/ProxySpec.scala | 3 +- .../FileBasedOriginsFileChangeMonitorSpec.kt | 10 ++-- .../styx/resiliency/OriginResourcesSpec.kt | 1 - .../styx/routing/ConditionRoutingSpec.kt | 7 +-- .../styx/routing/PathPrefixRoutingSpec.kt | 7 +-- .../hotels/styx/routing/RoutingRestApiSpec.kt | 7 +-- ...tySpec2.kt => VersionFilesPropertySpec.kt} | 14 ++--- 24 files changed, 121 insertions(+), 207 deletions(-) rename system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/{VersionFilesPropertySpec2.kt => VersionFilesPropertySpec.kt} (81%) diff --git a/components/client/src/main/java/com/hotels/styx/client/Connection.java b/components/client/src/main/java/com/hotels/styx/client/Connection.java index 38face57df..c1a57d0625 100644 --- a/components/client/src/main/java/com/hotels/styx/client/Connection.java +++ b/components/client/src/main/java/com/hotels/styx/client/Connection.java @@ -42,10 +42,6 @@ interface Factory { * @return the newly created connection */ Mono createConnection(Origin origin, ConnectionSettings connectionSettings); - - default CompletableFuture close() { - return CompletableFuture.completedFuture(null); - } } /** diff --git a/components/client/src/main/java/com/hotels/styx/client/StyxHttpClient.java b/components/client/src/main/java/com/hotels/styx/client/StyxHttpClient.java index 730957b196..ae93667066 100644 --- a/components/client/src/main/java/com/hotels/styx/client/StyxHttpClient.java +++ b/components/client/src/main/java/com/hotels/styx/client/StyxHttpClient.java @@ -26,7 +26,6 @@ import com.hotels.styx.api.extension.service.TlsSettings; import com.hotels.styx.client.netty.connectionpool.HttpRequestOperation; import com.hotels.styx.client.netty.connectionpool.NettyConnectionFactory; -import com.hotels.styx.client.netty.eventloop.PlatformAwareClientEventLoopGroupFactory; import com.hotels.styx.client.ssl.SslContextFactory; import io.netty.handler.ssl.SslContext; import reactor.core.publisher.Mono; @@ -58,15 +57,6 @@ private StyxHttpClient(NettyConnectionFactory connectionFactory, Builder paramet this.connectionFactory = connectionFactory; } - /** - * Shuts the styx HTTP client thread pool. - * - * @return A {@link CompletableFuture} that completes when the thread pool is terminated - */ - public CompletableFuture shutdown() { - return connectionFactory.close(); - } - /** * Indicates that a request should be sent using secure {@code https} protocol. * @@ -184,7 +174,6 @@ private static Origin originFromRequest(LiveHttpRequest request, Boolean isHttps * Builder for {@link StyxHttpClient}. */ public static class Builder { - private String threadName = "simple-netty-http-client"; private int connectTimeoutMillis = 1000; private int maxResponseSize = 1024 * 100; private int responseTimeout = 60000; @@ -197,7 +186,6 @@ public Builder() { } Builder(Builder another) { - this.threadName = another.threadName; this.connectTimeoutMillis = another.connectTimeoutMillis; this.maxResponseSize = another.maxResponseSize; this.responseTimeout = another.responseTimeout; @@ -207,17 +195,6 @@ public Builder() { this.userAgent = another.userAgent; } - /** - * Sets a thread name used for the thread pool. - * - * @param threadName thread name - * @return this {@link Builder} - */ - public Builder threadName(String threadName) { - this.threadName = threadName; - return this; - } - /** * Sets the TCP connection timeout. * @@ -340,7 +317,6 @@ Builder copy() { */ public StyxHttpClient build() { NettyConnectionFactory connectionFactory = new NettyConnectionFactory.Builder() - .eventLoopGroupFactory(new PlatformAwareClientEventLoopGroupFactory(threadName, 1)) .httpConfig(newHttpConfigBuilder().setMaxHeadersSize(maxHeaderSize).build()) .tlsSettings(tlsSettings) .httpRequestOperationFactory(request -> new HttpRequestOperation( diff --git a/components/client/src/main/java/com/hotels/styx/client/connectionpool/ExpiringConnectionFactory.java b/components/client/src/main/java/com/hotels/styx/client/connectionpool/ExpiringConnectionFactory.java index d8658950e5..2446e88355 100644 --- a/components/client/src/main/java/com/hotels/styx/client/connectionpool/ExpiringConnectionFactory.java +++ b/components/client/src/main/java/com/hotels/styx/client/connectionpool/ExpiringConnectionFactory.java @@ -21,7 +21,6 @@ import com.hotels.styx.client.ConnectionSettings; import reactor.core.publisher.Mono; -import java.util.concurrent.CompletableFuture; import java.util.function.Supplier; import static java.util.Objects.requireNonNull; @@ -48,11 +47,6 @@ public Mono createConnection(Origin origin, ConnectionSettings conne .map(this::decorate); } - @Override - public CompletableFuture close() { - return connectionFactory.close(); - } - private Connection decorate(Connection conn) { return new ExpiringConnection(conn, connectionExpirationSeconds, SYSTEM_TICKER); } diff --git a/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnectionFactory.java b/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnectionFactory.java index 75d48e60f0..5bd56e2ac0 100644 --- a/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnectionFactory.java +++ b/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnectionFactory.java @@ -23,7 +23,6 @@ import com.hotels.styx.client.ConnectionSettings; import com.hotels.styx.client.HttpConfig; import com.hotels.styx.client.HttpRequestOperationFactory; -import com.hotels.styx.client.netty.ClientEventLoopFactory; import com.hotels.styx.client.netty.eventloop.PlatformAwareClientEventLoopGroupFactory; import com.hotels.styx.client.ssl.SslContextFactory; import io.netty.bootstrap.Bootstrap; @@ -36,17 +35,14 @@ import reactor.core.publisher.Mono; import java.util.Optional; -import java.util.concurrent.CompletableFuture; import static com.hotels.styx.client.HttpConfig.defaultHttpConfig; import static com.hotels.styx.client.HttpRequestOperationFactory.Builder.httpRequestOperationFactoryBuilder; -import static com.hotels.styx.common.CompletableFutures.fromNettyFuture; import static io.netty.channel.ChannelOption.ALLOCATOR; import static io.netty.channel.ChannelOption.CONNECT_TIMEOUT_MILLIS; import static io.netty.channel.ChannelOption.SO_KEEPALIVE; import static io.netty.channel.ChannelOption.TCP_NODELAY; import static java.util.Objects.requireNonNull; -import static java.util.concurrent.CompletableFuture.completedFuture; /** * A connection factory that creates connections using netty. @@ -68,13 +64,9 @@ public class NettyConnectionFactory implements Connection.Factory { private Class clientSocketChannelClass; private NettyConnectionFactory(Builder builder) { - this.clientSocketChannelClass = builder.eventLoopGroupFactory != null - ? builder.eventLoopGroupFactory.clientSocketChannelClass() - : GLOBAL_CLIENT_EVENT_LOOP_CLASS; + this.clientSocketChannelClass = builder.channelClass; - this.eventLoopGroup = builder.eventLoopGroupFactory != null - ? builder.eventLoopGroupFactory.newClientWorkerEventLoopGroup() - : GLOBAL_CLIENT_EVENT_LOOP; + this.eventLoopGroup = builder.eventLoopGroup; this.httpConfig = requireNonNull(builder.httpConfig); this.sslContext = builder.tlsSettings == null ? null : SslContextFactory.get(builder.tlsSettings); @@ -103,15 +95,6 @@ public Mono createConnection(Origin origin, ConnectionSettings conne }); } - @Override - public CompletableFuture close() { - if (eventLoopGroup == GLOBAL_CLIENT_EVENT_LOOP) { - return completedFuture(null); - } - - return fromNettyFuture(eventLoopGroup.shutdownGracefully()); - } - private ChannelFuture openConnection(Origin origin, ConnectionSettings connectionSettings) { bootstrap(connectionSettings); return bootstrap.connect(origin.host(), origin.port()); @@ -146,10 +129,12 @@ public static final class Builder { private HttpRequestOperationFactory httpRequestOperationFactory = httpRequestOperationFactoryBuilder().build(); private HttpConfig httpConfig = defaultHttpConfig(); private TlsSettings tlsSettings; - private ClientEventLoopFactory eventLoopGroupFactory; + private EventLoopGroup eventLoopGroup = GLOBAL_CLIENT_EVENT_LOOP; + private Class channelClass = GLOBAL_CLIENT_EVENT_LOOP_CLASS; - public Builder eventLoopGroupFactory(ClientEventLoopFactory eventLoopGroupFactory) { - this.eventLoopGroupFactory = eventLoopGroupFactory; + public Builder nettyEventLoop(EventLoopGroup eventLoopGroup, Class channelClass) { + this.eventLoopGroup = eventLoopGroup; + this.channelClass = channelClass; return this; } diff --git a/components/client/src/test/unit/java/com/hotels/styx/client/StyxHttpClientTest.java b/components/client/src/test/unit/java/com/hotels/styx/client/StyxHttpClientTest.java index 135c406e04..5a1b9cbd48 100644 --- a/components/client/src/test/unit/java/com/hotels/styx/client/StyxHttpClientTest.java +++ b/components/client/src/test/unit/java/com/hotels/styx/client/StyxHttpClientTest.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -45,7 +45,6 @@ import static com.hotels.styx.common.StyxFutures.await; import static com.hotels.styx.support.server.UrlMatchingStrategies.urlStartingWith; import static java.lang.String.format; -import static java.lang.Thread.getAllStackTraces; import static java.util.concurrent.TimeUnit.SECONDS; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; @@ -80,28 +79,6 @@ public void tearDown() { server.stop(); } - @Test - public void closesThreadPoolAfterUse() throws InterruptedException, ExecutionException { - StyxHttpClient client = new StyxHttpClient.Builder() - .threadName("test-client") - .build(); - - // Ensures that a thread is created before the assertions - client.send(httpRequest).get(); - - assertThat(threadExists("test-client"), is(true)); - - client.shutdown().get(); - - assertThat(threadExists("test-client"), is(false)); - } - - private static Boolean threadExists(String threadName) { - return getAllStackTraces().keySet().stream() - .anyMatch(thread -> - thread.getName().startsWith(threadName)); - } - /* * StyxHttpClient.Builder * - Cannot retrospectively modify user agent string diff --git a/components/proxy/src/main/java/com/hotels/styx/StyxPipelineFactory.java b/components/proxy/src/main/java/com/hotels/styx/StyxPipelineFactory.java index 2bc99aaf44..f5f2805507 100644 --- a/components/proxy/src/main/java/com/hotels/styx/StyxPipelineFactory.java +++ b/components/proxy/src/main/java/com/hotels/styx/StyxPipelineFactory.java @@ -21,7 +21,6 @@ import com.hotels.styx.api.extension.service.BackendService; import com.hotels.styx.api.extension.service.spi.Registry; import com.hotels.styx.api.extension.service.spi.StyxService; -import com.hotels.styx.client.netty.ClientEventLoopFactory; import com.hotels.styx.proxy.plugin.NamedPlugin; import com.hotels.styx.routing.HttpPipelineFactory; import com.hotels.styx.routing.RoutingObject; @@ -33,6 +32,8 @@ import com.hotels.styx.routing.handlers.HttpInterceptorPipeline; import com.hotels.styx.startup.PipelineFactory; import com.hotels.styx.startup.StyxServerComponents; +import io.netty.channel.EventLoopGroup; +import io.netty.channel.socket.SocketChannel; import java.util.List; import java.util.Map; @@ -52,20 +53,25 @@ public final class StyxPipelineFactory implements PipelineFactory { private final Environment environment; private final Map services; private final List plugins; - private final ClientEventLoopFactory eventLoopGroupFactory; + private final EventLoopGroup eventLoopGroup; + private final Class nettySocketChannelClass; + public StyxPipelineFactory( StyxObjectStore routeDb, RoutingObjectFactory routingObjectFactory, Environment environment, Map services, - List plugins, ClientEventLoopFactory eventLoopGroupFactory) { + List plugins, + EventLoopGroup eventLoopGroup, + Class nettySocketChannelClass) { this.routeDb = requireNonNull(routeDb); this.routingObjectFactory = requireNonNull(routingObjectFactory); this.environment = requireNonNull(environment); this.services = requireNonNull(services); this.plugins = requireNonNull(plugins); - this.eventLoopGroupFactory = requireNonNull(eventLoopGroupFactory); + this.eventLoopGroup = requireNonNull(eventLoopGroup); + this.nettySocketChannelClass = requireNonNull(nettySocketChannelClass); } @Override @@ -90,7 +96,7 @@ private RoutingObject configuredPipeline(RoutingObjectFactory routingObjectFacto }) .orElseGet(() -> { Registry backendServicesRegistry = (Registry) services.get("backendServiceRegistry"); - return new StaticPipelineFactory(environment, backendServicesRegistry, plugins, eventLoopGroupFactory, requestTracking); + return new StaticPipelineFactory(environment, backendServicesRegistry, plugins, eventLoopGroup, nettySocketChannelClass, requestTracking); }); return pipelineBuilder.build(); diff --git a/components/proxy/src/main/java/com/hotels/styx/StyxServer.java b/components/proxy/src/main/java/com/hotels/styx/StyxServer.java index 886a04f21a..3a66c3283e 100644 --- a/components/proxy/src/main/java/com/hotels/styx/StyxServer.java +++ b/components/proxy/src/main/java/com/hotels/styx/StyxServer.java @@ -151,7 +151,8 @@ public StyxServer(StyxServerComponents components, Stopwatch stopwatch) { components.environment(), components.services(), components.plugins(), - components.eventLoopGroupFactory())); + components.eventLoopGroup(), + components.nettySocketChannelClass())); this.proxyServer = proxyServerSetUp.createProxyServer(components); this.adminServer = createAdminServer(components); diff --git a/components/proxy/src/main/java/com/hotels/styx/proxy/BackendServicesRouter.java b/components/proxy/src/main/java/com/hotels/styx/proxy/BackendServicesRouter.java index 194fb74a30..21d9cdba4d 100644 --- a/components/proxy/src/main/java/com/hotels/styx/proxy/BackendServicesRouter.java +++ b/components/proxy/src/main/java/com/hotels/styx/proxy/BackendServicesRouter.java @@ -43,9 +43,10 @@ import com.hotels.styx.client.healthcheck.OriginHealthStatusMonitor; import com.hotels.styx.client.healthcheck.OriginHealthStatusMonitorFactory; import com.hotels.styx.client.healthcheck.UrlRequestHealthCheck; -import com.hotels.styx.client.netty.ClientEventLoopFactory; import com.hotels.styx.client.netty.connectionpool.NettyConnectionFactory; import com.hotels.styx.server.HttpRouter; +import io.netty.channel.EventLoopGroup; +import io.netty.channel.socket.SocketChannel; import org.slf4j.Logger; import java.util.Map; @@ -69,13 +70,18 @@ public class BackendServicesRouter implements HttpRouter, Registry.ChangeListene private final BackendServiceClientFactory clientFactory; private final Environment environment; - private final ClientEventLoopFactory eventLoopGroupFactory; + private final EventLoopGroup nettyEventLoopGroup; + private final Class socketChannelClass; private final ConcurrentMap routes; - public BackendServicesRouter(BackendServiceClientFactory clientFactory, Environment environment, ClientEventLoopFactory eventLoopGroupFactory) { + public BackendServicesRouter(BackendServiceClientFactory clientFactory, + Environment environment, + EventLoopGroup nettyEventLoopGroup, + Class socketChannelClass) { this.clientFactory = requireNonNull(clientFactory); this.environment = requireNonNull(environment); - this.eventLoopGroupFactory = requireNonNull(eventLoopGroupFactory); + this.nettyEventLoopGroup = requireNonNull(nettyEventLoopGroup); + this.socketChannelClass = requireNonNull(socketChannelClass); this.routes = new ConcurrentSkipListMap<>( comparingInt(String::length).reversed() @@ -118,7 +124,7 @@ public void onChange(Registry.Changes changes) { ConnectionPoolSettings poolSettings = backendService.connectionPoolConfig(); Connection.Factory connectionFactory = connectionFactory( - eventLoopGroupFactory, + nettyEventLoopGroup, socketChannelClass, backendService.responseTimeoutMillis(), backendService.tlsSettings().orElse(null), requestLoggingEnabled, @@ -153,7 +159,6 @@ public void onChange(Registry.Changes changes) { pipeline = new ProxyToClientPipeline(newClientHandler(backendService, inventory, originStatsFactory), () -> { inventory.close(); healthStatusMonitor.stop(); - healthCheckClient.shutdown(); }); routes.put(backendService.path(), pipeline); @@ -175,7 +180,6 @@ private OriginHealthStatusMonitor healthStatusMonitor(BackendService backendServ private StyxHttpClient healthCheckClient(BackendService backendService) { StyxHttpClient.Builder builder = new StyxHttpClient.Builder() .connectTimeout(backendService.connectionPoolConfig().connectTimeoutMillis(), MILLISECONDS) - .threadName("Health-Check-Monitor-" + backendService.id()) .userAgent("Styx/" + environment.buildInfo().releaseVersion()); backendService.tlsSettings().ifPresent(builder::tlsSettings); @@ -184,7 +188,8 @@ private StyxHttpClient healthCheckClient(BackendService backendService) { } private Connection.Factory connectionFactory( - ClientEventLoopFactory eventLoopGroupFactory, + EventLoopGroup nettyEventLoopGroup, + Class socketChannelClass, int responseTimeoutMillis, TlsSettings tlsSettings, boolean requestLoggingEnabled, @@ -193,7 +198,7 @@ private Connection.Factory connectionFactory( long connectionExpiration) { Connection.Factory factory = new NettyConnectionFactory.Builder() - .eventLoopGroupFactory(eventLoopGroupFactory) + .nettyEventLoop(nettyEventLoopGroup, socketChannelClass) .httpRequestOperationFactory( httpRequestOperationFactoryBuilder() .flowControlEnabled(true) diff --git a/components/proxy/src/main/java/com/hotels/styx/routing/StaticPipelineFactory.java b/components/proxy/src/main/java/com/hotels/styx/routing/StaticPipelineFactory.java index 64c22805ae..e0c479bc74 100644 --- a/components/proxy/src/main/java/com/hotels/styx/routing/StaticPipelineFactory.java +++ b/components/proxy/src/main/java/com/hotels/styx/routing/StaticPipelineFactory.java @@ -19,13 +19,14 @@ import com.hotels.styx.Environment; import com.hotels.styx.api.extension.service.BackendService; import com.hotels.styx.api.extension.service.spi.Registry; -import com.hotels.styx.client.netty.ClientEventLoopFactory; import com.hotels.styx.proxy.BackendServiceClientFactory; import com.hotels.styx.proxy.BackendServicesRouter; import com.hotels.styx.proxy.InterceptorPipelineBuilder; import com.hotels.styx.proxy.RouteHandlerAdapter; import com.hotels.styx.proxy.StyxBackendServiceClientFactory; import com.hotels.styx.proxy.plugin.NamedPlugin; +import io.netty.channel.EventLoopGroup; +import io.netty.channel.socket.SocketChannel; import static java.util.Objects.requireNonNull; @@ -38,7 +39,8 @@ public class StaticPipelineFactory implements HttpPipelineFactory { private final Environment environment; private final Registry registry; private final Iterable plugins; - private final ClientEventLoopFactory eventLoopGroupFactory; + private final EventLoopGroup eventLoopGroup; + private final Class nettySocketChannelClass; private final boolean trackRequests; @VisibleForTesting @@ -46,22 +48,25 @@ public class StaticPipelineFactory implements HttpPipelineFactory { Environment environment, Registry registry, Iterable plugins, - ClientEventLoopFactory eventLoopGroupFactory, + EventLoopGroup eventLoopGroup, + Class nettySocketChannelClass, boolean trackRequests) { this.clientFactory = requireNonNull(clientFactory); this.environment = requireNonNull(environment); this.registry = requireNonNull(registry); this.plugins = requireNonNull(plugins); + this.eventLoopGroup = requireNonNull(eventLoopGroup); + this.nettySocketChannelClass = requireNonNull(nettySocketChannelClass); this.trackRequests = trackRequests; - this.eventLoopGroupFactory = requireNonNull(eventLoopGroupFactory); } public StaticPipelineFactory(Environment environment, Registry registry, Iterable plugins, - ClientEventLoopFactory eventLoopGroupFactory, + EventLoopGroup eventLoopGroup, + Class nettySocketChannelClass, boolean trackRequests) { - this(createClientFactory(environment), environment, registry, plugins, eventLoopGroupFactory, trackRequests); + this(createClientFactory(environment), environment, registry, plugins, eventLoopGroup, nettySocketChannelClass, trackRequests); } private static BackendServiceClientFactory createClientFactory(Environment environment) { @@ -70,7 +75,7 @@ private static BackendServiceClientFactory createClientFactory(Environment envir @Override public RoutingObject build() { - BackendServicesRouter backendServicesRouter = new BackendServicesRouter(clientFactory, environment, eventLoopGroupFactory); + BackendServicesRouter backendServicesRouter = new BackendServicesRouter(clientFactory, environment, eventLoopGroup, nettySocketChannelClass); registry.addListener(backendServicesRouter); RouteHandlerAdapter router = new RouteHandlerAdapter(backendServicesRouter); diff --git a/components/proxy/src/main/java/com/hotels/styx/routing/handlers/BackendServiceProxy.java b/components/proxy/src/main/java/com/hotels/styx/routing/handlers/BackendServiceProxy.java index a94f218fe2..5257c0c857 100644 --- a/components/proxy/src/main/java/com/hotels/styx/routing/handlers/BackendServiceProxy.java +++ b/components/proxy/src/main/java/com/hotels/styx/routing/handlers/BackendServiceProxy.java @@ -32,6 +32,8 @@ import com.hotels.styx.routing.RoutingObject; import com.hotels.styx.routing.config.HttpHandlerFactory; import com.hotels.styx.routing.config.RoutingObjectDefinition; +import io.netty.channel.EventLoopGroup; +import io.netty.channel.socket.SocketChannel; import java.util.List; import java.util.Map; @@ -51,8 +53,9 @@ public class BackendServiceProxy implements RoutingObject { private BackendServiceProxy( BackendServiceClientFactory serviceClientFactory, Registry registry, Environment environment, - PlatformAwareClientEventLoopGroupFactory eventLoopGroupFactory) { - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, eventLoopGroupFactory); + EventLoopGroup eventLoopGroup, + Class nettySocketChannelClass) { + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, eventLoopGroup, nettySocketChannelClass); registry.addListener(router); handler = new RouteHandlerAdapter(router); } @@ -100,7 +103,8 @@ public RoutingObject build(List parents, Context context, RoutingObjectD join(".", append(parents, "backendProvider")), provider)); } - return new BackendServiceProxy(serviceClientFactory, registry, environment, new PlatformAwareClientEventLoopGroupFactory("BackendServiceProxy", 0)); + PlatformAwareClientEventLoopGroupFactory factory = new PlatformAwareClientEventLoopGroupFactory("BackendServiceProxy", 0); + return new BackendServiceProxy(serviceClientFactory, registry, environment, factory.newClientWorkerEventLoopGroup(), factory.clientSocketChannelClass()); } } diff --git a/components/proxy/src/main/java/com/hotels/styx/routing/handlers/ProxyToBackend.java b/components/proxy/src/main/java/com/hotels/styx/routing/handlers/ProxyToBackend.java index 1e136523e5..73bfee1be9 100644 --- a/components/proxy/src/main/java/com/hotels/styx/routing/handlers/ProxyToBackend.java +++ b/components/proxy/src/main/java/com/hotels/styx/routing/handlers/ProxyToBackend.java @@ -95,8 +95,10 @@ static RoutingObject build(List parents, Context context, RoutingObjectD OriginStatsFactory originStatsFactory = new CachingOriginStatsFactory(context.environment().metricRegistry()); + PlatformAwareClientEventLoopGroupFactory factory = new PlatformAwareClientEventLoopGroupFactory("Styx", clientWorkerThreadsCount); + Connection.Factory connectionFactory = new NettyConnectionFactory.Builder() - .eventLoopGroupFactory(new PlatformAwareClientEventLoopGroupFactory("Styx", clientWorkerThreadsCount)) + .nettyEventLoop(factory.newClientWorkerEventLoopGroup(), factory.clientSocketChannelClass()) .httpRequestOperationFactory( httpRequestOperationFactoryBuilder() .flowControlEnabled(true) diff --git a/components/proxy/src/main/java/com/hotels/styx/startup/StyxServerComponents.java b/components/proxy/src/main/java/com/hotels/styx/startup/StyxServerComponents.java index cde2257b16..5dfec78b2a 100644 --- a/components/proxy/src/main/java/com/hotels/styx/startup/StyxServerComponents.java +++ b/components/proxy/src/main/java/com/hotels/styx/startup/StyxServerComponents.java @@ -28,9 +28,7 @@ import com.hotels.styx.api.extension.service.spi.StyxService; import com.hotels.styx.api.metrics.codahale.CodaHaleMetricRegistry; import com.hotels.styx.api.plugins.spi.Plugin; -import com.hotels.styx.client.netty.ClientEventLoopFactory; import com.hotels.styx.client.netty.eventloop.PlatformAwareClientEventLoopGroupFactory; -import com.hotels.styx.common.Pair; import com.hotels.styx.infrastructure.configuration.yaml.JsonNodeConfig; import com.hotels.styx.proxy.plugin.NamedPlugin; import com.hotels.styx.routing.RoutingObject; @@ -47,10 +45,8 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.atomic.AtomicReference; import static com.hotels.styx.Version.readVersionFrom; -import static com.hotels.styx.common.Pair.pair; import static com.hotels.styx.infrastructure.logging.LOGBackConfigurer.initLogging; import static com.hotels.styx.routing.config.RoutingObjectFactory.BUILTIN_HANDLER_FACTORIES; import static com.hotels.styx.startup.ServicesLoader.SERVICES_FROM_CONFIG; @@ -69,7 +65,8 @@ public class StyxServerComponents { private final List plugins; private final StyxObjectStore routeObjectStore = new StyxObjectStore<>(); private final RoutingObjectFactory routingObjectFactory; - private final ClientEventLoopFactory eventLoopGroupFactory; + private final EventLoopGroup eventLoopGroup; + private final Class nettySocketChannelClass; private StyxServerComponents(Builder builder) { StyxConfig styxConfig = requireNonNull(builder.styxConfig); @@ -77,9 +74,12 @@ private StyxServerComponents(Builder builder) { this.environment = newEnvironment(styxConfig, builder.metricRegistry); builder.loggingSetUp.setUp(environment); - this.eventLoopGroupFactory = new MemoizedEventLoopFactory(new PlatformAwareClientEventLoopGroupFactory( + PlatformAwareClientEventLoopGroupFactory factory = new PlatformAwareClientEventLoopGroupFactory( "Styx", - environment.configuration().proxyServerConfig().clientWorkerThreadsCount())); + environment.configuration().proxyServerConfig().clientWorkerThreadsCount()); + + this.eventLoopGroup = factory.newClientWorkerEventLoopGroup(); + this.nettySocketChannelClass = factory.clientSocketChannelClass(); // TODO In further refactoring, we will probably want this loading to happen outside of this constructor call, // so that it doesn't delay the admin server from starting up @@ -147,8 +147,12 @@ public RoutingObjectFactory routingObjectFactory() { return this.routingObjectFactory; } - public ClientEventLoopFactory eventLoopGroupFactory() { - return this.eventLoopGroupFactory; + public EventLoopGroup eventLoopGroup() { + return this.eventLoopGroup; + } + + public Class nettySocketChannelClass() { + return this.nettySocketChannelClass; } private static Environment newEnvironment(StyxConfig styxConfig, MetricRegistry metricRegistry) { @@ -266,36 +270,4 @@ static LoggingSetUp from(String logConfigLocation) { return environment -> setUpLogging(logConfigLocation); } } - - static class MemoizedEventLoopFactory implements ClientEventLoopFactory { - private final AtomicReference>> value = new AtomicReference<>(); - private final ClientEventLoopFactory delegate; - - private MemoizedEventLoopFactory(ClientEventLoopFactory delegate) { - this.delegate = requireNonNull(delegate); - } - - @Override - public EventLoopGroup newClientWorkerEventLoopGroup() { - return getOrCreate().key(); - } - - @Override - public Class clientSocketChannelClass() { - return getOrCreate().value(); - } - - private synchronized Pair> getOrCreate() { - if (value.get() == null) { - - EventLoopGroup eventLoop = delegate.newClientWorkerEventLoopGroup(); - Class socketChannelClass = delegate.clientSocketChannelClass(); - - value.set(pair(eventLoop, socketChannelClass)); - } - - return value.get(); - } - } - } diff --git a/components/proxy/src/test/java/com/hotels/styx/proxy/BackendServicesRouterTest.java b/components/proxy/src/test/java/com/hotels/styx/proxy/BackendServicesRouterTest.java index 5995142979..b233169c5b 100644 --- a/components/proxy/src/test/java/com/hotels/styx/proxy/BackendServicesRouterTest.java +++ b/components/proxy/src/test/java/com/hotels/styx/proxy/BackendServicesRouterTest.java @@ -26,9 +26,14 @@ import com.hotels.styx.client.BackendServiceClient; import com.hotels.styx.client.OriginStatsFactory; import com.hotels.styx.client.OriginsInventory; +import com.hotels.styx.client.netty.ClientEventLoopFactory; import com.hotels.styx.client.netty.eventloop.PlatformAwareClientEventLoopGroupFactory; import com.hotels.styx.server.HttpInterceptorContext; +import io.kotlintest.specs.AbstractAnnotationSpec; +import io.netty.channel.EventLoopGroup; +import io.netty.channel.socket.SocketChannel; import org.mockito.ArgumentCaptor; +import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import reactor.core.publisher.Flux; @@ -64,7 +69,14 @@ public class BackendServicesRouterTest { private Environment environment; - private PlatformAwareClientEventLoopGroupFactory factory = mock(PlatformAwareClientEventLoopGroupFactory.class); + private ClientEventLoopFactory factory = new PlatformAwareClientEventLoopGroupFactory("x", 0); + private EventLoopGroup eventLoopGroup = this.factory.newClientWorkerEventLoopGroup(); + private Class channelClass = this.factory.clientSocketChannelClass(); + + @AbstractAnnotationSpec.AfterAll + public void tearDown() { + eventLoopGroup.shutdown(); + } @BeforeMethod public void before() { @@ -78,7 +90,7 @@ public void registersAllRoutes() { appB().newCopy().path("/badheaders").build(), appB().newCopy().id("appB-03").path("/cookies").build()); - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, eventLoopGroup, channelClass); router.onChange(changes); assertThat(router.routes().keySet(), contains("/badheaders", "/cookies", "/headers")); @@ -90,7 +102,7 @@ public void selectsServiceBasedOnPath() throws Exception { appA().newCopy().path("/").build(), appB().newCopy().path("/appB/hotel/details.html").build()); - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, eventLoopGroup, channelClass); router.onChange(changes); LiveHttpRequest request = get("/appB/hotel/details.html").build(); @@ -105,7 +117,7 @@ public void selectsApplicationBasedOnPathIfAppsAreProvidedInOppositeOrder() thro appB().newCopy().path("/appB/hotel/details.html").build(), appA().newCopy().path("/").build()); - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, eventLoopGroup, channelClass); router.onChange(changes); LiveHttpRequest request = get("/appB/hotel/details.html").build(); @@ -121,7 +133,7 @@ public void selectsUsingSingleSlashPath() throws Exception { appA().newCopy().path("/").build(), appB().newCopy().path("/appB/hotel/details.html").build()); - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, eventLoopGroup, channelClass); router.onChange(changes); LiveHttpRequest request = get("/").build(); @@ -136,7 +148,7 @@ public void selectsUsingSingleSlashPathIfAppsAreProvidedInOppositeOrder() throws appB().newCopy().path("/appB/hotel/details.html").build(), appA().newCopy().path("/").build()); - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, eventLoopGroup, channelClass); router.onChange(changes); LiveHttpRequest request = get("/").build(); @@ -151,7 +163,7 @@ public void selectsUsingPathWithNoSubsequentCharacters() throws Exception { appA().newCopy().path("/").build(), appB().newCopy().path("/appB/").build()); - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, eventLoopGroup, channelClass); router.onChange(changes); LiveHttpRequest request = get("/appB/").build(); @@ -162,7 +174,7 @@ public void selectsUsingPathWithNoSubsequentCharacters() throws Exception { @Test public void doesNotMatchRequestIfFinalSlashIsMissing() { - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, eventLoopGroup, channelClass); router.onChange(added(appB().newCopy().path("/appB/hotel/details.html").build())); LiveHttpRequest request = get("/ba/").build(); @@ -174,7 +186,7 @@ public void doesNotMatchRequestIfFinalSlashIsMissing() { @Test public void throwsExceptionWhenNoApplicationMatches() { - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, eventLoopGroup, channelClass); router.onChange(added(appB().newCopy().path("/appB/hotel/details.html").build())); LiveHttpRequest request = get("/qwertyuiop").build(); @@ -183,7 +195,7 @@ public void throwsExceptionWhenNoApplicationMatches() { @Test public void removesExistingServicesBeforeAddingNewOnes() throws Exception { - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, eventLoopGroup, channelClass); router.onChange(added(appB())); router.onChange(new Registry.Changes.Builder() @@ -198,7 +210,7 @@ public void removesExistingServicesBeforeAddingNewOnes() throws Exception { @Test public void updatesRoutesOnBackendServicesChange() throws Exception { - BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, factory); + BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, eventLoopGroup, channelClass); LiveHttpRequest request = get("/appB/").build(); @@ -227,7 +239,7 @@ public void closesClientWhenBackendServicesAreUpdated() { .thenReturn(firstClient) .thenReturn(secondClient); - BackendServicesRouter router = new BackendServicesRouter(clientFactory, environment, factory); + BackendServicesRouter router = new BackendServicesRouter(clientFactory, environment, eventLoopGroup, channelClass); BackendService bookingApp = appB(); router.onChange(added(bookingApp)); @@ -255,7 +267,7 @@ public void closesClientWhenBackendServicesAreRemoved() { .thenReturn(firstClient) .thenReturn(secondClient); - BackendServicesRouter router = new BackendServicesRouter(clientFactory, environment, factory); + BackendServicesRouter router = new BackendServicesRouter(clientFactory, environment, eventLoopGroup, channelClass); BackendService bookingApp = appB(); router.onChange(added(bookingApp)); @@ -276,7 +288,7 @@ public void deregistersAndReregistersMetricsAppropriately() { .metricRegistry(metrics) .build(); BackendServicesRouter router = new BackendServicesRouter( - new StyxBackendServiceClientFactory(environment), environment, factory); + new StyxBackendServiceClientFactory(environment), environment, eventLoopGroup, channelClass); router.onChange(added(backendService(APP_B, "/appB/", 9094, "appB-01", 9095, "appB-02"))); diff --git a/components/proxy/src/test/java/com/hotels/styx/routing/StaticPipelineBuilderTest.java b/components/proxy/src/test/java/com/hotels/styx/routing/StaticPipelineBuilderTest.java index d27fce3de6..dc4c7a501e 100644 --- a/components/proxy/src/test/java/com/hotels/styx/routing/StaticPipelineBuilderTest.java +++ b/components/proxy/src/test/java/com/hotels/styx/routing/StaticPipelineBuilderTest.java @@ -27,6 +27,8 @@ import com.hotels.styx.proxy.BackendServiceClientFactory; import com.hotels.styx.proxy.plugin.NamedPlugin; import com.hotels.styx.server.HttpInterceptorContext; +import io.netty.channel.EventLoopGroup; +import io.netty.channel.socket.SocketChannel; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import reactor.core.publisher.Mono; @@ -50,7 +52,10 @@ public class StaticPipelineBuilderTest { private Environment environment; private BackendServiceClientFactory clientFactory; private Registry registry; - private PlatformAwareClientEventLoopGroupFactory factory = new PlatformAwareClientEventLoopGroupFactory("Styx", 0); + + private final PlatformAwareClientEventLoopGroupFactory factory = new PlatformAwareClientEventLoopGroupFactory("Styx", 0); + private final EventLoopGroup eventLoopGroup = factory.newClientWorkerEventLoopGroup(); + private final Class socketChannelClass = factory.clientSocketChannelClass(); @BeforeMethod @@ -63,7 +68,7 @@ public void staticPipelineBuilderTest() { @Test public void buildsInterceptorPipelineForBackendServices() throws Exception { - HttpHandler handler = new StaticPipelineFactory(clientFactory, environment, registry, ImmutableList.of(), factory,false).build(); + HttpHandler handler = new StaticPipelineFactory(clientFactory, environment, registry, ImmutableList.of(), eventLoopGroup, socketChannelClass,false).build(); LiveHttpResponse response = Mono.from(handler.handle(get("/foo").build(), HttpInterceptorContext.create())).block(); assertThat(response.status(), is(OK)); } @@ -75,7 +80,7 @@ public void appliesPluginsInOrderTheyAreConfigured() throws Exception { interceptor("Test-B", appendResponseHeader("X-From-Plugin", "B")) ); - HttpHandler handler = new StaticPipelineFactory(clientFactory, environment, registry, plugins, factory,false).build(); + HttpHandler handler = new StaticPipelineFactory(clientFactory, environment, registry, plugins, eventLoopGroup, socketChannelClass,false).build(); LiveHttpResponse response = Mono.from(handler.handle(get("/foo").build(), HttpInterceptorContext.create())).block(); assertThat(response.status(), is(OK)); diff --git a/system-tests/e2e-suite/src/test/java/com/hotels/styx/metrics/StyxMetrics.java b/system-tests/e2e-suite/src/test/java/com/hotels/styx/metrics/StyxMetrics.java index b817093339..97e0bc6d63 100644 --- a/system-tests/e2e-suite/src/test/java/com/hotels/styx/metrics/StyxMetrics.java +++ b/system-tests/e2e-suite/src/test/java/com/hotels/styx/metrics/StyxMetrics.java @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -160,7 +160,6 @@ private static Object toGauge(Map map) { private static String downloadJsonString(String host, int port) { StyxHttpClient client = new StyxHttpClient.Builder().build(); HttpResponse response = await(client.sendRequest(get(format("http://%s:%d/admin/metrics", host, port)).build())); - await(client.shutdown()); return response.bodyAs(UTF_8); } diff --git a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/StyxClientSupplier.scala b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/StyxClientSupplier.scala index 5a5f014905..2f19f4c73e 100644 --- a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/StyxClientSupplier.scala +++ b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/StyxClientSupplier.scala @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -33,13 +33,11 @@ trait StyxClientSupplier extends BeforeAndAfterAll { val FIVE_SECONDS: Int = 5 * 1000 val client: StyxHttpClient = new StyxHttpClient.Builder() - .threadName("scalatest-e2e-client") .connectTimeout(1000, MILLISECONDS) .maxHeaderSize(2 * 8192) .build() override protected def afterAll() = { - client.shutdown() super.afterAll() } diff --git a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/BigFileDownloadSpec.scala b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/BigFileDownloadSpec.scala index acac98c257..469109d4e7 100644 --- a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/BigFileDownloadSpec.scala +++ b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/BigFileDownloadSpec.scala @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -13,7 +13,6 @@ See the License for the specific language governing permissions and limitations under the License. */ - package com.hotels.styx.proxy import java.io.{File, IOException, RandomAccessFile} @@ -41,7 +40,6 @@ class BigFileDownloadSpec extends FunSpec val fileServer = new MockServer(0) val myClient: StyxHttpClient = new StyxHttpClient.Builder() - .threadName("streaming-scalatest-e2e-client") .connectTimeout(1000, MILLISECONDS) .maxHeaderSize(2 * 8192) .build() diff --git a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/ProxySpec.scala b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/ProxySpec.scala index 165e81e936..639326f624 100644 --- a/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/ProxySpec.scala +++ b/system-tests/e2e-suite/src/test/scala/com/hotels/styx/proxy/ProxySpec.scala @@ -1,5 +1,5 @@ /* - Copyright (C) 2013-2018 Expedia Inc. + Copyright (C) 2013-2019 Expedia Inc. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -126,7 +126,6 @@ class ProxySpec extends FunSpec val client: HttpClient = new StyxHttpClient.Builder() .connectTimeout(1000, MILLISECONDS) .maxHeaderSize(2 * 8192) - .threadName("scalatest-e2e-client") .build() val req = head("/bodiless") diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/FileBasedOriginsFileChangeMonitorSpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/FileBasedOriginsFileChangeMonitorSpec.kt index be9705c9d8..b8a2afd322 100644 --- a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/FileBasedOriginsFileChangeMonitorSpec.kt +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/admin/FileBasedOriginsFileChangeMonitorSpec.kt @@ -27,6 +27,8 @@ import com.hotels.styx.support.StyxServerProvider import com.hotels.styx.support.proxyHttpHostHeader import com.hotels.styx.support.wait import io.kotlintest.Spec +import io.kotlintest.eventually +import io.kotlintest.seconds import io.kotlintest.shouldBe import io.kotlintest.specs.StringSpec import java.io.ByteArrayInputStream @@ -52,10 +54,11 @@ class FileBasedOriginsFileChangeMonitorSpec: StringSpec() { client.send(reqToApp02).wait().status() shouldBe BAD_GATEWAY writeConfig(styxOriginsFile, configTemplate.format("appv2", "/app02/", mockServer.port())) - Thread.sleep(2000) - client.send(reqToApp01).wait().status() shouldBe BAD_GATEWAY - client.send(reqToApp02).wait().status() shouldBe OK + eventually(3.seconds, AssertionError::class.java) { + client.send(reqToApp01).wait().status() shouldBe BAD_GATEWAY + client.send(reqToApp02).wait().status() shouldBe OK + } } } @@ -116,7 +119,6 @@ class FileBasedOriginsFileChangeMonitorSpec: StringSpec() { override fun afterSpec(spec: Spec) { styxServer.stop() mockServer.stop() - client.shutdown() } } diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt index f5edcdcba6..6dfb44cb18 100644 --- a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt @@ -135,7 +135,6 @@ class OriginResourcesSpec : StringSpec() { override fun afterSpec(spec: Spec) { styxServer.stop() mockServer.stop() - client.shutdown() } fun appDeclaration(prefix: String) = """ diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/ConditionRoutingSpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/ConditionRoutingSpec.kt index 0908573c43..6d89ce2fef 100644 --- a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/ConditionRoutingSpec.kt +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/ConditionRoutingSpec.kt @@ -28,7 +28,6 @@ import io.kotlintest.shouldBe import io.kotlintest.specs.StringSpec import reactor.core.publisher.Mono import java.nio.charset.StandardCharsets.UTF_8 -import java.util.concurrent.TimeUnit.MILLISECONDS class ConditionRoutingSpec : StringSpec() { @@ -105,11 +104,7 @@ class ConditionRoutingSpec : StringSpec() { content: "Hello, from http server!" """.trimIndent() - val client: StyxHttpClient = StyxHttpClient.Builder() - .threadName("functional-test-client") - .connectTimeout(1000, MILLISECONDS) - .maxHeaderSize(2 * 8192) - .build() + val client: StyxHttpClient = StyxHttpClient.Builder().build() val styxServer = StyxServer(StyxServerComponents.Builder() .styxConfig(StyxConfig.fromYaml(yamlText)) diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/PathPrefixRoutingSpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/PathPrefixRoutingSpec.kt index 28b5e7ed1c..ba2e5bfecf 100644 --- a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/PathPrefixRoutingSpec.kt +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/PathPrefixRoutingSpec.kt @@ -27,7 +27,6 @@ import io.kotlintest.shouldBe import io.kotlintest.specs.StringSpec import reactor.core.publisher.toMono import java.nio.charset.StandardCharsets.UTF_8 -import java.util.concurrent.TimeUnit.MILLISECONDS class PathPrefixRoutingSpec : StringSpec() { @@ -100,11 +99,7 @@ class PathPrefixRoutingSpec : StringSpec() { httpPipeline: root """.trimIndent() - val client: StyxHttpClient = StyxHttpClient.Builder() - .threadName("functional-test-client") - .connectTimeout(1000, MILLISECONDS) - .maxHeaderSize(2 * 8192) - .build() + val client: StyxHttpClient = StyxHttpClient.Builder().build() val styxServer = StyxServer(StyxServerComponents.Builder() .styxConfig(StyxConfig.fromYaml(yamlText)) diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/RoutingRestApiSpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/RoutingRestApiSpec.kt index 2e66aebf6d..7d034e02f6 100644 --- a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/RoutingRestApiSpec.kt +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/RoutingRestApiSpec.kt @@ -33,7 +33,6 @@ import io.kotlintest.shouldBe import io.kotlintest.specs.StringSpec import reactor.core.publisher.toMono import java.nio.charset.StandardCharsets.UTF_8 -import java.util.concurrent.TimeUnit.MILLISECONDS class RoutingRestApiSpec : StringSpec() { @@ -165,11 +164,7 @@ class RoutingRestApiSpec : StringSpec() { } } - val client: StyxHttpClient = StyxHttpClient.Builder() - .threadName("functional-test-client") - .connectTimeout(1000, MILLISECONDS) - .maxHeaderSize(2 * 8192) - .build() + val client: StyxHttpClient = StyxHttpClient.Builder().build() val styxServer = StyxServer(StyxServerComponents.Builder() .styxConfig(StyxConfig.fromYaml(yamlText)) diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/VersionFilesPropertySpec2.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/VersionFilesPropertySpec.kt similarity index 81% rename from system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/VersionFilesPropertySpec2.kt rename to system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/VersionFilesPropertySpec.kt index 894dfb11e3..754b766605 100644 --- a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/VersionFilesPropertySpec2.kt +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/routing/VersionFilesPropertySpec.kt @@ -26,14 +26,12 @@ import com.hotels.styx.support.ResourcePaths.fixturesHome import io.kotlintest.Spec import io.kotlintest.shouldBe import io.kotlintest.specs.StringSpec -import reactor.core.publisher.Mono import reactor.core.publisher.toMono import java.nio.charset.StandardCharsets.UTF_8 -import java.util.concurrent.TimeUnit.MILLISECONDS -class VersionFilesPropertySpec2 : StringSpec() { - val fileLocation = fixturesHome(VersionFilesPropertySpec2::class.java,"/version.txt") - val originsOk = fixturesHome(VersionFilesPropertySpec2::class.java, "/conf/origins/origins-correct.yml") +class VersionFilesPropertySpec : StringSpec() { + val fileLocation = fixturesHome(VersionFilesPropertySpec::class.java,"/version.txt") + val originsOk = fixturesHome(VersionFilesPropertySpec::class.java, "/conf/origins/origins-correct.yml") val yamlText = """ services: factories: @@ -65,11 +63,7 @@ class VersionFilesPropertySpec2 : StringSpec() { } } - val client: StyxHttpClient = StyxHttpClient.Builder() - .threadName("functional-test-client") - .connectTimeout(1000, MILLISECONDS) - .maxHeaderSize(2 * 8192) - .build() + val client: StyxHttpClient = StyxHttpClient.Builder().build() val styxServer = StyxServer(StyxServerComponents.Builder() .styxConfig(StyxConfig.fromYaml(yamlText)) From e048e356b32f44f7ca18cde642a2c8e814385d5a Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Mon, 3 Jun 2019 17:05:01 +0100 Subject: [PATCH 4/7] Tidy up as per code review comments. --- .../netty/connectionpool/NettyConnectionFactory.java | 10 ++++------ .../com/hotels/styx/proxy/BackendServicesRouter.java | 3 ++- .../hotels/styx/proxy/BackendServicesRouterTest.java | 5 ++--- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnectionFactory.java b/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnectionFactory.java index 5bd56e2ac0..ed3eee3a4a 100644 --- a/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnectionFactory.java +++ b/components/client/src/main/java/com/hotels/styx/client/netty/connectionpool/NettyConnectionFactory.java @@ -64,10 +64,8 @@ public class NettyConnectionFactory implements Connection.Factory { private Class clientSocketChannelClass; private NettyConnectionFactory(Builder builder) { - this.clientSocketChannelClass = builder.channelClass; - - this.eventLoopGroup = builder.eventLoopGroup; - + this.clientSocketChannelClass = requireNonNull(builder.channelClass); + this.eventLoopGroup = requireNonNull(builder.eventLoopGroup); this.httpConfig = requireNonNull(builder.httpConfig); this.sslContext = builder.tlsSettings == null ? null : SslContextFactory.get(builder.tlsSettings); this.httpRequestOperationFactory = requireNonNull(builder.httpRequestOperationFactory); @@ -133,8 +131,8 @@ public static final class Builder { private Class channelClass = GLOBAL_CLIENT_EVENT_LOOP_CLASS; public Builder nettyEventLoop(EventLoopGroup eventLoopGroup, Class channelClass) { - this.eventLoopGroup = eventLoopGroup; - this.channelClass = channelClass; + this.eventLoopGroup = requireNonNull(eventLoopGroup); + this.channelClass = requireNonNull(channelClass); return this; } diff --git a/components/proxy/src/main/java/com/hotels/styx/proxy/BackendServicesRouter.java b/components/proxy/src/main/java/com/hotels/styx/proxy/BackendServicesRouter.java index 21d9cdba4d..fe04c6f3ec 100644 --- a/components/proxy/src/main/java/com/hotels/styx/proxy/BackendServicesRouter.java +++ b/components/proxy/src/main/java/com/hotels/styx/proxy/BackendServicesRouter.java @@ -124,7 +124,8 @@ public void onChange(Registry.Changes changes) { ConnectionPoolSettings poolSettings = backendService.connectionPoolConfig(); Connection.Factory connectionFactory = connectionFactory( - nettyEventLoopGroup, socketChannelClass, + nettyEventLoopGroup, + socketChannelClass, backendService.responseTimeoutMillis(), backendService.tlsSettings().orElse(null), requestLoggingEnabled, diff --git a/components/proxy/src/test/java/com/hotels/styx/proxy/BackendServicesRouterTest.java b/components/proxy/src/test/java/com/hotels/styx/proxy/BackendServicesRouterTest.java index b233169c5b..ad6cbf8598 100644 --- a/components/proxy/src/test/java/com/hotels/styx/proxy/BackendServicesRouterTest.java +++ b/components/proxy/src/test/java/com/hotels/styx/proxy/BackendServicesRouterTest.java @@ -29,11 +29,10 @@ import com.hotels.styx.client.netty.ClientEventLoopFactory; import com.hotels.styx.client.netty.eventloop.PlatformAwareClientEventLoopGroupFactory; import com.hotels.styx.server.HttpInterceptorContext; -import io.kotlintest.specs.AbstractAnnotationSpec; import io.netty.channel.EventLoopGroup; import io.netty.channel.socket.SocketChannel; import org.mockito.ArgumentCaptor; -import org.testng.annotations.AfterMethod; +import org.testng.annotations.AfterClass; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; import reactor.core.publisher.Flux; @@ -73,7 +72,7 @@ public class BackendServicesRouterTest { private EventLoopGroup eventLoopGroup = this.factory.newClientWorkerEventLoopGroup(); private Class channelClass = this.factory.clientSocketChannelClass(); - @AbstractAnnotationSpec.AfterAll + @AfterClass public void tearDown() { eventLoopGroup.shutdown(); } From c68edb42d7b3f9554dfdd90750e5af4363c018ac Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Tue, 4 Jun 2019 07:46:49 +0100 Subject: [PATCH 5/7] Tidy up test. --- .../styx/resiliency/OriginResourcesSpec.kt | 57 +++++++++++-------- 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt index 6dfb44cb18..1e26f744da 100644 --- a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt @@ -26,6 +26,11 @@ import com.hotels.styx.support.StyxServerProvider import com.hotels.styx.support.proxyHttpHostHeader import com.hotels.styx.support.wait import io.kotlintest.Spec +import io.kotlintest.eventually +import io.kotlintest.matchers.boolean.shouldBeTrue +import io.kotlintest.matchers.numerics.shouldBeGreaterThan +import io.kotlintest.matchers.withClue +import io.kotlintest.seconds import io.kotlintest.shouldBe import io.kotlintest.specs.StringSpec import java.io.ByteArrayInputStream @@ -47,37 +52,36 @@ class OriginResourcesSpec : StringSpec() { init { "Cleans up threads after use" { - "---" + (1..count) - .map { appDeclaration("aaa-$it") } - .joinToString("\n") - .let { writeConfig(styxOriginsFile, it) } - - // Wait for styx to pick up the configuration - Thread.sleep(2000) - - (1..count).forEach { - client.send(get("/aaa-$it").header(HOST, styxServer().proxyHttpHostHeader()).build()) - .wait(debug = false) - .status() shouldBe OK - } - val threadCountBefore = threadCount("Styx-Client-Worker") + val threadCountBefore = run { + writeConfig(styxOriginsFile, + "---\n" + (1..count) + .map { appDeclaration("aaa-$it") } + .joinToString("\n")) - "---" + (1..count) - .map { appDeclaration("bbb-$it") } - .joinToString("\n") - .let { writeConfig(styxOriginsFile, it) } + eventually(2.seconds, AssertionError::class.java) { + (1..count).all { configurationApplied("/aaa-$it") }.shouldBeTrue() + } - // Wait for styx to pick up the new configuration - Thread.sleep(2000) + threadCount("Styx-Client-Worker") + } - (1..count).forEach { - client.send(get("/bbb-$it").header(HOST, styxServer().proxyHttpHostHeader()).build()) - .wait(debug = false) - .status() shouldBe OK + withClue("No `Styx-Client-Worker` threads found. Has the thrad name changed?") { + threadCountBefore shouldBeGreaterThan 1 } - val threadCountAfter = threadCount("Styx-Client-Worker") + val threadCountAfter = run { + writeConfig(styxOriginsFile, + "---\n" + (1..count) + .map { appDeclaration("bbb-$it") } + .joinToString("\n")) + + eventually(2.seconds, AssertionError::class.java) { + (1..count).all { configurationApplied("/bbb-$it") }.shouldBeTrue() + } + + threadCount("Styx-Client-Worker") + } threadCountBefore shouldBe threadCountAfter } @@ -149,6 +153,9 @@ class OriginResourcesSpec : StringSpec() { .filter { it.contains(namePattern) } .count() + fun configurationApplied(prefix: String) = client.send(get(prefix).header(HOST, styxServer().proxyHttpHostHeader()).build()) + .wait(debug = false) + .status() == OK } val client: StyxHttpClient = StyxHttpClient.Builder().build() From 718eada9aced39ab0848b752961898cee6f03ee5 Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Tue, 4 Jun 2019 07:57:04 +0100 Subject: [PATCH 6/7] Add a new test. --- .../styx/resiliency/OriginResourcesSpec.kt | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt index 1e26f744da..0fb0f61c38 100644 --- a/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt +++ b/system-tests/ft-suite/src/test/kotlin/com/hotels/styx/resiliency/OriginResourcesSpec.kt @@ -51,7 +51,25 @@ class OriginResourcesSpec : StringSpec() { init { - "Cleans up threads after use" { + "Client thread pool configuration" { + val clientThreadCount = run { + writeConfig(styxOriginsFile, + "---\n" + (1..count) + .map { appDeclaration("aaa-$it") } + .joinToString("\n")) + + eventually(2.seconds, AssertionError::class.java) { + (1..count).all { configurationApplied("/aaa-$it") }.shouldBeTrue() + } + + threadCount("Styx-Client-Worker") + } + + // From the static configuration below + clientThreadCount shouldBe 3 + } + + "Uses the same thread pool for reloaded origins" { val threadCountBefore = run { writeConfig(styxOriginsFile, @@ -114,6 +132,7 @@ class OriginResourcesSpec : StringSpec() { connectors: http: port: 0 + clientWorkerThreadsCount: 3 admin: connectors: From 912c2ed4bfed4daa770a3358e71f4c5b15acd064 Mon Sep 17 00:00:00 2001 From: Mikko Karjalainen Date: Tue, 4 Jun 2019 09:57:45 +0100 Subject: [PATCH 7/7] Tidy up. --- .../com/hotels/styx/routing/handlers/BackendServiceProxy.java | 3 ++- .../com/hotels/styx/routing/StaticPipelineBuilderTest.java | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/components/proxy/src/main/java/com/hotels/styx/routing/handlers/BackendServiceProxy.java b/components/proxy/src/main/java/com/hotels/styx/routing/handlers/BackendServiceProxy.java index 5257c0c857..9bfc1094e7 100644 --- a/components/proxy/src/main/java/com/hotels/styx/routing/handlers/BackendServiceProxy.java +++ b/components/proxy/src/main/java/com/hotels/styx/routing/handlers/BackendServiceProxy.java @@ -52,7 +52,8 @@ public class BackendServiceProxy implements RoutingObject { private BackendServiceProxy( BackendServiceClientFactory serviceClientFactory, - Registry registry, Environment environment, + Registry registry, + Environment environment, EventLoopGroup eventLoopGroup, Class nettySocketChannelClass) { BackendServicesRouter router = new BackendServicesRouter(serviceClientFactory, environment, eventLoopGroup, nettySocketChannelClass); diff --git a/components/proxy/src/test/java/com/hotels/styx/routing/StaticPipelineBuilderTest.java b/components/proxy/src/test/java/com/hotels/styx/routing/StaticPipelineBuilderTest.java index dc4c7a501e..2b5a2db300 100644 --- a/components/proxy/src/test/java/com/hotels/styx/routing/StaticPipelineBuilderTest.java +++ b/components/proxy/src/test/java/com/hotels/styx/routing/StaticPipelineBuilderTest.java @@ -68,7 +68,7 @@ public void staticPipelineBuilderTest() { @Test public void buildsInterceptorPipelineForBackendServices() throws Exception { - HttpHandler handler = new StaticPipelineFactory(clientFactory, environment, registry, ImmutableList.of(), eventLoopGroup, socketChannelClass,false).build(); + HttpHandler handler = new StaticPipelineFactory(clientFactory, environment, registry, ImmutableList.of(), eventLoopGroup, socketChannelClass, false).build(); LiveHttpResponse response = Mono.from(handler.handle(get("/foo").build(), HttpInterceptorContext.create())).block(); assertThat(response.status(), is(OK)); } @@ -80,7 +80,7 @@ public void appliesPluginsInOrderTheyAreConfigured() throws Exception { interceptor("Test-B", appendResponseHeader("X-From-Plugin", "B")) ); - HttpHandler handler = new StaticPipelineFactory(clientFactory, environment, registry, plugins, eventLoopGroup, socketChannelClass,false).build(); + HttpHandler handler = new StaticPipelineFactory(clientFactory, environment, registry, plugins, eventLoopGroup, socketChannelClass, false).build(); LiveHttpResponse response = Mono.from(handler.handle(get("/foo").build(), HttpInterceptorContext.create())).block(); assertThat(response.status(), is(OK));