Skip to content

Commit

Permalink
REST HTTP Client Keep Alive and socket/connect timeout NOK
Browse files Browse the repository at this point in the history 
397411
  • Loading branch information
@paolobazzi
paolobazzi committed Oct 4, 2024
1 parent 207881f commit 737c2e5
Show file tree
Hide file tree
Showing 3 changed files with 138 additions and 28 deletions.
85 changes: 67 additions & 18 deletions ...ey.client/src/main/java/org/eclipse/scout/rt/rest/jersey/client/ScoutApacheConnector.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,11 @@
import org.apache.hc.core5.http.config.RegistryBuilder;
import org.apache.hc.core5.http.impl.io.DefaultHttpRequestWriterFactory;
import org.apache.hc.core5.http.io.HttpConnectionFactory;
import org.apache.hc.core5.http.io.SocketConfig;
import org.apache.hc.core5.http.io.entity.AbstractHttpEntity;
import org.apache.hc.core5.http.io.entity.BufferedHttpEntity;
import org.apache.hc.core5.util.TimeValue;
import org.apache.hc.core5.util.Timeout;
import org.eclipse.scout.rt.platform.BEANS;
import org.eclipse.scout.rt.platform.config.AbstractIntegerConfigProperty;
import org.eclipse.scout.rt.platform.config.AbstractLongConfigProperty;
Expand Down Expand Up @@ -177,10 +179,14 @@ RegistryBuilder.<ConnectionSocketFactory> create()
.register("http", PlainConnectionSocketFactory.getSocketFactory())
.register("https", sslConnectionSocketFactory)
.build(),
null, null, TimeValue.ofMilliseconds(getKeepAliveTimeoutMillis(config)), connFactory);
null, null, TimeValue.ofMilliseconds(getConnectionTimeToLiveMillis(config)), connFactory);

Timeout connectTimeout = getConnectTimeoutMillis(config);
Timeout sockerTimeout = getReadTimeoutMillis(config);
Builder connectionConfigBuilder = ConnectionConfig.custom()
.setTimeToLive(TimeValue.ofMilliseconds(getKeepAliveTimeoutMillis(config)));
.setConnectTimeout(connectTimeout)
.setSocketTimeout(sockerTimeout)
.setTimeToLive(TimeValue.ofMilliseconds(getConnectionTimeToLiveMillis(config)));

final int validateAfterInactivityMillis = getValidateAfterInactivityMillis(config);
if (validateAfterInactivityMillis > 0) {
Expand All @@ -197,11 +203,31 @@ RegistryBuilder.<ConnectionSocketFactory> create()
connectionManager.setDefaultMaxPerRoute(defaultMaxPerRoute);
}

SocketConfig.Builder socketConfigBuilder = SocketConfig.custom()
.setSoTimeout(sockerTimeout);

connectionManager.setDefaultConnectionConfig(connectionConfigBuilder.build());
connectionManager.setDefaultSocketConfig(socketConfigBuilder.build());
initMetrics(config, connectionManager);
return connectionManager;
}

protected Timeout getConnectTimeoutMillis(Configuration config) {
Long timeout = TypeCastUtility.castValue(config.getProperty(RestClientProperties.CONNECT_TIMEOUT), Long.class);
if (timeout != null) {
return Timeout.of(timeout, TimeUnit.MILLISECONDS);
}
return Timeout.DISABLED;
}

protected Timeout getReadTimeoutMillis(Configuration config) {
Long timeout = TypeCastUtility.castValue(config.getProperty(RestClientProperties.READ_TIMEOUT), Long.class);
if (timeout != null) {
return Timeout.of(timeout, TimeUnit.MILLISECONDS);
}
return Timeout.DISABLED;
}

/**
* Initializes metrics for this HTTP connection manager (if configured by setting a value for
* {@link RestClientProperties#OTEL_HTTP_CLIENT_NAME}).
Expand Down Expand Up @@ -233,22 +259,13 @@ protected String[] split(String input) {
}

/**
* @deprecated This method will be removed with Scout release 25.1. Use the configuration property to change this
* configuration or override method {@link #getKeepAliveTimeoutMillis(Configuration)}.
*/
@Deprecated
@SuppressWarnings("DeprecatedIsStillUsed")
protected long getKeepAliveTimeoutMillis() {
return CONFIG.getPropertyValue(RestHttpTransportConnectionKeepAliveProperty.class);
}

