helix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject [helix] branch master updated: Add timeout for stoppable post request (#1013)
Date Fri, 22 May 2020 22:19:43 GMT
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git


The following commit(s) were added to refs/heads/master by this push:
     new 0a8dcf4  Add timeout for stoppable post request (#1013)
0a8dcf4 is described below

commit 0a8dcf4b54c805c006427a69244fb3652b41e24d
Author: Ali Reza Zamani Zadeh Najari <anajari@linkedin.com>
AuthorDate: Fri May 22 15:19:36 2020 -0700

    Add timeout for stoppable post request (#1013)
    
    Add timeout for stoppable post request
    
    In this commit, a http timeout has been added for the POST requests
    which is mainly used for REST's stoppable check.
---
 .../helix/rest/client/CustomRestClientFactory.java | 18 ++++++++++++++---
 .../apache/helix/rest/common/HttpConstants.java    |  1 +
 .../helix/rest/common/RestSystemPropertyKeys.java  |  6 ++++++
 .../helix/rest/client/TestCustomRestClient.java    | 23 ++++++++++++++++++++++
 4 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/helix-rest/src/main/java/org/apache/helix/rest/client/CustomRestClientFactory.java
b/helix-rest/src/main/java/org/apache/helix/rest/client/CustomRestClientFactory.java
index 40bc6f4..2b54ed8 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/client/CustomRestClientFactory.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/client/CustomRestClientFactory.java
@@ -19,10 +19,15 @@ package org.apache.helix.rest.client;
  * under the License.
  */
 
+import org.apache.helix.rest.common.HttpConstants;
+import org.apache.helix.rest.common.RestSystemPropertyKeys;
 import org.apache.helix.rest.server.HelixRestServer;
+import org.apache.helix.util.HelixUtil;
 import org.apache.http.client.HttpClient;
+import org.apache.http.client.config.RequestConfig;
 import org.apache.http.conn.ssl.NoopHostnameVerifier;
 import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
+import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.impl.client.HttpClients;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -32,9 +37,13 @@ import org.slf4j.LoggerFactory;
  */
 public class CustomRestClientFactory {
   private static final Logger LOG = LoggerFactory.getLogger(CustomRestClientFactory.class);
-
   private static CustomRestClient INSTANCE = null;
 
+  // Here int has been used for timeout value because setConnectTimeout,
+  // setConnectionRequestTimeout and setSocketTimeout are getting int as input
+  private static final int HTTP_REQUEST_TIMEOUT = HelixUtil.getSystemPropertyAsInt(
+      RestSystemPropertyKeys.REST_HTTP_TIMEOUT_MS, HttpConstants.DEFAULT_HTTP_REQUEST_TIMEOUT);
+
   private CustomRestClientFactory() {
   }
 
@@ -44,14 +53,17 @@ public class CustomRestClientFactory {
         if (INSTANCE == null) {
           try {
             HttpClient httpClient;
+            RequestConfig config = RequestConfig.custom().setConnectTimeout(HTTP_REQUEST_TIMEOUT)
+                .setConnectionRequestTimeout(HTTP_REQUEST_TIMEOUT)
+                .setSocketTimeout(HTTP_REQUEST_TIMEOUT).build();
             if (HelixRestServer.REST_SERVER_SSL_CONTEXT != null) {
               httpClient =
                   HttpClients.custom().setSSLContext(HelixRestServer.REST_SERVER_SSL_CONTEXT)
                       .setSSLSocketFactory(new SSLConnectionSocketFactory(
                           HelixRestServer.REST_SERVER_SSL_CONTEXT, new NoopHostnameVerifier()))
-                      .build();
+                      .setDefaultRequestConfig(config).build();
             } else {
-              httpClient = HttpClients.createDefault();
+              httpClient = HttpClientBuilder.create().setDefaultRequestConfig(config).build();
             }
             INSTANCE = new CustomRestClientImpl(httpClient);
             return INSTANCE;
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/common/HttpConstants.java b/helix-rest/src/main/java/org/apache/helix/rest/common/HttpConstants.java
index d5f3bd9..d6eec6d 100644
--- a/helix-rest/src/main/java/org/apache/helix/rest/common/HttpConstants.java
+++ b/helix-rest/src/main/java/org/apache/helix/rest/common/HttpConstants.java
@@ -28,4 +28,5 @@ public class HttpConstants {
   }
 
   public static final String HTTP_PROTOCOL_PREFIX = "http://";
+  public static final int DEFAULT_HTTP_REQUEST_TIMEOUT = 60 * 1000;
 }
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/common/RestSystemPropertyKeys.java
b/helix-rest/src/main/java/org/apache/helix/rest/common/RestSystemPropertyKeys.java
new file mode 100644
index 0000000..5478dcb
--- /dev/null
+++ b/helix-rest/src/main/java/org/apache/helix/rest/common/RestSystemPropertyKeys.java
@@ -0,0 +1,6 @@
+package org.apache.helix.rest.common;
+
+public class RestSystemPropertyKeys {
+  // System property for REST HTTP request timeout
+  public static final String REST_HTTP_TIMEOUT_MS = "rest.http.timeout.ms";
+}
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/client/TestCustomRestClient.java
b/helix-rest/src/test/java/org/apache/helix/rest/client/TestCustomRestClient.java
index adad6c6..6d6831d 100644
--- a/helix-rest/src/test/java/org/apache/helix/rest/client/TestCustomRestClient.java
+++ b/helix-rest/src/test/java/org/apache/helix/rest/client/TestCustomRestClient.java
@@ -13,6 +13,7 @@ import org.apache.http.StatusLine;
 import org.apache.http.client.ClientProtocolException;
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpPost;
+import org.apache.http.conn.ConnectTimeoutException;
 import org.apache.http.impl.client.HttpClients;
 import org.junit.Assert;
 import org.mockito.Mock;
@@ -145,6 +146,28 @@ public class TestCustomRestClient {
     Assert.assertEquals(json.get("data").asText(), "{}");
   }
 
+  @Test
+  public void testGetPartitionStoppableCheckWhenTimeout() throws IOException {
+    MockCustomRestClient customRestClient = new MockCustomRestClient(_httpClient);
+
+    HttpResponse httpResponse = mock(HttpResponse.class);
+    StatusLine statusLine = mock(StatusLine.class);
+
+    when(statusLine.getStatusCode()).thenReturn(HttpStatus.SC_OK);
+    when(httpResponse.getStatusLine()).thenReturn(statusLine);
+    when(_httpClient.execute(any(HttpPost.class)))
+        .thenThrow(new ConnectTimeoutException("Timeout Exception Happened!"));
+
+    boolean timeoutExceptionHappened = false;
+    try {
+      customRestClient.getPartitionStoppableCheck(HTTP_LOCALHOST, ImmutableList.of("db0",
"db1"),
+          Collections.emptyMap());
+    } catch (ConnectTimeoutException e) {
+      timeoutExceptionHappened = true;
+    }
+    Assert.assertTrue(timeoutExceptionHappened);
+  }
+
   private class MockCustomRestClient extends CustomRestClientImpl {
     private String _jsonResponse = "";
 


Mime
View raw message