You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@knox.apache.org by am...@apache.org on 2023/06/02 07:55:11 UTC
[knox] branch master updated: KNOX-2912 - Don't fail over non idempotent requests unless it's a connect exception (#757)
This is an automated email from the ASF dual-hosted git repository.
amagyar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/knox.git
The following commit(s) were added to refs/heads/master by this push:
new 2c339f746 KNOX-2912 - Don't fail over non idempotent requests unless it's a connect exception (#757)
2c339f746 is described below
commit 2c339f746236599ad8dc1637133361cb0d8339b3
Author: Attila Magyar <m....@gmail.com>
AuthorDate: Fri Jun 2 09:55:04 2023 +0200
KNOX-2912 - Don't fail over non idempotent requests unless it's a connect exception (#757)
---
.../ha/dispatch/ConfigurableHADispatch.java | 10 ++++++--
.../ha/dispatch/i18n/HaDispatchMessages.java | 2 +-
.../knox/gateway/dispatch/DefaultDispatch.java | 2 +-
.../org/apache/knox/gateway/util/HttpUtils.java | 27 ++++++++++++++++++++++
.../apache/knox/gateway/util/HttpUtilsTest.java | 16 +++++++++++++
5 files changed, 53 insertions(+), 4 deletions(-)
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 826275bfe..8d5f5b859 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 @@ public class ConfigurableHADispatch extends ConfigurableDispatch {
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 @@ public class ConfigurableHADispatch extends ConfigurableDispatch {
}
}
+ 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 004af14d2..901d87bc6 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 @@ public interface HaDispatchMessages {
@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 066a842d5..58f06942d 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 @@ public class DefaultDispatch extends AbstractGatewayDispatch {
// 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 ac9621411..46d36f599 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.Map;
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 @@ public class HttpUtils {
}
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 05aea58cc..7a7309519 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 @@ package org.apache.knox.gateway.util;
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 @@ public class HttpUtilsTest {
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));
+ }
}