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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
