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 {