You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2020/11/09 12:55:55 UTC

[GitHub] [apisix] zfs123 opened a new pull request #2676: chash pick next if last doesn't work

zfs123 opened a new pull request #2676:
URL: https://github.com/apache/apisix/pull/2676


   chash will pick next if last doesn't work to enable set_more_retries().
   
   ### Pre-submission checklist:
   
   * [x] Did you explain what problem does this PR solve? Or what new features have been added?
   * [ ] Have you added corresponding test cases?
   * [ ] Have you modified the corresponding document?
   * [x] Is this PR backward compatible?
   


----------------------------------------------------------------
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] [apisix] tokers merged pull request #2676: fix: make set_more_retries() work when upstream_type is chash

Posted by GitBox <gi...@apache.org>.
tokers merged pull request #2676:
URL: https://github.com/apache/apisix/pull/2676


   


----------------------------------------------------------------
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] [apisix] spacewander commented on pull request #2676: fix: make set_more_retries() work when upstream_type is chash

Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #2676:
URL: https://github.com/apache/apisix/pull/2676#issuecomment-728662156


   Please write new test to verify the fix and make sure the tests are passed.


----------------------------------------------------------------
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] [apisix] spacewander commented on a change in pull request #2676: fix: make set_more_retries() work when upstream_type is chash

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2676:
URL: https://github.com/apache/apisix/pull/2676#discussion_r522823505



##########
File path: apisix/balancer/chash.lua
##########
@@ -55,6 +55,7 @@ end
 
 function _M.new(up_nodes, upstream)
     local str_null = str_char(0)
+    local last_server_index

Review comment:
       The `last_server_index` should be in ctx scope, not the upstream one. We should not share the same index across different requests to an upstream.




----------------------------------------------------------------
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] [apisix] moonming commented on pull request #2676: fix: make set_more_retries() work when upstream_type is chash

Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2676:
URL: https://github.com/apache/apisix/pull/2676#issuecomment-754301463


   ping @zfs123 


----------------------------------------------------------------
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] [apisix] spacewander commented on a change in pull request #2676: fix: make set_more_retries() work when upstream_type is chash

Posted by GitBox <gi...@apache.org>.
spacewander commented on a change in pull request #2676:
URL: https://github.com/apache/apisix/pull/2676#discussion_r524860338



##########
File path: apisix/balancer/chash.lua
##########
@@ -68,8 +68,13 @@ function _M.new(up_nodes, upstream)
     return {
         upstream = upstream,
         get = function (ctx)
-            local chash_key = fetch_chash_hash_key(ctx, upstream)
-            local id = picker:find(chash_key)
+            local id
+            if ctx.balancer_try_count > 1 and ctx.last_server_index then
+                id, ctx.last_server_index = picker:next(ctx.last_server_index)

Review comment:
       Better to use name like `ctx.chash_last_server_index`




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