From 5d09760b948ec9fd798716d9e4fc18b878dae56a Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 1 Oct 2019 19:27:17 -0400 Subject: [PATCH] Applying #3407 to the 2.1.x branch --- .../route/SimpleHostRoutingFilter.java | 29 +++++-- .../route/SimpleHostRoutingFilterTests.java | 81 +++++++++++++++++-- 2 files changed, 97 insertions(+), 13 deletions(-) diff --git a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java index 71deaaf18..ef74b5872 100644 --- a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java +++ b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java @@ -81,6 +81,7 @@ import static org.springframework.cloud.netflix.zuul.filters.support.FilterConst * @author Dave Syer * @author Bilal Alp * @author Gang Li + * @author Denys Ivano */ public class SimpleHostRoutingFilter extends ZuulFilter implements ApplicationListener { @@ -132,12 +133,20 @@ public class SimpleHostRoutingFilter extends ZuulFilter if (createNewClient) { try { - SimpleHostRoutingFilter.this.httpClient.close(); + this.httpClient.close(); } catch (IOException ex) { log.error("error closing client", ex); } - SimpleHostRoutingFilter.this.httpClient = newClient(); + // Re-create connection manager (may be shut down on HTTP client close) + try { + this.connectionManager.shutdown(); + } + catch (RuntimeException ex) { + log.error("error shutting down connection manager", ex); + } + this.connectionManager = newConnectionManager(); + this.httpClient = newClient(); } } } @@ -170,12 +179,7 @@ public class SimpleHostRoutingFilter extends ZuulFilter @PostConstruct private void initialize() { if (!customHttpClient) { - this.connectionManager = connectionManagerFactory.newConnectionManager( - !this.sslHostnameValidationEnabled, - this.hostProperties.getMaxTotalConnections(), - this.hostProperties.getMaxPerRouteConnections(), - this.hostProperties.getTimeToLive(), - this.hostProperties.getTimeUnit(), null); + this.connectionManager = newConnectionManager(); this.httpClient = newClient(); this.connectionManagerTimer.schedule(new TimerTask() { @Override @@ -287,6 +291,15 @@ public class SimpleHostRoutingFilter extends ZuulFilter return connectionManager; } + protected HttpClientConnectionManager newConnectionManager() { + return connectionManagerFactory.newConnectionManager( + !this.sslHostnameValidationEnabled, + this.hostProperties.getMaxTotalConnections(), + this.hostProperties.getMaxPerRouteConnections(), + this.hostProperties.getTimeToLive(), this.hostProperties.getTimeUnit(), + null); + } + protected CloseableHttpClient newClient() { final RequestConfig requestConfig = RequestConfig.custom() .setConnectionRequestTimeout( diff --git a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java index c0b58edfd..93d04aa1f 100644 --- a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java +++ b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java @@ -22,7 +22,9 @@ import java.lang.reflect.Field; import java.net.URL; import java.util.Arrays; import java.util.Collections; +import java.util.HashSet; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import com.netflix.zuul.context.RequestContext; import com.netflix.zuul.monitoring.CounterFactory; @@ -31,6 +33,7 @@ import org.apache.http.HttpRequest; import org.apache.http.client.config.RequestConfig; import org.apache.http.client.methods.Configurable; import org.apache.http.client.methods.HttpPatch; +import org.apache.http.conn.HttpClientConnectionManager; import org.apache.http.entity.InputStreamEntity; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; @@ -66,6 +69,7 @@ import static org.springframework.util.StreamUtils.copyToByteArray; * @author Andreas Kluth * @author Spencer Gibb * @author Gang Li + * @author Denys Ivano */ public class SimpleHostRoutingFilterTests { @@ -175,16 +179,83 @@ public class SimpleHostRoutingFilterTests { public void zuulHostKeysUpdateHttpClient() { setupContext(); SimpleHostRoutingFilter filter = getFilter(); - CloseableHttpClient httpClient = (CloseableHttpClient) ReflectionTestUtils - .getField(filter, "httpClient"); + CloseableHttpClient httpClient = extractHttpClient(filter); EnvironmentChangeEvent event = new EnvironmentChangeEvent( Collections.singleton("zuul.host.mykey")); - filter.onPropertyChange(event); - CloseableHttpClient newhttpClient = (CloseableHttpClient) ReflectionTestUtils - .getField(filter, "httpClient"); + filter.onApplicationEvent(event); + CloseableHttpClient newhttpClient = extractHttpClient(filter); Assertions.assertThat(httpClient).isNotEqualTo(newhttpClient); } + private CloseableHttpClient extractHttpClient(SimpleHostRoutingFilter filter) { + return (CloseableHttpClient) ReflectionTestUtils.getField(filter, "httpClient"); + } + + @Test + public void zuulHostKeysUpdateHttpClientConnectionManagerIsNotShutDown() { + setupContext(); + SimpleHostRoutingFilter filter = getFilter(); + EnvironmentChangeEvent event = new EnvironmentChangeEvent( + Collections.singleton("zuul.host.mykey")); + filter.onApplicationEvent(event); + + CloseableHttpClient httpClient = extractHttpClient(filter); + PoolingHttpClientConnectionManager connMgr = (PoolingHttpClientConnectionManager) extractConnectionManager( + httpClient); + AtomicBoolean isShutDown = getField(connMgr, "isShutDown"); + assertThat(isShutDown.get()).as("Connection manager shut down").isFalse(); + Object pool = getField(connMgr, "pool"); + assertThat(pool).hasFieldOrPropertyWithValue("isShutDown", false); + } + + private HttpClientConnectionManager extractConnectionManager( + CloseableHttpClient httpClient) { + // Default implementation is org.apache.http.impl.client.InternalHttpClient + return getField(httpClient, "connManager"); + } + + @Test + public void zuulHostKeysUpdateHttpClientUsesNewConnectionManager() { + setupContext(); + SimpleHostRoutingFilter filter = getFilter(); + PoolingHttpClientConnectionManager connMgr = (PoolingHttpClientConnectionManager) filter + .getConnectionManager(); + EnvironmentChangeEvent event = new EnvironmentChangeEvent( + Collections.singleton("zuul.host.mykey")); + filter.onApplicationEvent(event); + + PoolingHttpClientConnectionManager newConnMgr = (PoolingHttpClientConnectionManager) filter + .getConnectionManager(); + CloseableHttpClient httpClient = extractHttpClient(filter); + HttpClientConnectionManager usedConnMgr = extractConnectionManager(httpClient); + assertThat(usedConnMgr).isNotEqualTo(connMgr); + assertThat(usedConnMgr).isEqualTo(newConnMgr); + } + + @Test + public void zuulHostKeysUpdateConnectionManagerPropertiesAreChanged() { + setupContext(); + SimpleHostRoutingFilter filter = getFilter(); + ZuulProperties.Host host = context.getBean(ZuulProperties.class).getHost(); + host.setMaxTotalConnections(50); + host.setMaxPerRouteConnections(10); + host.setTimeToLive(10); + host.setTimeUnit(TimeUnit.SECONDS); + EnvironmentChangeEvent event = new EnvironmentChangeEvent( + new HashSet<>(Arrays.asList("zuul.host.maxTotalConnections", + "zuul.host.maxPerRouteConnections", "zuul.host.timeToLive", + "zuul.host.timeUnit"))); + filter.onApplicationEvent(event); + + PoolingHttpClientConnectionManager connMgr = (PoolingHttpClientConnectionManager) filter + .getConnectionManager(); + assertThat(connMgr.getMaxTotal()).isEqualTo(50); + assertThat(connMgr.getDefaultMaxPerRoute()).isEqualTo(10); + Object pool = getField(connMgr, "pool"); + assertThat(pool).hasFieldOrPropertyWithValue("timeToLive", 10L); + assertThat(pool).hasFieldOrPropertyWithValue("timeUnit", TimeUnit.SECONDS); + } + @Test public void getRequestBody() throws Exception { setupContext();