/**
* Max timeout in ms connections are kept open when idle (requires keep-alive support).
* Specifies the maximum time to live of an HTTP connection within the HTTP connection pool. May be zero if the
* connection does not have an expiry deadline.
*/
protected long getKeepAliveTimeoutMillis(Configuration config) {
protected long getConnectionTimeToLiveMillis(Configuration config) {
return ObjectUtility.nvl(
TypeCastUtility.castValue(config.getProperty(RestClientProperties.CONNECTION_KEEP_ALIVE), Long.class),
getKeepAliveTimeoutMillis());
TypeCastUtility.castValue(config.getProperty(RestClientProperties.CONNECTION_TIME_TO_LIVE), Long.class),
CONFIG.getPropertyValue(RestHttpTransportConnectionTimeToLiveProperty.class));
}

/**
Expand All @@ -273,7 +290,7 @@ protected int getMaxConnectionsTotal(Configuration config) {

/**
* @deprecated This method will be removed with Scout release 25.1. Use the configuration property to change this
* configuration or override method {@link #getMaxConnectionsPerRoute(Configuration)}.
* configuration or override method {@link #getMaxConnectionsPerRoute(Configuration)}.
*/
@Deprecated
@SuppressWarnings("DeprecatedIsStillUsed")
Expand All @@ -293,7 +310,7 @@ protected int getMaxConnectionsPerRoute(Configuration config) {

/**
* @deprecated This method will be removed with Scout release 25.1. Use the configuration property to change this
* configuration or override method {@link #getValidateAfterInactivityMillis(Configuration)}.
* configuration or override method {@link #getValidateAfterInactivityMillis(Configuration)}.
*/
@Deprecated
@SuppressWarnings("DeprecatedIsStillUsed")
Expand Down Expand Up @@ -457,6 +474,7 @@ protected HttpUriRequestBase getUriHttpRequest(final ClientRequest clientRequest

initConnectTimeout(clientRequest, requestConfigBuilder);
initSocketTimeout(clientRequest, requestConfigBuilder);
initKeepAliveTimeout(clientRequest, requestConfigBuilder);

boolean redirectsEnabled = BooleanUtility.nvl(clientRequest.resolveProperty(RestClientProperties.FOLLOW_REDIRECTS, m_requestConfig.isRedirectsEnabled()));
requestConfigBuilder.setRedirectsEnabled(redirectsEnabled);
Expand Down Expand Up @@ -502,6 +520,18 @@ protected void initSocketTimeout(ClientRequest clientRequest, RequestConfig.Buil
}
}

protected void initKeepAliveTimeout(ClientRequest clientRequest, RequestConfig.Builder requestConfigBuilder) {
// check if keep alive timeout was set by Scout RestClientProperties on request
Long keepAliveTimeout = clientRequest.resolveProperty(RestClientProperties.CONNECTION_KEEP_ALIVE, -1L);
if (keepAliveTimeout != null && keepAliveTimeout >= 0) {
requestConfigBuilder.setDefaultKeepAlive(keepAliveTimeout, TimeUnit.MILLISECONDS);
}
else {
// use config property value as fallback
requestConfigBuilder.setDefaultKeepAlive(CONFIG.getPropertyValue(RestHttpTransportConnectionKeepAliveProperty.class), TimeUnit.MILLISECONDS);
}
}

/**
* Creates {@link HttpEntity} wrapping payload out of given {@code clientRequest}.
*/
Expand Down Expand Up @@ -803,4 +833,23 @@ public String getKey() {
return "scout.rest.client.http.validateAfterInactivity";
}
}

