fix: add configurable evictIdleConnections (#431)

Author: jaeopt

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)}

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);
         }

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,13 +47,17 @@ 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";
 
     public static final long DEFAULT_POLLING_DURATION  = 5;
     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) {

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";

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");

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 {