On 12/10/2014 02:50 PM, Yann Ylavic wrote:
On Wed, Dec 10, 2014 at 2:21 PM, Jan Kaluža <jkal...@redhat.com> wrote:
On 12/10/2014 01:49 PM, Plüm, Rüdiger, Vodafone Group wrote:
Isn't the config merge on a critical path with every request? So double
for loops always worry me a little bit from performance point of view.

I think that happens only in ap_fixup_virtual_hosts call, which is executed
only during the httpd start. From this point of view, double for loop should
be OK.

Right, but since there will possibly more balancers/workers (main and
vhosts ones), the child_init() of mod_proxy and mod_proxy_balancer may
take longer to initialize all the balancers/workers.
This is not at request time still.

That's true, but I'm not sure I see different way how to do it without changing the way how the balancers are stored.

I could for example create temporary apr_hash which would allow iterating override balancers just once, but I'm not sure it's so much better than the current way. (Btw, is there a reason, why are we storing balancers in array?)

Also, what will the balancer_handler() do now, manage main
workers/balancers only?

Hm, this should not change anything done by balancer_handler().

The patch only sets the balancer->s before the post_config is called. The vhost has still its own proxy_balancer instance as well as the main config. The only thing changed by the patch is that if you set some option in vhost config section, it will get set in proxy_balancer too.

The question here is if it is a valid use-case to use balancer with the same name in the main config and also in the vhost, but treat them as two independent balancers. If that's valid use-case, then the patch is wrong, because it changes the main balancer according to things done in vhost.

If you use BalancerPersist, it (imho) does not matter what you do with balancer->s before post_config, because after the post_config, it will get replaced by the stored shared memory.

Regards,
Yann.



Regards,
Jan Kaluza

Reply via email to