[GitHub] [knox] shanyu commented on a change in pull request #177: [WIP] KNOX-2095 - Adding in DefaultDispatch code and tests to handle 504 errors

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org


With regards,
Apache Git Services