[ 
https://issues.apache.org/jira/browse/HIVE-24786?focusedWorklogId=553949&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-553949
 ]

ASF GitHub Bot logged work on HIVE-24786:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 17/Feb/21 22:54
            Start Date: 17/Feb/21 22:54
    Worklog Time Spent: 10m 
      Work Description: t3rmin4t0r commented on a change in pull request #1983:
URL: https://github.com/apache/hive/pull/1983#discussion_r577852301



##########
File path: jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java
##########
@@ -581,21 +596,99 @@ public long getRetryInterval() {
     } else {
       httpClientBuilder = HttpClientBuilder.create();
     }
-    // In case the server's idletimeout is set to a lower value, it might 
close it's side of
-    // connection. However we retry one more time on NoHttpResponseException
+
+    // Beeline <------> LB <------> Reverse Proxy <-----> Hiveserver2
+    // In case of deployments like above, the LoadBalancer (LB) can be 
configured with Idle Timeout after which the LB
+    // will send TCP RST to Client (Beeline) and Backend (Reverse Proxy). If 
user is connected to beeline, idle for
+    // sometime and resubmits a query after the idle timeout there is a broken 
pipe between beeline and LB. When Beeline
+    // tries to submit the query one of two things happen, it either hangs or 
times out (if socketTimeout is defined in
+    // the jdbc param). The hang is because of the default infinite socket 
timeout for which there is no auto-recovery
+    // (user have to manually interrupt the query). If the socketTimeout jdbc 
param was specified, beeline will receive
+    // SocketTimeoutException (Read Timeout) or NoHttpResponseException both 
of which can be retried if maxRetries is
+    // also specified by the user (jdbc param).
+    // The following retry handler handles the above cases in addition to 
retries for idempotent and unsent requests.
     httpClientBuilder.setRetryHandler(new HttpRequestRetryHandler() {
+      // This handler is mostly a copy of DefaultHttpRequestRetryHandler 
except it also retries some exceptions
+      // which could be thrown in certain cases where idle timeout from 
intermediate proxy triggers a connection reset.
+      private final List<Class<? extends IOException>> nonRetriableClasses = 
Arrays.asList(
+              InterruptedIOException.class,
+              UnknownHostException.class,
+              ConnectException.class,
+              SSLException.class);
+      // socket exceptions could happen because of timeout, broken pipe or 
server not responding in which case it is
+      // better to reopen the connection and retry if user specified maxRetries
+      private final List<Class<? extends IOException>> retriableClasses = 
Arrays.asList(
+              SocketTimeoutException.class,
+              SocketException.class,
+              NoHttpResponseException.class
+      );
+
       @Override
       public boolean retryRequest(IOException exception, int executionCount, 
HttpContext context) {
-        if (executionCount > 1) {
-          LOG.info("Retry attempts to connect to server exceeded.");
+        Args.notNull(exception, "Exception parameter");
+        Args.notNull(context, "HTTP context");
+        if (executionCount > maxRetries) {
+          // Do not retry if over max retry count
+          LOG.error("Max retries (" + maxRetries + ") exhausted.", exception);
+          return false;
+        }
+        if (this.retriableClasses.contains(exception.getClass())) {
+          LOG.info("Retrying " + exception.getClass() + " as it is in 
retriable classes list.");
+          return true;
+        }
+        if (this.nonRetriableClasses.contains(exception.getClass())) {
+          LOG.info("Not retrying as the class (" + exception.getClass() + ") 
is non-retriable class.");
+          return false;
+        } else {
+          for (final Class<? extends IOException> rejectException : 
this.nonRetriableClasses) {
+            if (rejectException.isInstance(exception)) {
+              LOG.info("Not retrying as the class (" + exception.getClass() + 
") is an instance of is non-retriable class.");;
+              return false;
+            }
+          }
+        }
+        final HttpClientContext clientContext = 
HttpClientContext.adapt(context);
+        final HttpRequest request = clientContext.getRequest();
+
+        if(requestIsAborted(request)){
+          LOG.info("Not retrying as request is aborted.");
           return false;
         }
-        if (exception instanceof org.apache.http.NoHttpResponseException) {
-          LOG.info("Could not connect to the server. Retrying one more time.");
+
+        if (handleAsIdempotent(request)) {
+          LOG.info("Retrying idempotent request. Attempt " + executionCount + 
" of " + maxRetries);
+          // Retry if the request is considered idempotent
+          return true;
+        }
+
+        if (!clientContext.isRequestSent()) {
+          LOG.info("Retrying unsent request. Attempt " + executionCount + " of 
" + maxRetries);
+          // Retry if the request has not been sent fully or
+          // if it's OK to retry methods that have been sent
           return true;
         }
+
+        LOG.info("Not retrying as the request is not idempotent or is already 
sent.");
+        // otherwise do not retry
         return false;
       }
+
+      // requests that handles "Expect continue" handshakes. If server 
received the header and is waiting for body
+      // then those requests can be retried. Most basic http method methods 
except DELETE are idempotent as long as they

Review comment:
       Specifically, a "POST" body we haven't sent yet hasn't left any unclean 
state on the server side (in case of a network error on response). 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 553949)
    Time Spent: 0.5h  (was: 20m)

> JDBC HttpClient should retry for idempotent and unsent http methods
> -------------------------------------------------------------------
>
>                 Key: HIVE-24786
>                 URL: https://issues.apache.org/jira/browse/HIVE-24786
>             Project: Hive
>          Issue Type: Bug
>    Affects Versions: 4.0.0
>            Reporter: Prasanth Jayachandran
>            Assignee: Prasanth Jayachandran
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> When hiveserver2 is behind multiple proxies there is possibility of "broken 
> pipe", "connect timeout" and "read timeout" exceptions if one of the 
> intermediate proxies or load balancers decided to reset the underlying tcp 
> socket after idle timeout. When the connection is broken and when the query 
> is submitted after idle timeout from beeline (or client) perspective the 
> connection is open but http methods (POST/GET) fails with socket related 
> exceptions. Since these methods are not sent to the server these are safe for 
> client side retries. 
>  
> Also HIVE-12371 seems to apply the socket timeout only to binary transport. 
> Same can be passed on to http client as well to avoid retry hang issues with 
> infinite timeouts. 



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to