From d6932c16217de82f9af7a3b0bba73ec3bf233728 Mon Sep 17 00:00:00 2001
From: Akash Dwivedi <akash.exe@gmail.com>
Date: Wed, 17 Jan 2018 13:54:18 -0800
Subject: [PATCH] java-util version update + Add UnusedConnectionTimeout
 config. (#5239)

* java-util version update + Add UnusedConnectionTimeout config.

* warn  if unusedConnectionTime >= readTimeout.

* Doc update + addressed comment.

* Use compareTo  to compare duration.

* remove unused variable.

* addressed comments and default for unusedConnectionTimeout.
---
 docs/content/configuration/broker.md          |  1 +
 pom.xml                                       |  2 +-
 .../guice/http/DruidHttpClientConfig.java     | 19 +++++++++++++++++++
 .../io/druid/guice/http/HttpClientModule.java |  3 ++-
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/docs/content/configuration/broker.md b/docs/content/configuration/broker.md
index 6731817cb03..edddca824db 100644
--- a/docs/content/configuration/broker.md
+++ b/docs/content/configuration/broker.md
@@ -44,6 +44,7 @@ Druid uses Jetty to serve HTTP requests.
 |`druid.broker.http.numConnections`|Size of connection pool for the Broker to connect to historical and real-time processes. If there are more queries than this number that all need to speak to the same node, then they will queue up.|20|
 |`druid.broker.http.compressionCodec`|Compression codec the Broker uses to communicate with historical and real-time processes. May be "gzip" or "identity".|gzip|
 |`druid.broker.http.readTimeout`|The timeout for data reads from historical and real-time processes.|PT15M|
+|`druid.broker.http.unusedConnectionTimeout`|The timeout for idle connections in connection pool. This timeout should be less than `druid.broker.http.readTimeout`. Set this timeout = ~90% of `druid.broker.http.readTimeout`|`PT4M`|
 |`druid.server.http.maxQueryTimeout`|Maximum allowed value (in milliseconds) for `timeout` parameter. See [query-context](query-context.html) to know more about `timeout`. Query is rejected if the query context `timeout` is greater than this value. |Long.MAX_VALUE|
 |`druid.server.http.maxRequestHeaderSize`|Maximum size of a request header in bytes. Larger headers consume more memory and can make a server more vulnerable to denial of service attacks. |8 * 1024|
 
diff --git a/pom.xml b/pom.xml
index a68da8ea929..41233b5e42c 100644
--- a/pom.xml
+++ b/pom.xml
@@ -151,7 +151,7 @@
             <dependency>
                 <groupId>com.metamx</groupId>
                 <artifactId>java-util</artifactId>
-                <version>1.3.2</version>
+                <version>1.3.3</version>
             </dependency>
             <dependency>
                 <groupId>commons-codec</groupId>
diff --git a/server/src/main/java/io/druid/guice/http/DruidHttpClientConfig.java b/server/src/main/java/io/druid/guice/http/DruidHttpClientConfig.java
index 4d6dbce748e..8588b5da33f 100644
--- a/server/src/main/java/io/druid/guice/http/DruidHttpClientConfig.java
+++ b/server/src/main/java/io/druid/guice/http/DruidHttpClientConfig.java
@@ -20,6 +20,7 @@
 package io.druid.guice.http;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
+import io.druid.java.util.common.logger.Logger;
 import org.joda.time.Duration;
 import org.joda.time.Period;
 
@@ -31,6 +32,7 @@ import javax.validation.constraints.Min;
 public class DruidHttpClientConfig
 {
   private final String DEFAULT_COMPRESSION_CODEC = "gzip";
+  private static final Logger LOG = new Logger(DruidHttpClientConfig.class);
 
   @JsonProperty
   @Min(0)
@@ -53,6 +55,9 @@ public class DruidHttpClientConfig
   @JsonProperty
   private int requestBuffersize = 8 * 1024;
 
+  @JsonProperty
+  private Period unusedConnectionTimeout = new Period("PT4M");
+
   public int getNumConnections()
   {
     return numConnections;
@@ -82,4 +87,18 @@ public class DruidHttpClientConfig
   {
     return requestBuffersize;
   }
+
+  public Duration getUnusedConnectionTimeout()
+  {
+    if (unusedConnectionTimeout != null && readTimeout != null
+        && unusedConnectionTimeout.toStandardDuration().compareTo(readTimeout.toStandardDuration()) >= 0) {
+      LOG.warn(
+          "Ohh no! UnusedConnectionTimeout[%s] is longer than readTimeout[%s], please correct"
+          + " the configuration, this might not be supported in future.",
+          unusedConnectionTimeout,
+          readTimeout
+      );
+    }
+    return unusedConnectionTimeout == null ? null : unusedConnectionTimeout.toStandardDuration();
+  }
 }
diff --git a/server/src/main/java/io/druid/guice/http/HttpClientModule.java b/server/src/main/java/io/druid/guice/http/HttpClientModule.java
index 73b06163c0c..92c1e72712d 100644
--- a/server/src/main/java/io/druid/guice/http/HttpClientModule.java
+++ b/server/src/main/java/io/druid/guice/http/HttpClientModule.java
@@ -142,7 +142,8 @@ public class HttpClientModule implements Module
           .withWorkerCount(config.getNumMaxThreads())
           .withCompressionCodec(
               HttpClientConfig.CompressionCodec.valueOf(StringUtils.toUpperCase(config.getCompressionCodec()))
-          );
+          )
+          .withUnusedConnectionTimeoutDuration(config.getUnusedConnectionTimeout());
 
       if (getSslContextBinding() != null) {
         builder.withSslContext(getSslContextBinding().getProvider().get());