From 57357fb21f7bb4f733efbb2da81243bbd3cb0ca4 Mon Sep 17 00:00:00 2001 From: sebr72 Date: Fri, 24 Jan 2025 23:51:00 +0100 Subject: [PATCH] Use more appropriate http timeouts --- .../http/MfClientHttpRequestFactoryImpl.java | 61 ++++++++----------- .../mapfish-spring-application-context.xml | 4 +- .../main/resources/mapfish-spring.properties | 16 ++++- .../mapfish/print/TestHttpClientFactory.java | 3 +- .../MfClientHttpRequestFactoryImplTest.java | 3 +- ...-bug-fix-processor-application-context.xml | 7 ++- .../org/mapfish/print/AbstractApiTest.java | 3 +- 7 files changed, 49 insertions(+), 48 deletions(-) diff --git a/core/src/main/java/org/mapfish/print/http/MfClientHttpRequestFactoryImpl.java b/core/src/main/java/org/mapfish/print/http/MfClientHttpRequestFactoryImpl.java index 0cbafd288f..8d88a68bb6 100644 --- a/core/src/main/java/org/mapfish/print/http/MfClientHttpRequestFactoryImpl.java +++ b/core/src/main/java/org/mapfish/print/http/MfClientHttpRequestFactoryImpl.java @@ -12,7 +12,6 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -32,7 +31,6 @@ import org.apache.http.protocol.HTTP; import org.apache.http.protocol.HttpContext; import org.mapfish.print.config.Configuration; -import org.mapfish.print.servlet.job.impl.ThreadPoolJobManager; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.http.HttpHeaders; @@ -53,12 +51,25 @@ public class MfClientHttpRequestFactoryImpl extends HttpComponentsClientHttpRequ * * @param maxConnTotal Maximum total connections. * @param maxConnPerRoute Maximum connections per route. + * @param connectionRequestTimeout Number of milliseconds used when requesting a connection from + * the connection manager. + * @param connectTimeout Number of milliseconds until a connection is established. + * @param socketTimeout Maximum number of milliseconds during which a socket remains inactive + * between two consecutive data packets. */ public MfClientHttpRequestFactoryImpl( final int maxConnTotal, final int maxConnPerRoute, - final ThreadPoolJobManager threadPoolJobManager) { - super(createHttpClient(maxConnTotal, maxConnPerRoute, threadPoolJobManager)); + final int connectionRequestTimeout, + final int connectTimeout, + final int socketTimeout) { + super( + createHttpClient( + maxConnTotal, + maxConnPerRoute, + connectionRequestTimeout, + connectTimeout, + socketTimeout)); } @Nullable @@ -66,41 +77,17 @@ static Configuration getCurrentConfiguration() { return CURRENT_CONFIGURATION.get(); } - /** - * Return the number of milliseconds until the timeout Use the Automatic cancellation timeout if - * not defined. - * - * @param name timeout idemtifier - * @return the number of milliseconds until the timeout - */ - private static int getTimeoutValue( - final String name, final ThreadPoolJobManager threadPoolJobManager) { - final String value = System.getProperty(name); - if (value == null) { - long millis = TimeUnit.SECONDS.toMillis(threadPoolJobManager.getTimeout()); - if (millis > Integer.MAX_VALUE) { - LOGGER.warn( - "The value of {} is too large. The timeout will be set to the maximum value of {}", - name, - Integer.MAX_VALUE); - return Integer.MAX_VALUE; - } else { - return Integer.parseInt(Long.toString(millis)); - } - } - return Integer.parseInt(value); - } - private static CloseableHttpClient createHttpClient( final int maxConnTotal, final int maxConnPerRoute, - final ThreadPoolJobManager threadPoolJobManager) { + final int connectionRequestTimeout, + final int connectTimeout, + final int socketTimeout) { final RequestConfig requestConfig = RequestConfig.custom() - .setConnectionRequestTimeout( - getTimeoutValue("http.connectionRequestTimeout", threadPoolJobManager)) - .setConnectTimeout(getTimeoutValue("http.connectTimeout", threadPoolJobManager)) - .setSocketTimeout(getTimeoutValue("http.socketTimeout", threadPoolJobManager)) + .setConnectionRequestTimeout(connectionRequestTimeout) + .setConnectTimeout(connectTimeout) + .setSocketTimeout(socketTimeout) .build(); final HttpClientBuilder httpClientBuilder = @@ -118,9 +105,9 @@ private static CloseableHttpClient createHttpClient( LOGGER.debug( "Created CloseableHttpClient using connectionRequestTimeout: {} connectTimeout: {}" + " socketTimeout: {}", - getTimeoutValue("http.connectionRequestTimeout", threadPoolJobManager), - getTimeoutValue("http.connectTimeout", threadPoolJobManager), - getTimeoutValue("http.socketTimeout", threadPoolJobManager)); + connectionRequestTimeout, + connectTimeout, + socketTimeout); return closeableHttpClient; } diff --git a/core/src/main/resources/mapfish-spring-application-context.xml b/core/src/main/resources/mapfish-spring-application-context.xml index 2a42ddc332..fe9e5ac333 100644 --- a/core/src/main/resources/mapfish-spring-application-context.xml +++ b/core/src/main/resources/mapfish-spring-application-context.xml @@ -63,7 +63,9 @@ - + + + diff --git a/core/src/main/resources/mapfish-spring.properties b/core/src/main/resources/mapfish-spring.properties index 9bc9b61347..ee46ea2aef 100644 --- a/core/src/main/resources/mapfish-spring.properties +++ b/core/src/main/resources/mapfish-spring.properties @@ -56,5 +56,19 @@ httpRequest.fetchRetry.intervalMillis=100 # Amount of time in the past where we check if a print job was executed by this server healthStatus.expectedMaxTime.sinceLastPrint.InSeconds=300 -# Maximum number of Print Jobs queued before raising it i +# Maximum number of Print Jobs queued before raising it is an issue healthStatus.unhealthyThreshold.maxNbrPrintJobQueued=4 + +# Number of milliseconds used when requesting a connection from the connection manager. +# Recommended 2s for interactive application +# Recommended 10s for batch application +http.connectionRequestTimeout=10000 + +# Number of milliseconds until a connection is established. +# Recommended 5s for applications in general +# Recommended 10s for tolerant application with slow connection +http.connectTimeout=10000 + +# Maximum number of milliseconds during which a socket remains inactive between two consecutive data packets. +# Using 5 minutes by default to support very slow and large downloads +http.socketTimeout=300000 diff --git a/core/src/test/java/org/mapfish/print/TestHttpClientFactory.java b/core/src/test/java/org/mapfish/print/TestHttpClientFactory.java index db8cf6e245..7c327f0cc1 100644 --- a/core/src/test/java/org/mapfish/print/TestHttpClientFactory.java +++ b/core/src/test/java/org/mapfish/print/TestHttpClientFactory.java @@ -12,7 +12,6 @@ import org.mapfish.print.http.ConfigurableRequest; import org.mapfish.print.http.MfClientHttpRequestFactory; import org.mapfish.print.http.MfClientHttpRequestFactoryImpl; -import org.mapfish.print.servlet.job.impl.ThreadPoolJobManager; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; import org.springframework.http.HttpStatus; @@ -27,7 +26,7 @@ public class TestHttpClientFactory extends MfClientHttpRequestFactoryImpl private final Map, Handler> handlers = new ConcurrentHashMap<>(); public TestHttpClientFactory() { - super(20, 10, new ThreadPoolJobManager()); + super(20, 10, 1000, 1000, 1000); } public void registerHandler(Predicate matcher, Handler handler) { diff --git a/core/src/test/java/org/mapfish/print/http/MfClientHttpRequestFactoryImplTest.java b/core/src/test/java/org/mapfish/print/http/MfClientHttpRequestFactoryImplTest.java index bfadcb69d7..dc90f34077 100644 --- a/core/src/test/java/org/mapfish/print/http/MfClientHttpRequestFactoryImplTest.java +++ b/core/src/test/java/org/mapfish/print/http/MfClientHttpRequestFactoryImplTest.java @@ -9,7 +9,6 @@ import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; -import org.mapfish.print.servlet.job.impl.ThreadPoolJobManager; import org.springframework.http.HttpMethod; import org.springframework.http.client.ClientHttpResponse; @@ -41,7 +40,7 @@ public void testGetHeaders() throws Exception { }); MfClientHttpRequestFactoryImpl factory = - new MfClientHttpRequestFactoryImpl(20, 10, new ThreadPoolJobManager()); + new MfClientHttpRequestFactoryImpl(20, 10, 1000, 1000, 1000); final ConfigurableRequest request = factory.createRequest( new URI("http://" + HttpProxyTest.LOCALHOST + ":" + TARGET_PORT + "/request"), diff --git a/core/src/test/resources/org/mapfish/print/processor/http/map-uri/map-uri-228-bug-fix-processor-application-context.xml b/core/src/test/resources/org/mapfish/print/processor/http/map-uri/map-uri-228-bug-fix-processor-application-context.xml index a4caf87f91..59ffcf4d42 100644 --- a/core/src/test/resources/org/mapfish/print/processor/http/map-uri/map-uri-228-bug-fix-processor-application-context.xml +++ b/core/src/test/resources/org/mapfish/print/processor/http/map-uri/map-uri-228-bug-fix-processor-application-context.xml @@ -1,13 +1,14 @@ + http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd"> + + + diff --git a/examples/src/test/java/org/mapfish/print/AbstractApiTest.java b/examples/src/test/java/org/mapfish/print/AbstractApiTest.java index 558ee4b5d0..03838f63b3 100644 --- a/examples/src/test/java/org/mapfish/print/AbstractApiTest.java +++ b/examples/src/test/java/org/mapfish/print/AbstractApiTest.java @@ -10,7 +10,6 @@ import java.util.Map; import org.apache.commons.io.IOUtils; import org.mapfish.print.http.MfClientHttpRequestFactoryImpl; -import org.mapfish.print.servlet.job.impl.ThreadPoolJobManager; import org.springframework.http.HttpMethod; import org.springframework.http.MediaType; import org.springframework.http.client.ClientHttpRequest; @@ -22,7 +21,7 @@ public abstract class AbstractApiTest { protected static final String PRINT_SERVER = "http://print:8080/"; protected ClientHttpRequestFactory httpRequestFactory = - new MfClientHttpRequestFactoryImpl(10, 10, new ThreadPoolJobManager()); + new MfClientHttpRequestFactoryImpl(20, 10, 1000, 1000, 1000); protected ClientHttpRequest getRequest(String path, HttpMethod method) throws IOException, URISyntaxException {