You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@knox.apache.org by GitBox <gi...@apache.org> on 2021/06/09 16:40:14 UTC

[GitHub] [knox] pzampino opened a new pull request #456: KNOX-2619 - HA dispatch should failover regardless of noFallback conf…

pzampino opened a new pull request #456:
URL: https://github.com/apache/knox/pull/456


   …ig until session is established
   
   ## What changes were proposed in this pull request?
   
   Modified ConfigurableHaDispatch behavior wrt to failover when sticky sessions are enabled, but fallback (i.e., failover) is disabled, and the initial request fails. With this change, the dispatch will failover until a session is established, regardless of the fallback configuration.
   
   ## How was this patch tested?
   
   Added tests in org.apache.knox.gateway.ha.dispatch.DefaultHaDispatchTest (testFailoverStickyOnFallbackOffLoadbalancingOn, testFailoverStickyOnFallbackOffLoadbalancingOff).
   
   mvn -Ppackage,release clean install
   


-- 
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



[GitHub] [knox] pzampino merged pull request #456: KNOX-2619 - HA dispatch should failover regardless of noFallback conf…

Posted by GitBox <gi...@apache.org>.
pzampino merged pull request #456:
URL: https://github.com/apache/knox/pull/456


   


-- 
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



[GitHub] [knox] pzampino commented on a change in pull request #456: KNOX-2619 - HA dispatch should failover regardless of noFallback conf…

Posted by GitBox <gi...@apache.org>.
pzampino commented on a change in pull request #456:
URL: https://github.com/apache/knox/pull/456#discussion_r648643626



##########
File path: gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/ConfigurableHADispatch.java
##########
@@ -224,7 +224,7 @@ private void setKnoxHaCookie(HttpServletRequest inboundRequest,
 
   protected void failoverRequest(HttpUriRequest outboundRequest, HttpServletRequest inboundRequest, HttpServletResponse outboundResponse, HttpResponse inboundResponse, Exception exception) throws IOException {
     /* check for a case where no fallback is configured */
-    if(stickySessionsEnabled && noFallbackEnabled) {
+    if(stickySessionsEnabled && noFallbackEnabled && inboundRequest.getCookies() != null) {

Review comment:
       I've added a check for the knox session cookie as the additional condition.




-- 
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



[GitHub] [knox] moresandeep commented on a change in pull request #456: KNOX-2619 - HA dispatch should failover regardless of noFallback conf…

Posted by GitBox <gi...@apache.org>.
moresandeep commented on a change in pull request #456:
URL: https://github.com/apache/knox/pull/456#discussion_r648517128



##########
File path: gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/ConfigurableHADispatch.java
##########
@@ -224,7 +224,7 @@ private void setKnoxHaCookie(HttpServletRequest inboundRequest,
 
   protected void failoverRequest(HttpUriRequest outboundRequest, HttpServletRequest inboundRequest, HttpServletResponse outboundResponse, HttpResponse inboundResponse, Exception exception) throws IOException {
     /* check for a case where no fallback is configured */
-    if(stickySessionsEnabled && noFallbackEnabled) {
+    if(stickySessionsEnabled && noFallbackEnabled && inboundRequest.getCookies() != null) {

Review comment:
       That is a good point, there could be a chance that browser/client might send other cookies during the first request(which i did not think about). Checking for `stickySessionCookieName` cookie would make this more robust.




-- 
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



[GitHub] [knox] lmccay commented on a change in pull request #456: KNOX-2619 - HA dispatch should failover regardless of noFallback conf…

Posted by GitBox <gi...@apache.org>.
lmccay commented on a change in pull request #456:
URL: https://github.com/apache/knox/pull/456#discussion_r648502609



##########
File path: gateway-provider-ha/src/main/java/org/apache/knox/gateway/ha/dispatch/ConfigurableHADispatch.java
##########
@@ -224,7 +224,7 @@ private void setKnoxHaCookie(HttpServletRequest inboundRequest,
 
   protected void failoverRequest(HttpUriRequest outboundRequest, HttpServletRequest inboundRequest, HttpServletResponse outboundResponse, HttpResponse inboundResponse, Exception exception) throws IOException {
     /* check for a case where no fallback is configured */
-    if(stickySessionsEnabled && noFallbackEnabled) {
+    if(stickySessionsEnabled && noFallbackEnabled && inboundRequest.getCookies() != null) {

Review comment:
       Is it sufficient to check that there are no cookies at all or do we need to check for our own cookie for the sticky sessions? I suppose that the currently the driver isn't proactively adding a cookie to the first request but can we count on that assumption not going bad?




-- 
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