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.
There is another thing adding a new worker via /balancer-manager/ probably
requires some kind of reset() otherwise all the load
moves to the new worker which is not the best.... May be calling age() or
triggering calls to age() can help.
Doesn't ap_proxy_sync_balancer take care of this which is called before
processing a request?
Regards
Rüdiger
--
Cheers
Jean-Frederic
