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/12/03 16:22:03 UTC

[GitHub] [apisix] whl739 opened a new issue #2954: ctx.var.upstream_addr may be separated by commas.

whl739 opened a new issue #2954:
URL: https://github.com/apache/apisix/issues/2954


   ### Issue description
   See code in `apisix/balancer/ewma.lua`:
   ```
   local function _ewma_after_balance(ctx, before_retry)
       if before_retry then
           -- don't count tries which fail to complete
           return nil 
       end 
   
       local response_time = tonumber(ctx.var.upstream_response_time) or 0
       local connect_time = tonumber(ctx.var.upstream_connect_time) or 0
       local rtt = connect_time + response_time
       local upstream = ctx.var.upstream_addr
   
       if not upstream then
           return nil, "no upstream addr found"
       end 
   
       return get_or_update_ewma(upstream, rtt, true)
   end
   ```
   
   Both var `upstream_response_time` and `upstream_addr` may  be separated by commas,  it will cause unexpected result.
   And if openresty is not patched with `lua-var-nginx-module`, these times are all 0.
   


----------------------------------------------------------------
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 issue #2954: ctx.var.upstream_addr may be separated by commas.

Posted by GitBox <gi...@apache.org>.
spacewander commented on issue #2954:
URL: https://github.com/apache/apisix/issues/2954#issuecomment-738700318


   I think there are two solution
   1. use `ngx.var` instead
   2. explicitly clear the cache before retrying
   
   What about your opinion? @membphis 


----------------------------------------------------------------
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] membphis commented on issue #2954: ctx.var.upstream_addr may be separated by commas.

Posted by GitBox <gi...@apache.org>.
membphis commented on issue #2954:
URL: https://github.com/apache/apisix/issues/2954#issuecomment-738896061


   2th is better for me


----------------------------------------------------------------
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 closed issue #2954: ctx.var.upstream_addr may be separated by commas.

Posted by GitBox <gi...@apache.org>.
spacewander closed issue #2954:
URL: https://github.com/apache/apisix/issues/2954


   


----------------------------------------------------------------
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] whl739 commented on issue #2954: ctx.var.upstream_addr may be separated by commas.

Posted by GitBox <gi...@apache.org>.
whl739 commented on issue #2954:
URL: https://github.com/apache/apisix/issues/2954#issuecomment-738694675


   There's another issue here, ctx.var will cache the result, see case:
   if upstream fails, balancer phase will enter multi times, a balancer algorithm may want to update upstream status(e.g. response time), but `ctx.var.upstream_xxx`'s value is the first one.


----------------------------------------------------------------
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 issue #2954: ctx.var.upstream_addr may be separated by commas.

Posted by GitBox <gi...@apache.org>.
spacewander commented on issue #2954:
URL: https://github.com/apache/apisix/issues/2954#issuecomment-738500790


   PR is welcome! The value is 0 because `tonumber("xxx")` is nil.


----------------------------------------------------------------
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 issue #2954: ctx.var.upstream_addr may be separated by commas.

Posted by GitBox <gi...@apache.org>.
spacewander commented on issue #2954:
URL: https://github.com/apache/apisix/issues/2954#issuecomment-739605847


   I look through the code and find that we don't use `ctx.var.upstream_xxx` in balancer phase. As there is nothing to purge, we can simply leave a comment.


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