From 4428fe28bfac53ea859741cec2cb04212c5a6034 Mon Sep 17 00:00:00 2001 From: Mark Payne Date: Thu, 8 Feb 2018 15:02:06 -0500 Subject: [PATCH] NIFI-4858: Expose Request Timeout from HTTP Context Map and use that to set as the timeout of the 'AsyncContext' in HandleHttpRequest. Otherwise, the request will never timeout (which is OK because the HttpContextMap will handle this). However, Jetty behind the scenes is adding a task to Scheduled Executor for each request with a delay of whatever the timeout is set to. Since it's currently set to Long.MAX_VALUE, that task will never be run and as a result the ExecutorService's queue will grow indefinitely, eventually exhausting the JVM Heap This closes #2460. Signed-off-by: Bryan Bende --- .../nifi/processors/standard/HandleHttpRequest.java | 4 +++- .../nifi/processors/standard/TestHandleHttpRequest.java | 6 ++++++ .../nifi/processors/standard/TestHandleHttpResponse.java | 6 ++++++ .../main/java/org/apache/nifi/http/HttpContextMap.java | 9 +++++++++ .../org/apache/nifi/http/StandardHttpContextMap.java | 5 +++++ 5 files changed, 29 insertions(+), 1 deletion(-) diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/HandleHttpRequest.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/HandleHttpRequest.java index d436312430..5a09dab115 100644 --- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/HandleHttpRequest.java +++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/HandleHttpRequest.java @@ -281,6 +281,8 @@ public class HandleHttpRequest extends AbstractProcessor { final String host = context.getProperty(HOSTNAME).getValue(); final int port = context.getProperty(PORT).asInteger(); final SSLContextService sslService = context.getProperty(SSL_CONTEXT).asControllerService(SSLContextService.class); + final HttpContextMap httpContextMap = context.getProperty(HTTP_CONTEXT_MAP).asControllerService(HttpContextMap.class); + final long requestTimeout = httpContextMap.getRequestTimeout(TimeUnit.MILLISECONDS); final String clientAuthValue = context.getProperty(CLIENT_AUTH).getValue(); final boolean need; @@ -404,7 +406,7 @@ public class HandleHttpRequest extends AbstractProcessor { // Right now, that information, though, is only in the ProcessSession, not the ProcessContext, // so it is not known to us. Should see if it can be added to the ProcessContext. final AsyncContext async = baseRequest.startAsync(); - async.setTimeout(Long.MAX_VALUE); // timeout is handled by HttpContextMap + async.setTimeout(requestTimeout); final boolean added = containerQueue.offer(new HttpRequestContainer(request, response, async)); if (added) { diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestHandleHttpRequest.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestHandleHttpRequest.java index 0315ba2f22..19e147e0d9 100644 --- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestHandleHttpRequest.java +++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestHandleHttpRequest.java @@ -26,6 +26,7 @@ import java.util.HashMap; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.SSLContext; @@ -304,5 +305,10 @@ public class TestHandleHttpRequest { public void setRegisterSuccessfully(boolean registerSuccessfully) { this.registerSuccessfully = registerSuccessfully; } + + @Override + public long getRequestTimeout(TimeUnit timeUnit) { + return timeUnit.convert(30000, TimeUnit.MILLISECONDS); + } } } diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestHandleHttpResponse.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestHandleHttpResponse.java index d9fc0b5ed4..d492485658 100644 --- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestHandleHttpResponse.java +++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestHandleHttpResponse.java @@ -28,6 +28,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import javax.servlet.AsyncContext; @@ -268,5 +269,10 @@ public class TestHandleHttpResponse { public int getCompletionCount() { return completedCount.get(); } + + @Override + public long getRequestTimeout(TimeUnit timeUnit) { + return timeUnit.convert(30000, TimeUnit.MILLISECONDS); + } } } diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-http-context-map-api/src/main/java/org/apache/nifi/http/HttpContextMap.java b/nifi-nar-bundles/nifi-standard-services/nifi-http-context-map-api/src/main/java/org/apache/nifi/http/HttpContextMap.java index 0dcff031cc..d050be98f2 100644 --- a/nifi-nar-bundles/nifi-standard-services/nifi-http-context-map-api/src/main/java/org/apache/nifi/http/HttpContextMap.java +++ b/nifi-nar-bundles/nifi-standard-services/nifi-http-context-map-api/src/main/java/org/apache/nifi/http/HttpContextMap.java @@ -16,6 +16,8 @@ */ package org.apache.nifi.http; +import java.util.concurrent.TimeUnit; + import javax.servlet.AsyncContext; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; @@ -66,4 +68,11 @@ public interface HttpContextMap extends ControllerService { */ void complete(String identifier); + /** + * Returns the configured timeout for HTTP Requests + * + * @param timeUnit the desired time unit + * @return the configured timeout for HTTP Requests + */ + long getRequestTimeout(TimeUnit timeUnit); } diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-http-context-map-bundle/nifi-http-context-map/src/main/java/org/apache/nifi/http/StandardHttpContextMap.java b/nifi-nar-bundles/nifi-standard-services/nifi-http-context-map-bundle/nifi-http-context-map/src/main/java/org/apache/nifi/http/StandardHttpContextMap.java index 88ce51a953..ba70e0ce66 100644 --- a/nifi-nar-bundles/nifi-standard-services/nifi-http-context-map-bundle/nifi-http-context-map/src/main/java/org/apache/nifi/http/StandardHttpContextMap.java +++ b/nifi-nar-bundles/nifi-standard-services/nifi-http-context-map-bundle/nifi-http-context-map/src/main/java/org/apache/nifi/http/StandardHttpContextMap.java @@ -194,4 +194,9 @@ public class StandardHttpContextMap extends AbstractControllerService implements } } } + + @Override + public long getRequestTimeout(final TimeUnit timeUnit) { + return timeUnit.convert(maxRequestNanos, TimeUnit.NANOSECONDS); + } }