vozlt / nginx-module-vts

Nginx virtual host traffic status module
BSD 2-Clause "Simplified" License
3.17k stars 456 forks source link

the status of server is not current when max_fails = 0 #275

Closed suningzh closed 11 months ago

suningzh commented 11 months ago

the upstream is upstream test { server 10.88.188.1:80 weight=1 fail_timeout=10 max_fails=0; }

the status of this server is:

"weight": 1, "maxFails": 0, "failTimeout": 10, "backup": false, "down": true,

code is usn.down = (peer->fails >= peer->max_fails || peer->down);

should we first judge the (peer->max_fails && peer->fails >= peer->max_fails)

u5surf commented 11 months ago

@suningzh Hi, Thanks for reporting. https://nginx.org/en/docs/http/ngx_http_upstream_module.html#server The spec of ngx_http_upstream_module said below

max_fails=number
sets the number of unsuccessful attempts to communicate with the server that should happen in the duration set by the fail_timeout parameter to consider the server unavailable for a duration also set by the fail_timeout parameter. 
...
The zero value disables the accounting of attempts. 

Because the calculating of the peer failure is disabled, usn.down should not work completely. Thus it indicates down.

should we first judge the (peer->max_fails && peer->fails >= peer->max_fails)

if peer->max_fails is zero, it is obvious that peer->fails is grater or equal than peer->max_fails except peer->fails is negative value.

suningzh commented 11 months ago

@u5surf hi Because the calculating of the peer failure is disabled, usn.down should not work completely. Thus it indicates down

The zero value just disables the accounting of attempts. so you can't use the fails or max_fails to judge the down/up status of this peer

such as one peer with zero max_fails is reachable, should we set this peer as down ?

u5surf commented 11 months ago

@suningzh As you've already found out the fundamentals of this issue, we don't have any status which it cannot calulating of the peer failure, instead it has been included all in usn.down. In other words, we would like to have another status something like usn.indefinite which could not calculate the peer's failure. Unfortunately, it could quite difficult that has necessary to change many code in this module.

suningzh commented 11 months ago

@u5surf Thanks for your reply