public static class RestHttpTransportConnectionTimeToLiveProperty extends AbstractLongConfigProperty {

@Override
public Long getDefaultValue() {
return TimeUnit.MINUTES.toMillis(30);
}

@Override
public String description() {
return "Specifies the maximum time to live of a HTTP connection within the HTTP connection pool. May be zero if the connection does not have an expiry deadline. The default value is 30 minutes.";
}

@Override
public String getKey() {
return "scout.rest.client.http.connectionTimeToLive";
}
}

}
65 changes: 57 additions & 8 deletions ....test/src/test/java/org/eclipse/scout/rt/rest/jersey/client/ScoutApacheConnectorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.Function;
Expand All @@ -44,6 +45,7 @@
import org.apache.hc.client5.http.cookie.StandardCookieSpec;
import org.apache.hc.client5.http.impl.io.PoolingHttpClientConnectionManager;
import org.apache.hc.client5.http.io.HttpClientConnectionManager;
import org.apache.hc.core5.util.TimeValue;
import org.apache.hc.core5.util.Timeout;
import org.eclipse.scout.rt.dataobject.DoEntityBuilder;
import org.eclipse.scout.rt.dataobject.IDataObjectMapper;
Expand All @@ -63,6 +65,7 @@
import org.eclipse.scout.rt.rest.jersey.RestClientTestEchoResponse;
import org.eclipse.scout.rt.rest.jersey.RestClientTestEchoServlet;
import org.eclipse.scout.rt.rest.jersey.client.ScoutApacheConnector.RestHttpTransportConnectionKeepAliveProperty;
import org.eclipse.scout.rt.rest.jersey.client.ScoutApacheConnector.RestHttpTransportConnectionTimeToLiveProperty;
import org.eclipse.scout.rt.rest.jersey.client.ScoutApacheConnector.RestHttpTransportMaxConnectionsPerRouteProperty;
import org.eclipse.scout.rt.rest.jersey.client.ScoutApacheConnector.RestHttpTransportMaxConnectionsTotalProperty;
import org.eclipse.scout.rt.rest.jersey.client.ScoutApacheConnector.RestHttpTransportValidateAfterInactivityProperty;
Expand Down Expand Up @@ -477,35 +480,35 @@ protected HttpClientConnectionManager createConnectionManager(Client client, Con
}

