KNOX-2912 - Don't fail over non idempotent requests unless it's a connect exception (#757)
diff --git a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/ConfigurableHADispatch.java b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/ConfigurableHADispatch.java
index 826275b..8d5f5b8 100644
--- a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/ConfigurableHADispatch.java
+++ b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/ConfigurableHADispatch.java
@@ -17,6 +17,8 @@
*/
package org.apache.knox.gateway.ha.dispatch;
+import static org.apache.knox.gateway.util.HttpUtils.isConnectionError;
+
import org.apache.commons.codec.digest.DigestUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.http.HttpResponse;
@@ -216,8 +218,8 @@
inboundResponse = executeOutboundRequest(outboundRequest);
writeOutboundResponse(outboundRequest, inboundRequest, outboundResponse, inboundResponse);
} catch ( IOException e ) {
- /* if non-idempotent requests are not allowed to failover */
- if(!failoverNonIdempotentRequestEnabled && nonIdempotentRequests.stream().anyMatch(outboundRequest.getMethod()::equalsIgnoreCase)) {
+ /* if non-idempotent requests are not allowed to failover, unless it's a connection error */
+ if(!isConnectionError(e.getCause()) && isNonIdempotentAndNonIdempotentFailoverDisabled(outboundRequest)) {
LOG.cannotFailoverNonIdempotentRequest(outboundRequest.getMethod(), e.toString());
/* mark endpoint as failed */
markEndpointFailed(outboundRequest, inboundRequest);
@@ -229,6 +231,10 @@
}
}
+ private boolean isNonIdempotentAndNonIdempotentFailoverDisabled(HttpUriRequest outboundRequest) {
+ return !failoverNonIdempotentRequestEnabled && nonIdempotentRequests.stream().anyMatch(outboundRequest.getMethod()::equalsIgnoreCase);
+ }
+
private Optional<URI> setBackendfromHaCookie(HttpUriRequest outboundRequest, HttpServletRequest inboundRequest) {
if (loadBalancingEnabled && stickySessionsEnabled && inboundRequest.getCookies() != null) {
for (Cookie cookie : inboundRequest.getCookies()) {
diff --git a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/i18n/HaDispatchMessages.java b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/i18n/HaDispatchMessages.java
index 004af14..901d87b 100644
--- a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/i18n/HaDispatchMessages.java
+++ b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/i18n/HaDispatchMessages.java
@@ -54,6 +54,6 @@
@Message(level = MessageLevel.ERROR, text = "Unsupported encoding, cause: {0}")
void unsupportedEncodingException(String cause);
- @Message(level = MessageLevel.ERROR, text = "Request is non-idempotent {0}, failover prevented, to allow non-idempotent requests to failover set 'failoverNonIdempotentRequestEnabled=true' in HA config. Cause {1}")
+ @Message(level = MessageLevel.ERROR, text = "Request is non-idempotent {0}, failover prevented, to allow non-idempotent requests to failover set 'failoverNonIdempotentRequestEnabled=true' in HA config. Non connection related error: {1}")
void cannotFailoverNonIdempotentRequest(String method, String cause);
}
diff --git a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultDispatch.java b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultDispatch.java
index 066a842..58f0694 100644
--- a/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultDispatch.java
+++ b/gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultDispatch.java
@@ -181,7 +181,7 @@
// We do not want to expose back end host. port end points to clients, see JIRA KNOX-58
auditor.audit( Action.DISPATCH, outboundRequest.getURI().toString(), ResourceType.URI, ActionOutcome.FAILURE );
LOG.dispatchServiceConnectionException( outboundRequest.getURI(), e );
- throw new IOException( RES.dispatchConnectionError() );
+ throw new IOException(RES.dispatchConnectionError(), e);
}
return inboundResponse;
}
diff --git a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/HttpUtils.java b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/HttpUtils.java
index ac96214..46d36f5 100644
--- a/gateway-util-common/src/main/java/org/apache/knox/gateway/util/HttpUtils.java
+++ b/gateway-util-common/src/main/java/org/apache/knox/gateway/util/HttpUtils.java
@@ -17,8 +17,14 @@
*/
package org.apache.knox.gateway.util;
+import static java.util.Arrays.asList;
+
+import java.io.IOException;
import java.io.UnsupportedEncodingException;
+import java.net.NoRouteToHostException;
+import java.net.SocketException;
import java.net.URLDecoder;
+import java.net.UnknownHostException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
@@ -28,6 +34,9 @@
import java.util.StringTokenizer;
public class HttpUtils {
+ private static final List<Class<? extends IOException>> connectionErrors = asList(UnknownHostException.class, NoRouteToHostException.class,
+ SocketException.class);
+
public static Map<String, List<String>> splitQuery(String queryString)
throws UnsupportedEncodingException {
final Map<String, List<String>> queryPairs = new LinkedHashMap<>();
@@ -117,4 +126,22 @@
}
map.put( name, values );
}
+
+ /**
+ * Determine if the specified exception represents an error condition that is related to a connection error.
+ *
+ * @return true, if the exception represents an connection error; otherwise, false.
+ */
+ public static boolean isConnectionError(Throwable exception) {
+ boolean isFailoverException = false;
+ if (exception != null) {
+ for (Class<? extends Exception> exceptionType : connectionErrors) {
+ if (exceptionType.isAssignableFrom(exception.getClass())) {
+ isFailoverException = true;
+ break;
+ }
+ }
+ }
+ return isFailoverException;
+ }
}
diff --git a/gateway-util-common/src/test/java/org/apache/knox/gateway/util/HttpUtilsTest.java b/gateway-util-common/src/test/java/org/apache/knox/gateway/util/HttpUtilsTest.java
index 05aea58..7a73095 100644
--- a/gateway-util-common/src/test/java/org/apache/knox/gateway/util/HttpUtilsTest.java
+++ b/gateway-util-common/src/test/java/org/apache/knox/gateway/util/HttpUtilsTest.java
@@ -19,11 +19,17 @@
import org.junit.Test;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.net.NoRouteToHostException;
+import java.net.PortUnreachableException;
+import java.net.UnknownHostException;
import java.util.Arrays;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
+import static org.apache.knox.gateway.util.HttpUtils.isConnectionError;
import static org.hamcrest.CoreMatchers.notNullValue;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
@@ -212,4 +218,14 @@
assertThat( map.get( "qry" ).size(), is( 1 ) );
assertThat( map.get( "qry" ).get(0), is( "Hadoop:service=NameNode,name=NameNodeInfo" ) );
}
+
+ @Test
+ public void testRelevantConnectionErrors() {
+ assertThat(isConnectionError(new UnknownHostException()), is(true));
+ assertThat(isConnectionError(new ConnectException()), is(true));
+ assertThat(isConnectionError(new NoRouteToHostException()), is(true));
+ assertThat(isConnectionError(new PortUnreachableException()), is(true));
+ assertThat(isConnectionError(new IOException()), is(false));
+ assertThat(isConnectionError(new RuntimeException()), is(false));
+ }
}
