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:
Hi,
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
this
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 ;-)
Regards
Rüdiger
--
Cheers
Jean-Frederic