@Test
public void testHttpConnectionKeepAlive_defaultValue() {
public void testHttpConnectionTimeToLive_defaultValue() {
Client client = Mockito.mock(Client.class);
Configuration config = Mockito.mock(Configuration.class);
ScoutApacheConnector connector = new ScoutApacheConnector(client, config);
assertEquals(30 * 60 * 1000, connector.getKeepAliveTimeoutMillis(config));
assertEquals(30 * 60 * 1000, connector.getConnectionTimeToLiveMillis(config));
}

@Test
public void testHttpConnectionKeepAlive_byConfigProperty() {
public void testHttpConnectionTimeToLive_byConfigProperty() {
List<IBean<?>> beans = new ArrayList<>();
try {
beans.add(BEANS.get(BeanTestingHelper.class).mockConfigProperty(RestHttpTransportConnectionKeepAliveProperty.class, 100L));
beans.add(BEANS.get(BeanTestingHelper.class).mockConfigProperty(RestHttpTransportConnectionTimeToLiveProperty.class, 100L));
Client client = Mockito.mock(Client.class);
Configuration config = Mockito.mock(Configuration.class);
ScoutApacheConnector connector = new ScoutApacheConnector(client, config);
assertEquals(100L, connector.getKeepAliveTimeoutMillis(config));
assertEquals(100L, connector.getConnectionTimeToLiveMillis(config));
}
finally {
beans.forEach(BeanTestingHelper.get()::unregisterBean);
}
}

@Test
public void testHttpConnectionKeepAlive_byClientProperty() {
public void testHttpConnectionTimeToLive_byClientProperty() {
Client client = Mockito.mock(Client.class);
Configuration config = Mockito.mock(Configuration.class);
when(config.getProperty(RestClientProperties.CONNECTION_KEEP_ALIVE)).thenReturn(200L);
when(config.getProperty(RestClientProperties.CONNECTION_TIME_TO_LIVE)).thenReturn(200L);
ScoutApacheConnector connector = new ScoutApacheConnector(client, config);
assertEquals(200L, connector.getKeepAliveTimeoutMillis(config));
assertEquals(200L, connector.getConnectionTimeToLiveMillis(config));
}

@Test
Expand Down Expand Up @@ -723,4 +726,50 @@ protected void runTestSocketTimeout(Consumer<ClientRequest> clientRequestConsume
connector.initSocketTimeout(clientRequest, requestConfigBuilder);
assertEquals(expectedTimeout >= 0 ? Timeout.ofMilliseconds(expectedTimeout) : null, requestConfigBuilder.build().getResponseTimeout());
}

@Test
public void testConnectionKeepAlive_default() {
RequestConfig.Builder requestConfigBuilder = RequestConfig.custom();
assertEquals(TimeValue.of(3, TimeUnit.MINUTES), requestConfigBuilder.build().getConnectionKeepAlive());
runTestConnectionKeepAlive(c -> {
}, -1);
}

@Test
public void testConnectionKeepAlive_null() {
// null value as property set
//noinspection CodeBlock2Expr
runTestConnectionKeepAlive(clientRequest -> {
when(clientRequest.resolveProperty(eq(RestClientProperties.CONNECTION_KEEP_ALIVE), any(Long.class))).thenReturn(null);
}, -1);
}

@Test
public void testConnectionKeepAlive_scoutPropertyValue() {
// value set by Scout property
//noinspection CodeBlock2Expr
runTestConnectionKeepAlive(clientRequest -> {
when(clientRequest.resolveProperty(eq(RestClientProperties.CONNECTION_KEEP_ALIVE), any(Long.class))).thenReturn(100L);
}, 100);
}

@Test
public void testConnectionKeepAlive_invalidScoutPropertyValue() {
// invalid value set by Scout property
//noinspection CodeBlock2Expr
runTestConnectionKeepAlive(clientRequest -> {
when(clientRequest.resolveProperty(eq(RestClientProperties.CONNECTION_CLOSE), any(Long.class))).thenReturn(-42L);
}, -1);
}

protected void runTestConnectionKeepAlive(Consumer<ClientRequest> clientRequestConsumer, int expected) {
RequestConfig.Builder requestConfigBuilder = RequestConfig.custom();
Client client = Mockito.mock(Client.class);
Configuration configuration = Mockito.mock(Configuration.class);
ScoutApacheConnector connector = new ScoutApacheConnector(client, configuration);
ClientRequest clientRequest = Mockito.mock(ClientRequest.class);
clientRequestConsumer.accept(clientRequest);
connector.initKeepAliveTimeout(clientRequest, requestConfigBuilder);
assertEquals(expected >= 0 ? Timeout.ofMilliseconds(expected) : Timeout.of(BEANS.get(RestHttpTransportConnectionKeepAliveProperty.class).getDefaultValue(), TimeUnit.MILLISECONDS), requestConfigBuilder.build().getConnectionKeepAlive());
}
}
16 changes: 14 additions & 2 deletions ...se.scout.rt.rest/src/main/java/org/eclipse/scout/rt/rest/client/RestClientProperties.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ public final class RestClientProperties {
public static final String ENABLE_COOKIES = "scout.rest.client.enableCookies";

/**
* Standard cookie specification used by underlying http client. See {@code org.apache.hc.client5.http.cookie.StandardCookieSpec}
* for details.
* Standard cookie specification used by underlying http client. See
* {@code org.apache.hc.client5.http.cookie.StandardCookieSpec} for details.
* <p>
* The value MUST be an instance convertible to {@link java.lang.String}.
* </p>
Expand Down Expand Up @@ -265,6 +265,18 @@ public enum LoggerVerbosity {
*/
public static final String VALIDATE_CONNECTION_AFTER_INACTIVITY = "scout.rest.client.http.validateAfterInactivity";

/**
* Specifies the maximum time to live of an HTTP connection within the HTTP connection pool. May be zero if the
* connection does not have an expiry deadline.
* <p>
* The value MUST be an instance convertible to {@link java.lang.Long}.
* </p>
* <p>
* The default value is 30 * 60 * 1000 (e.g. 30 minutes).
* </p>
*/
public static final String CONNECTION_TIME_TO_LIVE = "scout.rest.client.http.connectionTimeToLive";

/**
* Activates OpenTelemetry metrics for HTTP client connection pool using property value as HTTP client name.
* <p>
Expand Down

0 comments on commit 737c2e5

Please sign in to comment.