jameschen1519 commented on a change in pull request #177: [WIP] KNOX-2095 -
Adding in DefaultDispatch code and tests to handle 504 errors
URL: https://github.com/apache/knox/pull/177#discussion_r344322230
##########
File path:
gateway-spi/src/main/java/org/apache/knox/gateway/dispatch/DefaultDispatch.java
##########
@@ -138,6 +144,10 @@ protected HttpResponse executeOutboundRequest(
HttpUriRequest outboundRequest )
}
}
auditor.audit( Action.DISPATCH, outboundRequest.getURI().toString(),
ResourceType.URI, ActionOutcome.SUCCESS, RES.responseStatus( statusCode ) );
+ } catch( SocketTimeoutException e ) {
+ //Set a 504 instead of throwing an IO exception in the event of a
socket timeout,
+ //as an IO exception forces a 500 to be displayed.
Review comment:
Thanks for the feedback. I'll add in the audit and log lines, but regarding
the HA case, I had a discussion about this and a few points were raised against
adding in the HA:
1. In the active-passive case, there shouldn't be any socket timeouts.
2. In the active-active case from node balancing, if a socket timeout occurs
on one node, it is likely to occur on other nodes.
3. If failing over for a socket timeout, in the worst case scenario it may
result in a wait time of (socketTimeout x number of failover nodes) for the
client.
Would it still be worth adding in failovers for the HA case?
----------------------------------------------------------------
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]
With regards,
Apache Git Services
