diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/HttpClientUtils.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/HttpClientUtils.java index 8a4d104d5..c35bb4b3f 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/HttpClientUtils.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/HttpClientUtils.java @@ -23,9 +23,11 @@ */ public final class HttpClientUtils { - private static final int CONNECTION_TIMEOUT_MS = 10000; - private static final int CONNECTION_REQUEST_TIMEOUT_MS = 5000; - private static final int SOCKET_TIMEOUT_MS = 10000; + public static final int CONNECTION_TIMEOUT_MS = 10000; + public static final int CONNECTION_REQUEST_TIMEOUT_MS = 5000; + public static final int SOCKET_TIMEOUT_MS = 10000; + + private static RequestConfig requestConfigWithTimeout; private HttpClientUtils() { } @@ -36,6 +38,17 @@ private HttpClientUtils() { .setSocketTimeout(SOCKET_TIMEOUT_MS) .build(); + public static RequestConfig getDefaultRequestConfigWithTimeout(int timeoutMillis) { + if (requestConfigWithTimeout == null) { + requestConfigWithTimeout = RequestConfig.custom() + .setConnectTimeout(timeoutMillis) + .setConnectionRequestTimeout(CONNECTION_REQUEST_TIMEOUT_MS) + .setSocketTimeout(SOCKET_TIMEOUT_MS) + .build(); + } + return requestConfigWithTimeout; + } + public static OptimizelyHttpClient getDefaultHttpClient() { return OptimizelyHttpClient.builder().build(); } 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 37c2163ac..f4040276f 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 @@ -1,6 +1,6 @@ /** * - * Copyright 2019, Optimizely + * Copyright 2019, 2022 Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,8 @@ import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.client.HttpClients; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.io.Closeable; import java.io.IOException; @@ -38,6 +40,8 @@ */ public class OptimizelyHttpClient implements Closeable { + private static final Logger logger = LoggerFactory.getLogger(OptimizelyHttpClient.class); + private final CloseableHttpClient httpClient; OptimizelyHttpClient(CloseableHttpClient httpClient) { @@ -78,6 +82,7 @@ public static class Builder { // force-close the connection after this idle time (with 0, eviction is disabled by default) long evictConnectionIdleTimePeriod = 0; TimeUnit evictConnectionIdleTimeUnit = TimeUnit.MILLISECONDS; + private int timeoutMillis = HttpClientUtils.CONNECTION_TIMEOUT_MS; private Builder() { @@ -103,6 +108,11 @@ public Builder withEvictIdleConnections(long maxIdleTime, TimeUnit maxIdleTimeUn this.evictConnectionIdleTimeUnit = maxIdleTimeUnit; return this; } + + public Builder setTimeoutMillis(int timeoutMillis) { + this.timeoutMillis = timeoutMillis; + return this; + } public OptimizelyHttpClient build() { PoolingHttpClientConnectionManager poolingHttpClientConnectionManager = new PoolingHttpClientConnectionManager(); @@ -111,11 +121,13 @@ public OptimizelyHttpClient build() { poolingHttpClientConnectionManager.setValidateAfterInactivity(validateAfterInactivity); HttpClientBuilder builder = HttpClients.custom() - .setDefaultRequestConfig(HttpClientUtils.DEFAULT_REQUEST_CONFIG) + .setDefaultRequestConfig(HttpClientUtils.getDefaultRequestConfigWithTimeout(timeoutMillis)) .setConnectionManager(poolingHttpClientConnectionManager) .disableCookieManagement() .useSystemProperties(); + logger.debug("Creating HttpClient with timeout: " + timeoutMillis); + if (evictConnectionIdleTimePeriod > 0) { builder.evictIdleConnections(evictConnectionIdleTimePeriod, evictConnectionIdleTimeUnit); } diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/odp/DefaultODPApiManager.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/odp/DefaultODPApiManager.java index 576701066..636ed8eec 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/odp/DefaultODPApiManager.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/odp/DefaultODPApiManager.java @@ -33,15 +33,27 @@ public class DefaultODPApiManager implements ODPApiManager { private static final Logger logger = LoggerFactory.getLogger(DefaultODPApiManager.class); - private final OptimizelyHttpClient httpClient; + private final OptimizelyHttpClient httpClientSegments; + private final OptimizelyHttpClient httpClientEvents; public DefaultODPApiManager() { this(OptimizelyHttpClient.builder().build()); } + public DefaultODPApiManager(int segmentFetchTimeoutMillis, int eventDispatchTimeoutMillis) { + httpClientSegments = OptimizelyHttpClient.builder().setTimeoutMillis(segmentFetchTimeoutMillis).build(); + if (segmentFetchTimeoutMillis == eventDispatchTimeoutMillis) { + // If the timeouts are same, single httpClient can be used for both. + httpClientEvents = httpClientSegments; + } else { + httpClientEvents = OptimizelyHttpClient.builder().setTimeoutMillis(eventDispatchTimeoutMillis).build(); + } + } + @VisibleForTesting DefaultODPApiManager(OptimizelyHttpClient httpClient) { - this.httpClient = httpClient; + this.httpClientSegments = httpClient; + this.httpClientEvents = httpClient; } @VisibleForTesting @@ -150,7 +162,7 @@ public String fetchQualifiedSegments(String apiKey, String apiEndpoint, String u CloseableHttpResponse response = null; try { - response = httpClient.execute(request); + response = httpClientSegments.execute(request); } catch (IOException e) { logger.error("Error retrieving response from ODP service", e); return null; @@ -211,7 +223,7 @@ public Integer sendEvents(String apiKey, String apiEndpoint, String eventPayload CloseableHttpResponse response = null; try { - response = httpClient.execute(request); + response = httpClientEvents.execute(request); } catch (IOException e) { logger.error("Error retrieving response from event request", e); return 0; diff --git a/core-httpclient-impl/src/test/java/com/optimizely/ab/odp/DefaultODPApiManagerTest.java b/core-httpclient-impl/src/test/java/com/optimizely/ab/odp/DefaultODPApiManagerTest.java index 638a3e1ee..93b728fba 100644 --- a/core-httpclient-impl/src/test/java/com/optimizely/ab/odp/DefaultODPApiManagerTest.java +++ b/core-httpclient-impl/src/test/java/com/optimizely/ab/odp/DefaultODPApiManagerTest.java @@ -28,7 +28,6 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; -import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; @@ -129,4 +128,20 @@ public void eventDispatchFailStatus() throws Exception { apiManager.sendEvents("testKey", "testEndpoint", "[]]"); logbackVerifier.expectMessage(Level.ERROR, "ODP event send failed (Response code: 400, null)"); } + + @Test + public void apiTimeouts() { + // Default timeout is 10 seconds + new DefaultODPApiManager(); + logbackVerifier.expectMessage(Level.DEBUG, "Creating HttpClient with timeout: 10000", 1); + + // Same timeouts result in single httpclient + new DefaultODPApiManager(2222, 2222); + logbackVerifier.expectMessage(Level.DEBUG, "Creating HttpClient with timeout: 2222", 1); + + // Different timeouts result in different HttpClients + new DefaultODPApiManager(3333, 4444); + logbackVerifier.expectMessage(Level.DEBUG, "Creating HttpClient with timeout: 3333", 1); + logbackVerifier.expectMessage(Level.DEBUG, "Creating HttpClient with timeout: 4444", 1); + } }