shanyu 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_r346570126
##########
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:
@risdenk , for active/passive, it doesn't make sense to failover from active
to passive in the case of socket timeout. For active/active, it may have value
to failover. However, I have 2 arguments against this:
1) Usually the client side that talks to knox has its own timeout (think
about a Jupyter notebook), so letting service failover in the case of socket
timeout multiple times would eventually cause client side timeout.
2) Most likely the timeout is because the request itself took too much time
exceeding the socket timeout config (considering Livy session creation), so
failover to another server would also cause a socket timeout as well.
And for active-active type of service, if really needed we can always retry
at the client side.
I think letting client side know that the request caused timeout is very
important, instead of Letting them face a generic 500 error without knowing
what to do next.
Or we could
----------------------------------------------------------------
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
