On 2/15/24 13:04, Ruediger Pluem wrote:

On 2/15/24 9:28 AM, jean-frederic clere wrote:
On 2/14/24 20:54, Ruediger Pluem wrote:

On 2/14/24 3:45 PM, jean-frederic clere wrote:
On 2/14/24 08:19, Ruediger Pluem wrote:

On 2/9/24 11:59 AM, jean-frederic clere wrote:

I have noted to the reset() clean up too much in the balancers:
mod_lbmethod_bybusyness.c for example does:
       for (i = 0; i < balancer->workers->nelts; i++, worker++) {
           (*worker)->s->lbstatus = 0;
           ap_proxy_set_busy_count(*worker, 0); /* BAD */
In fact reset() is called by ap_proxy_initialize_balancer() when a child 
process is created... Resetting the counters messes up
the logic.

Does it make sense to stop calling reset() from ap_proxy_initialize_balancer() 
or is it better to fix all reset()?

I am not sure what the original idea / intention of reset was. Until this is 
clarified I would not remove the call to reset in
ap_proxy_initialize_balancer(). In ap_proxy_sync_balancer the call to it is 
guarded by a check for the need_reset flag. Maybe
gives a hint.

Rethinking this. I guess we lack a clear API definition of the reset method of 
a balancer provider and it does not really seem
sensible to reset the stats in case a new child is created. Hence I guess the 
regression risk is rather low when just removing
this call to reset.

The need_reset is set when a worker is enabled or when the load balancing 
method is changed.

Isn't that a similar situation to when a worker is added (in respect to the 
point below)?

The worker are added disabled.

Which means stuff gets reset once they get enabled?

Correct ;-)





Reply via email to