From 098e12e21e758aca7664a8afda144c16448fc58b Mon Sep 17 00:00:00 2001 From: Mark Robert Miller Date: Thu, 7 Jan 2016 19:43:09 +0000 Subject: [PATCH] SOLR-8450: Our HttpClient retry policy is too permissive. git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1723616 13f79535-47bb-0310-9956-ffa450edef68 --- .../client/solrj/impl/HttpClientUtil.java | 7 +- .../impl/SolrHttpRequestRetryHandler.java | 157 ++++++++++++++++++ 2 files changed, 158 insertions(+), 6 deletions(-) create mode 100644 solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrHttpRequestRetryHandler.java diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java index 9f68754969d..2ad89757e04 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpClientUtil.java @@ -263,12 +263,7 @@ public class HttpClientUtil { } else { // if the request is not fully sent, we retry // streaming updates are not a problem, because they are not retryable - httpClient.setHttpRequestRetryHandler(new DefaultHttpRequestRetryHandler(){ - @Override - protected boolean handleAsIdempotent(final HttpRequest request) { - return false; // we can't tell if a Solr request is idempotent - } - }); + httpClient.setHttpRequestRetryHandler(new SolrHttpRequestRetryHandler(3)); } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrHttpRequestRetryHandler.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrHttpRequestRetryHandler.java new file mode 100644 index 00000000000..a9307cfbb9d --- /dev/null +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrHttpRequestRetryHandler.java @@ -0,0 +1,157 @@ +package org.apache.solr.client.solrj.impl; + +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.io.IOException; +import java.io.InterruptedIOException; +import java.lang.invoke.MethodHandles; +import java.net.ConnectException; +import java.net.UnknownHostException; +import java.util.Arrays; +import java.util.Collection; +import java.util.HashSet; +import java.util.Set; + +import javax.net.ssl.SSLException; + +import org.apache.http.HttpRequest; +import org.apache.http.client.HttpRequestRetryHandler; +import org.apache.http.client.methods.HttpUriRequest; +import org.apache.http.client.protocol.HttpClientContext; +import org.apache.http.impl.client.RequestWrapper; +import org.apache.http.protocol.HttpContext; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public class SolrHttpRequestRetryHandler implements HttpRequestRetryHandler { + + private static final String GET = "GET"; + + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); + + public static final SolrHttpRequestRetryHandler INSTANCE = new SolrHttpRequestRetryHandler(); + + /** the number of times a method will be retried */ + private final int retryCount; + + private final Set> nonRetriableClasses; + + /** + * Create the request retry handler using the specified IOException classes + * + * @param retryCount + * how many times to retry; 0 means no retries + * true if it's OK to retry requests that have been sent + * @param clazzes + * the IOException types that should not be retried + */ + protected SolrHttpRequestRetryHandler(final int retryCount, final Collection> clazzes) { + super(); + this.retryCount = retryCount; + this.nonRetriableClasses = new HashSet>(); + for (final Class clazz : clazzes) { + this.nonRetriableClasses.add(clazz); + } + } + + /** + * Create the request retry handler using the following list of non-retriable IOException classes:
+ *
    + *
  • InterruptedIOException
    • + *
  • UnknownHostException
    • + *
  • ConnectException
    • + *
  • SSLException
    • + *
+ * + * @param retryCount + * how many times to retry; 0 means no retries + * true if it's OK to retry non-idempotent requests that have been sent + */ + @SuppressWarnings("unchecked") + public SolrHttpRequestRetryHandler(final int retryCount) { + this(retryCount, Arrays.asList(InterruptedIOException.class, UnknownHostException.class, + ConnectException.class, SSLException.class)); + } + + /** + * Create the request retry handler with a retry count of 3, requestSentRetryEnabled false and using the following + * list of non-retriable IOException classes:
+ *
    + *
  • InterruptedIOException
    • + *
  • UnknownHostException
    • + *
  • ConnectException
    • + *
  • SSLException
    • + *
+ */ + public SolrHttpRequestRetryHandler() { + this(3); + } + + @Override + public boolean retryRequest(final IOException exception, final int executionCount, final HttpContext context) { + log.debug("Retry http request {} out of {}", executionCount, this.retryCount); + if (executionCount > this.retryCount) { + log.debug("Do not retry, over max retry count"); + return false; + } + if (this.nonRetriableClasses.contains(exception.getClass())) { + log.debug("Do not retry, non retriable class {}" + exception.getClass()); + return false; + } else { + for (final Class rejectException : this.nonRetriableClasses) { + if (rejectException.isInstance(exception)) { + log.debug("Do not retry, non retriable class {}" + exception.getClass()); + return false; + } + } + } + final HttpClientContext clientContext = HttpClientContext.adapt(context); + final HttpRequest request = clientContext.getRequest(); + + if (requestIsAborted(request)) { + log.debug("Do not retry, request was aborted"); + return false; + } + + if (handleAsIdempotent(clientContext)) { + log.debug("Retry, request should be idempotent"); + return true; + } + + log.debug("Do not retry, no rules matched"); + return false; + } + + public int getRetryCount() { + return retryCount; + } + + protected boolean handleAsIdempotent(final HttpClientContext context) { + String method = context.getRequest().getRequestLine().getMethod(); + return method.equals(GET); + } + + protected boolean requestIsAborted(final HttpRequest request) { + HttpRequest req = request; + if (request instanceof RequestWrapper) { // does not forward request to original + req = ((RequestWrapper) request).getOriginal(); + } + return (req instanceof HttpUriRequest && ((HttpUriRequest) req).isAborted()); + } + +} \ No newline at end of file