Skip to content

fix: add configurable evictIdleConnections #431

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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() {

Expand All @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand Down Expand Up @@ -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 {
Expand Down