This is an automated email from the ASF dual-hosted git repository.
hanicz 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 b438768cd KNOX-3381: Fix HaDispatch unnecessary failover due to
IOException during writeOutboundResponse (#1308)
b438768cd is described below
commit b438768cd87b96d0bcc677745e3c39acb5e4d72d
Author: hanicz <[email protected]>
AuthorDate: Mon Jul 20 08:09:24 2026 +0200
KNOX-3381: Fix HaDispatch unnecessary failover due to IOException during
writeOutboundResponse (#1308)
---
.../gateway/ha/dispatch/AtlasApiHaDispatch.java | 4 +-
.../dispatch/AtlasApiTrustedProxyHaDispatch.java | 4 +-
.../knox/gateway/ha/dispatch/AtlasHaDispatch.java | 4 +-
.../ha/dispatch/AtlasTrustedProxyHaDispatch.java | 4 +-
.../ha/dispatch/ConfigurableHADispatch.java | 3 +-
.../ha/dispatch/ConfigurableHADispatchTest.java | 86 ++++++++++++++++++++++
.../gateway/ha/dispatch/DefaultHaDispatchTest.java | 26 ++++---
.../knox/gateway/dispatch/NiFiHaDispatch.java | 3 +-
8 files changed, 113 insertions(+), 21 deletions(-)
diff --git
a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasApiHaDispatch.java
b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasApiHaDispatch.java
index 59fbb9ec3..ce2b0ebb8 100644
---
a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasApiHaDispatch.java
+++
b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasApiHaDispatch.java
@@ -71,12 +71,12 @@ public class AtlasApiHaDispatch extends DefaultHaDispatch {
failoverRequest(outboundRequest, inboundRequest,
outboundResponse, inboundResponse, new Exception("Atlas HA redirection"));
}
- writeOutboundResponse(outboundRequest, inboundRequest,
outboundResponse, inboundResponse);
-
} catch (IOException e) {
LOG.errorConnectingToServer(outboundRequest.getURI().toString(),
e);
failoverRequest(outboundRequest, inboundRequest, outboundResponse,
inboundResponse, e);
+ return;
}
+ writeOutboundResponse(outboundRequest, inboundRequest,
outboundResponse, inboundResponse);
}
}
diff --git
a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasApiTrustedProxyHaDispatch.java
b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasApiTrustedProxyHaDispatch.java
index fdf4b91b4..e3948f1fc 100644
---
a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasApiTrustedProxyHaDispatch.java
+++
b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasApiTrustedProxyHaDispatch.java
@@ -44,11 +44,11 @@ public class AtlasApiTrustedProxyHaDispatch extends
DefaultHaDispatch {
failoverRequest(outboundRequest, inboundRequest, outboundResponse,
inboundResponse, new Exception("Atlas HA redirection"));
}
- writeOutboundResponse(outboundRequest, inboundRequest, outboundResponse,
inboundResponse);
-
} catch (IOException e) {
LOG.errorConnectingToServer(outboundRequest.getURI().toString(), e);
failoverRequest(outboundRequest, inboundRequest, outboundResponse,
inboundResponse, e);
+ return;
}
+ writeOutboundResponse(outboundRequest, inboundRequest, outboundResponse,
inboundResponse);
}
}
diff --git
a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasHaDispatch.java
b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasHaDispatch.java
index 788514927..4fb5ff610 100644
---
a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasHaDispatch.java
+++
b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasHaDispatch.java
@@ -64,12 +64,12 @@ public class AtlasHaDispatch extends DefaultHaDispatch {
}
}
- writeOutboundResponse(outboundRequest, inboundRequest,
outboundResponse, inboundResponse);
-
} catch (IOException e) {
LOG.errorConnectingToServer(outboundRequest.getURI().toString(),
e);
failoverRequest(outboundRequest, inboundRequest, outboundResponse,
inboundResponse, e);
+ return;
}
+ writeOutboundResponse(outboundRequest, inboundRequest,
outboundResponse, inboundResponse);
}
private boolean isLoginRedirect(Header locationHeader) {
diff --git
a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasTrustedProxyHaDispatch.java
b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasTrustedProxyHaDispatch.java
index b42ae1e10..c9e7993ef 100644
---
a/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasTrustedProxyHaDispatch.java
+++
b/gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/AtlasTrustedProxyHaDispatch.java
@@ -50,12 +50,12 @@ public class AtlasTrustedProxyHaDispatch extends
ConfigurableHADispatch {
}
}
- writeOutboundResponse(outboundRequest, inboundRequest, outboundResponse,
inboundResponse);
-
} catch (IOException e) {
LOG.errorConnectingToServer(outboundRequest.getURI().toString(), e);
failoverRequest(outboundRequest, inboundRequest, outboundResponse,
inboundResponse, e);
+ return;
}
+ writeOutboundResponse(outboundRequest, inboundRequest, outboundResponse,
inboundResponse);
}
private boolean isLoginRedirect(Header locationHeader) {
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 40382dd43..5c24f81a6 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
@@ -113,7 +113,6 @@ public class ConfigurableHADispatch extends
ConfigurableDispatch implements Comm
HttpResponse inboundResponse = null;
try {
inboundResponse = executeOutboundRequest(outboundRequest);
- writeOutboundResponse(outboundRequest, inboundRequest, outboundResponse,
inboundResponse);
} catch ( IOException e ) {
/* if non-idempotent requests are not allowed to failover, unless it's a
connection error */
if(!isConnectionError(e.getCause()) &&
isNonIdempotentAndNonIdempotentFailoverDisabled(outboundRequest)) {
@@ -125,7 +124,9 @@ public class ConfigurableHADispatch extends
ConfigurableDispatch implements Comm
LOG.errorConnectingToServer(outboundRequest.getURI().toString(), e);
failoverRequest(outboundRequest, inboundRequest, outboundResponse,
inboundResponse, e);
}
+ return;
}
+ writeOutboundResponse(outboundRequest, inboundRequest, outboundResponse,
inboundResponse);
}
protected void failoverRequest(HttpUriRequest outboundRequest,
HttpServletRequest inboundRequest, HttpServletResponse outboundResponse,
HttpResponse inboundResponse, Exception exception) throws IOException {
diff --git
a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/dispatch/ConfigurableHADispatchTest.java
b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/dispatch/ConfigurableHADispatchTest.java
index accca102e..92f1942dc 100644
---
a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/dispatch/ConfigurableHADispatchTest.java
+++
b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/dispatch/ConfigurableHADispatchTest.java
@@ -198,4 +198,90 @@ public class ConfigurableHADispatchTest {
Assert.assertEquals(DigestUtils.sha256Hex(activeURL),
captureCookieValue.getValue().getValue());
}
+ /**
+ * Test that a failure while copying an already-received backend response to
the client
+ * (e.g. a parse/rewrite error thrown from writeOutboundResponse) does NOT
trigger a failover.
+ * The backend has already served the request, so replaying it against
another node would be wrong.
+ */
+ @Test
+ public void testNoFailoverWhenResponseCopyFails() throws Exception {
+ String serviceName = "OOZIE";
+ HaDescriptor descriptor = HaDescriptorFactory.createDescriptor();
+
descriptor.addServiceConfig(HaDescriptorFactory.createServiceConfig(serviceName,
"true", "1", "1000", null, null, null, null, null, null, null));
+ HaProvider provider = new DefaultHaProvider(descriptor);
+ URI uri1 = new URI( "http://host1.valid"; );
+ URI uri2 = new URI( "http://host2.valid"; );
+ ArrayList<String> urlList = new ArrayList<>();
+ urlList.add(uri1.toString());
+ urlList.add(uri2.toString());
+ provider.addHaService(serviceName, urlList);
+
+ BasicHttpParams params = new BasicHttpParams();
+
+ HttpUriRequest outboundRequest =
EasyMock.createNiceMock(HttpRequestBase.class);
+ EasyMock.expect(outboundRequest.getMethod()).andReturn( "GET" ).anyTimes();
+ EasyMock.expect(outboundRequest.getURI()).andReturn( uri1 ).anyTimes();
+ EasyMock.expect(outboundRequest.getParams()).andReturn( params
).anyTimes();
+
+ HttpServletRequest inboundRequest =
EasyMock.createNiceMock(HttpServletRequest.class);
+ EasyMock.expect(inboundRequest.getRequestURL()).andReturn( new
StringBuffer(uri1.toString()) ).anyTimes();
+
EasyMock.expect(inboundRequest.getAttribute("dispatch.ha.failover.counter")).andReturn(new
AtomicInteger(0)).anyTimes();
+
+ /* backend response */
+ CloseableHttpResponse inboundResponse =
EasyMock.createNiceMock(CloseableHttpResponse.class);
+ final StatusLine statusLine = EasyMock.createNiceMock(StatusLine.class);
+ final HttpEntity entity = EasyMock.createNiceMock(HttpEntity.class);
+ final Header header = EasyMock.createNiceMock(Header.class);
+ final ServletContext context =
EasyMock.createNiceMock(ServletContext.class);
+ final GatewayConfig config = EasyMock.createNiceMock(GatewayConfig.class);
+ final ByteArrayInputStream backendResponse = new
ByteArrayInputStream("knox-backend".getBytes(
+ StandardCharsets.UTF_8));
+
+
EasyMock.expect(inboundResponse.getStatusLine()).andReturn(statusLine).anyTimes();
+
EasyMock.expect(statusLine.getStatusCode()).andReturn(HttpStatus.SC_OK).anyTimes();
+ EasyMock.expect(inboundResponse.getEntity()).andReturn(entity).anyTimes();
+ EasyMock.expect(inboundResponse.getAllHeaders()).andReturn(new
Header[0]).anyTimes();
+
EasyMock.expect(inboundRequest.getServletContext()).andReturn(context).anyTimes();
+ EasyMock.expect(entity.getContent()).andReturn(backendResponse).anyTimes();
+ EasyMock.expect(entity.getContentType()).andReturn(header).anyTimes();
+ EasyMock.expect(header.getElements()).andReturn(new
HeaderElement[]{}).anyTimes();
+ EasyMock.expect(entity.getContentLength()).andReturn(4L).anyTimes();
+
EasyMock.expect(context.getAttribute(GatewayConfig.GATEWAY_CONFIG_ATTRIBUTE)).andReturn(config).anyTimes();
+
+ HttpServletResponse outboundResponse =
EasyMock.createNiceMock(HttpServletResponse.class);
+ EasyMock.expect(outboundResponse.getOutputStream()).andAnswer( new
IAnswer<SynchronousServletOutputStreamAdapter>() {
+ @Override
+ public SynchronousServletOutputStreamAdapter answer() {
+ return new SynchronousServletOutputStreamAdapter() {
+ @Override
+ public void write( int b ) throws IOException {
+ throw new IOException( "response copy failed" );
+ }
+ };
+ }
+ }).once();
+
+ CloseableHttpClient mockHttpClient =
EasyMock.createNiceMock(CloseableHttpClient.class);
+
EasyMock.expect(mockHttpClient.execute(outboundRequest)).andReturn(inboundResponse).once();
+
+ EasyMock.replay(outboundRequest, inboundRequest, outboundResponse,
mockHttpClient, inboundResponse,
+ statusLine, entity, header, context, config);
+
+ ConfigurableHADispatch dispatch = new ConfigurableHADispatch();
+ dispatch.setHttpClient(mockHttpClient);
+ dispatch.setHaProvider(provider);
+ dispatch.setServiceRole(serviceName);
+ dispatch.init();
+
+ try {
+ dispatch.executeRequestWrapper(outboundRequest, inboundRequest,
outboundResponse);
+ Assert.fail("Expected the response-copy IOException to propagate");
+ } catch (IOException e) {
+ Assert.assertEquals("response copy failed", e.getMessage());
+ }
+
+ EasyMock.verify(mockHttpClient);
+ Assert.assertEquals(uri1.toString(), provider.getActiveURL(serviceName));
+ }
+
}
diff --git
a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/dispatch/DefaultHaDispatchTest.java
b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/dispatch/DefaultHaDispatchTest.java
index 02c711ac5..288756ea5 100644
---
a/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/dispatch/DefaultHaDispatchTest.java
+++
b/gateway-provider-ha/src/test/java/org/apache/knox/gateway/ha/dispatch/DefaultHaDispatchTest.java
@@ -828,17 +828,17 @@ public class DefaultHaDispatchTest {
outboundResponse.setStatus(EasyMock.captureInt(statusCodeCapture));
- EasyMock.expectLastCall().once();
+ EasyMock.expectLastCall().anyTimes();
EasyMock.expect(outboundResponse.getOutputStream())
.andAnswer((IAnswer<SynchronousServletOutputStreamAdapter>) () -> new
SynchronousServletOutputStreamAdapter() {
@Override
public void write( int b ) throws IOException {
- throw new IOException( "unreachable-host" ); // Fail-over condition
+ /* do nothing */
}
- }).atLeastOnce();
+ }).anyTimes();
CloseableHttpClient mockHttpClient =
EasyMock.createNiceMock(CloseableHttpClient.class);
-
EasyMock.expect(mockHttpClient.execute(outboundRequest)).andReturn(inboundResponse).anyTimes();
+ EasyMock.expect(mockHttpClient.execute(outboundRequest)).andThrow(new
IOException("unreachable-host")).andReturn(inboundResponse);
EasyMock.replay(filterConfig,
servletContext,
@@ -958,17 +958,17 @@ public class DefaultHaDispatchTest {
outboundResponse.setStatus(EasyMock.captureInt(statusCodeCapture));
- EasyMock.expectLastCall().once();
+ EasyMock.expectLastCall().anyTimes();
EasyMock.expect(outboundResponse.getOutputStream())
.andAnswer((IAnswer<SynchronousServletOutputStreamAdapter>) () -> new
SynchronousServletOutputStreamAdapter() {
@Override
public void write( int b ) throws IOException {
- throw new IOException( "unreachable-host" ); // Fail-over condition
+ /* do nothing */
}
- }).atLeastOnce();
+ }).anyTimes();
CloseableHttpClient mockHttpClient =
EasyMock.createNiceMock(CloseableHttpClient.class);
-
EasyMock.expect(mockHttpClient.execute(outboundRequest)).andReturn(inboundResponse).anyTimes();
+ EasyMock.expect(mockHttpClient.execute(outboundRequest)).andThrow(new
IOException("unreachable-host")).andReturn(inboundResponse);
EasyMock.replay(filterConfig,
servletContext,
@@ -1103,12 +1103,16 @@ public class DefaultHaDispatchTest {
.andAnswer((IAnswer<SynchronousServletOutputStreamAdapter>) () ->
new SynchronousServletOutputStreamAdapter() {
@Override
public void write( int b ) throws IOException {
- throw new IOException( "unreachable-host" ); // Fail-over
condition
+ /* do nothing */
}
- }).atLeastOnce();
+ }).anyTimes();
CloseableHttpClient mockHttpClient =
EasyMock.createNiceMock(CloseableHttpClient.class);
-
EasyMock.expect(mockHttpClient.execute(outboundRequest)).andReturn(inboundResponse).anyTimes();
+ if (withCookie) {
+ EasyMock.expect(mockHttpClient.execute(outboundRequest)).andThrow(new
IOException("unreachable-host")).once();
+ } else {
+ EasyMock.expect(mockHttpClient.execute(outboundRequest)).andThrow(new
IOException("unreachable-host")).andReturn(inboundResponse);
+ }
EasyMock.replay(filterConfig,
servletContext,
diff --git
a/gateway-service-nifi/src/main/java/org/apache/knox/gateway/dispatch/NiFiHaDispatch.java
b/gateway-service-nifi/src/main/java/org/apache/knox/gateway/dispatch/NiFiHaDispatch.java
index 6d390a3db..04dbfe2e5 100644
---
a/gateway-service-nifi/src/main/java/org/apache/knox/gateway/dispatch/NiFiHaDispatch.java
+++
b/gateway-service-nifi/src/main/java/org/apache/knox/gateway/dispatch/NiFiHaDispatch.java
@@ -42,11 +42,12 @@ public class NiFiHaDispatch extends DefaultHaDispatch {
try {
outboundRequest = NiFiRequestUtil.modifyOutboundRequest(outboundRequest,
inboundRequest);
inboundResponse = executeOutboundRequest(outboundRequest);
- writeOutboundResponse(outboundRequest, inboundRequest, outboundResponse,
inboundResponse);
} catch (IOException e) {
LOG.errorConnectingToServer(outboundRequest.getURI().toString(), e);
failoverRequest(outboundRequest, inboundRequest, outboundResponse,
inboundResponse, e);
+ return;
}
+ writeOutboundResponse(outboundRequest, inboundRequest, outboundResponse,
inboundResponse);
}
/**
