diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java index 2ba52d6e4..5d267d41e 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyFactory.java @@ -123,6 +123,28 @@ public static void setBlockingTimeout(long blockingDuration, TimeUnit blockingTi PropertyUtils.set(HttpProjectConfigManager.CONFIG_BLOCKING_UNIT, blockingTimeout.toString()); } + /** + * Convenience method for setting the evict idle connections. + * {@link HttpProjectConfigManager.Builder#withEvictIdleConnections(long, TimeUnit)} + * + * @param maxIdleTime The connection idle time duration (0 to disable eviction) + * @param maxIdleTimeUnit The connection idle time unit + */ + public static void setEvictIdleConnections(long maxIdleTime, TimeUnit maxIdleTimeUnit) { + if (maxIdleTimeUnit == null) { + logger.warn("TimeUnit cannot be null. Reverting to default configuration."); + return; + } + + if (maxIdleTime < 0) { + logger.warn("Timeout cannot be < 0. Reverting to default configuration."); + return; + } + + PropertyUtils.set(HttpProjectConfigManager.CONFIG_EVICT_DURATION, Long.toString(maxIdleTime)); + PropertyUtils.set(HttpProjectConfigManager.CONFIG_EVICT_UNIT, maxIdleTimeUnit.toString()); + } + /** * Convenience method for setting the polling interval on System properties. * {@link HttpProjectConfigManager.Builder#withPollingInterval(Long, TimeUnit)} diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java index 86801396a..37c2163ac 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java @@ -22,11 +22,13 @@ import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpUriRequest; import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.client.HttpClients; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; import java.io.Closeable; import java.io.IOException; +import java.util.concurrent.TimeUnit; /** * Basic HttpClient wrapper to be utilized for fetching the datafile @@ -73,6 +75,9 @@ public static class Builder { private int maxPerRoute = 20; // Defines period of inactivity in milliseconds after which persistent connections must be re-validated prior to being leased to the consumer. private int validateAfterInactivity = 5000; + // force-close the connection after this idle time (with 0, eviction is disabled by default) + long evictConnectionIdleTimePeriod = 0; + TimeUnit evictConnectionIdleTimeUnit = TimeUnit.MILLISECONDS; private Builder() { @@ -93,18 +98,29 @@ public Builder withValidateAfterInactivity(int validateAfterInactivity) { return this; } + public Builder withEvictIdleConnections(long maxIdleTime, TimeUnit maxIdleTimeUnit) { + this.evictConnectionIdleTimePeriod = maxIdleTime; + this.evictConnectionIdleTimeUnit = maxIdleTimeUnit; + return this; + } + public OptimizelyHttpClient build() { PoolingHttpClientConnectionManager poolingHttpClientConnectionManager = new PoolingHttpClientConnectionManager(); poolingHttpClientConnectionManager.setMaxTotal(maxTotalConnections); poolingHttpClientConnectionManager.setDefaultMaxPerRoute(maxPerRoute); poolingHttpClientConnectionManager.setValidateAfterInactivity(validateAfterInactivity); - CloseableHttpClient closableHttpClient = HttpClients.custom() + HttpClientBuilder builder = HttpClients.custom() .setDefaultRequestConfig(HttpClientUtils.DEFAULT_REQUEST_CONFIG) .setConnectionManager(poolingHttpClientConnectionManager) .disableCookieManagement() - .useSystemProperties() - .build(); + .useSystemProperties(); + + if (evictConnectionIdleTimePeriod > 0) { + builder.evictIdleConnections(evictConnectionIdleTimePeriod, evictConnectionIdleTimeUnit); + } + + CloseableHttpClient closableHttpClient = builder.build(); return new OptimizelyHttpClient(closableHttpClient); } diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java index c771da9fa..cef13fdcd 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/config/HttpProjectConfigManager.java @@ -27,6 +27,7 @@ import org.apache.http.client.ClientProtocolException; import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.util.EntityUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -46,6 +47,8 @@ public class HttpProjectConfigManager extends PollingProjectConfigManager { public static final String CONFIG_POLLING_UNIT = "http.project.config.manager.polling.unit"; public static final String CONFIG_BLOCKING_DURATION = "http.project.config.manager.blocking.duration"; public static final String CONFIG_BLOCKING_UNIT = "http.project.config.manager.blocking.unit"; + public static final String CONFIG_EVICT_DURATION = "http.project.config.manager.evict.duration"; + public static final String CONFIG_EVICT_UNIT = "http.project.config.manager.evict.unit"; public static final String CONFIG_SDK_KEY = "http.project.config.manager.sdk.key"; public static final String CONFIG_DATAFILE_AUTH_TOKEN = "http.project.config.manager.datafile.auth.token"; @@ -53,6 +56,8 @@ public class HttpProjectConfigManager extends PollingProjectConfigManager { public static final TimeUnit DEFAULT_POLLING_UNIT = TimeUnit.MINUTES; public static final long DEFAULT_BLOCKING_DURATION = 10; public static final TimeUnit DEFAULT_BLOCKING_UNIT = TimeUnit.SECONDS; + public static final long DEFAULT_EVICT_DURATION = 1; + public static final TimeUnit DEFAULT_EVICT_UNIT = TimeUnit.MINUTES; private static final Logger logger = LoggerFactory.getLogger(HttpProjectConfigManager.class); @@ -178,6 +183,10 @@ public static class Builder { long blockingTimeoutPeriod = PropertyUtils.getLong(CONFIG_BLOCKING_DURATION, DEFAULT_BLOCKING_DURATION); TimeUnit blockingTimeoutUnit = PropertyUtils.getEnum(CONFIG_BLOCKING_UNIT, TimeUnit.class, DEFAULT_BLOCKING_UNIT); + // force-close the persistent connection after this idle time + long evictConnectionIdleTimePeriod = PropertyUtils.getLong(CONFIG_EVICT_DURATION, DEFAULT_EVICT_DURATION); + TimeUnit evictConnectionIdleTimeUnit = PropertyUtils.getEnum(CONFIG_EVICT_UNIT, TimeUnit.class, DEFAULT_EVICT_UNIT); + public Builder withDatafile(String datafile) { this.datafile = datafile; return this; @@ -208,6 +217,25 @@ public Builder withOptimizelyHttpClient(OptimizelyHttpClient httpClient) { return this; } + /** + * Makes HttpClient proactively evict idle connections from theœ + * connection pool using a background thread. + * + * @see org.apache.http.impl.client.HttpClientBuilder#evictIdleConnections(long, TimeUnit) + * + * @param maxIdleTime maximum time persistent connections can stay idle while kept alive + * in the connection pool. Connections whose inactivity period exceeds this value will + * get closed and evicted from the pool. Set to 0 to disable eviction. + * @param maxIdleTimeUnit time unit for the above parameter. + * + * @return A HttpProjectConfigManager builder + */ + public Builder withEvictIdleConnections(long maxIdleTime, TimeUnit maxIdleTimeUnit) { + this.evictConnectionIdleTimePeriod = maxIdleTime; + this.evictConnectionIdleTimeUnit = maxIdleTimeUnit; + return this; + } + /** * Configure time to block before Completing the future. This timeout is used on the first call * to {@link PollingProjectConfigManager#getConfig()}. If the timeout is exceeded then the @@ -300,7 +328,9 @@ public HttpProjectConfigManager build(boolean defer) { } if (httpClient == null) { - httpClient = HttpClientUtils.getDefaultHttpClient(); + httpClient = OptimizelyHttpClient.builder() + .withEvictIdleConnections(evictConnectionIdleTimePeriod, evictConnectionIdleTimeUnit) + .build(); } if (url == null) { diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java index c860ee98b..b6d006173 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyFactoryTest.java @@ -45,6 +45,8 @@ public void setUp() { PropertyUtils.clear(HttpProjectConfigManager.CONFIG_POLLING_UNIT); PropertyUtils.clear(HttpProjectConfigManager.CONFIG_BLOCKING_DURATION); PropertyUtils.clear(HttpProjectConfigManager.CONFIG_BLOCKING_UNIT); + PropertyUtils.clear(HttpProjectConfigManager.CONFIG_EVICT_DURATION); + PropertyUtils.clear(HttpProjectConfigManager.CONFIG_EVICT_UNIT); PropertyUtils.clear(HttpProjectConfigManager.CONFIG_SDK_KEY); } @@ -152,6 +154,27 @@ public void setInvalidBlockingTimeout() { assertNull(PropertyUtils.getEnum(HttpProjectConfigManager.CONFIG_POLLING_UNIT, TimeUnit.class)); } + @Test + public void setEvictIdleConnections() { + Long duration = 2000L; + TimeUnit timeUnit = TimeUnit.SECONDS; + OptimizelyFactory.setEvictIdleConnections(duration, timeUnit); + + assertEquals(duration, PropertyUtils.getLong(HttpProjectConfigManager.CONFIG_EVICT_DURATION)); + assertEquals(timeUnit, PropertyUtils.getEnum(HttpProjectConfigManager.CONFIG_EVICT_UNIT, TimeUnit.class)); + } + + @Test + public void setInvalidEvictIdleConnections() { + OptimizelyFactory.setEvictIdleConnections(-1, TimeUnit.MICROSECONDS); + assertNull(PropertyUtils.getLong(HttpProjectConfigManager.CONFIG_EVICT_DURATION)); + assertNull(PropertyUtils.getEnum(HttpProjectConfigManager.CONFIG_EVICT_UNIT, TimeUnit.class)); + + OptimizelyFactory.setEvictIdleConnections(10, null); + assertNull(PropertyUtils.getLong(HttpProjectConfigManager.CONFIG_EVICT_DURATION)); + assertNull(PropertyUtils.getEnum(HttpProjectConfigManager.CONFIG_EVICT_UNIT, TimeUnit.class)); + } + @Test public void setSdkKey() { String expected = "sdk-key"; diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyHttpClientTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyHttpClientTest.java index 8b92e6fc1..7dc61f0f9 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyHttpClientTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/OptimizelyHttpClientTest.java @@ -27,7 +27,10 @@ import org.junit.Test; import java.io.IOException; +import java.util.concurrent.TimeUnit; +import static com.optimizely.ab.OptimizelyHttpClient.builder; +import static java.util.concurrent.TimeUnit.*; import static org.junit.Assert.*; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -46,24 +49,39 @@ public void tearDown() { @Test public void testDefaultConfiguration() { - OptimizelyHttpClient optimizelyHttpClient = OptimizelyHttpClient.builder().build(); + OptimizelyHttpClient optimizelyHttpClient = builder().build(); assertTrue(optimizelyHttpClient.getHttpClient() instanceof CloseableHttpClient); } @Test public void testNonDefaultConfiguration() { - OptimizelyHttpClient optimizelyHttpClient = OptimizelyHttpClient.builder() + OptimizelyHttpClient optimizelyHttpClient = builder() .withValidateAfterInactivity(1) .withMaxPerRoute(2) .withMaxTotalConnections(3) + .withEvictIdleConnections(5, MINUTES) .build(); assertTrue(optimizelyHttpClient.getHttpClient() instanceof CloseableHttpClient); } + @Test + public void testEvictTime() { + OptimizelyHttpClient.Builder builder = builder(); + long expectedPeriod = builder.evictConnectionIdleTimePeriod; + TimeUnit expectedTimeUnit = builder.evictConnectionIdleTimeUnit; + + assertEquals(expectedPeriod, 0L); + assertEquals(expectedTimeUnit, MILLISECONDS); + + builder.withEvictIdleConnections(10L, SECONDS); + assertEquals(10, builder.evictConnectionIdleTimePeriod); + assertEquals(SECONDS, builder.evictConnectionIdleTimeUnit); + } + @Test(expected = HttpHostConnectException.class) public void testProxySettings() throws IOException { - OptimizelyHttpClient optimizelyHttpClient = OptimizelyHttpClient.builder().build(); + OptimizelyHttpClient optimizelyHttpClient = builder().build(); // If this request succeeds then the proxy config was not picked up. HttpGet get = new HttpGet("https://www.optimizely.com"); diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java index fb76a27b7..c61a1f01a 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/config/HttpProjectConfigManagerTest.java @@ -42,6 +42,7 @@ import static com.optimizely.ab.config.HttpProjectConfigManager.*; import static java.util.concurrent.TimeUnit.SECONDS; +import static java.util.concurrent.TimeUnit.MINUTES; import static org.junit.Assert.*; import static org.mockito.Matchers.any; import static org.mockito.Mockito.*; @@ -254,6 +255,20 @@ public void testInvalidBlockingTimeout() { assertEquals(SECONDS, builder.blockingTimeoutUnit); } + @Test + public void testEvictTime() { + Builder builder = builder(); + long expectedPeriod = builder.evictConnectionIdleTimePeriod; + TimeUnit expectedTimeUnit = builder.evictConnectionIdleTimeUnit; + + assertEquals(expectedPeriod, 1L); + assertEquals(expectedTimeUnit, MINUTES); + + builder.withEvictIdleConnections(10L, SECONDS); + assertEquals(10, builder.evictConnectionIdleTimePeriod); + assertEquals(SECONDS, builder.evictConnectionIdleTimeUnit); + } + @Test @Ignore public void testGetDatafileHttpResponse2XX() throws Exception {