httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jan Kaluža <jkal...@redhat.com>
Subject Re: [PATCH] Balancers, VirtualHost and ProxyPass
Date Fri, 12 Dec 2014 12:46:07 GMT
On 12/12/2014 12:08 PM, Yann Ylavic wrote:
> Hi Jan,
>
> On Fri, Dec 12, 2014 at 9:44 AM, Jan Kaluža <jkaluza@redhat.com> wrote:
>> On 12/11/2014 03:05 PM, Plüm, Rüdiger, Vodafone Group wrote:
>>>
>>> Looks fine in general. Details:
>>
>> I hope I've finally fixed everything now :), see the attached patch please.
>
> Aren't the following parameters missing for the merge:
>      unsigned int    max_attempts_set:1;
>      unsigned int    vhosted:1;
>      unsigned int    inactive:1;
> vhosted does not seem to be used, but seems a good candidate to
> differentiate base vs vhost balancer.

max_attempts_set is merged. I've been merging only things which are set 
by set_balancer_param(). vhosted and inactive seems both to be unused. 
Without the semantics for those, I have no idea how to merge them 
(should I add vhosted_set/inactive_set or just merge them as they are...).

I think vhosted has been committed by a mistake in r1206286. I can't 
find a place where inactive is set (was just grepping for "inactive").

>
> Detail, sticky can't be NULL here:
> +                if ((!b2->s->sticky || *b2->s->sticky == 0)
> +                    && b1->s->sticky && *b1->s->sticky)
{
> +                    PROXY_STRNCPY(b2->s->sticky_path, b1->s->sticky_path);
> +                    PROXY_STRNCPY(b2->s->sticky, b1->s->sticky);
> +                }
> +

Ok, it looks like the rest of mod_proxy does not check for s->sticky == 
NULL too, so I will remove it from this method too.

Regards,
Jan Kaluza

> I am (still) confused about the shallow copy:
>>>
>>>    +                *b2 = *b1;
>
> IIUC, Rüdiger's point is that base balancers' parameters shouldn't be
> modified by vhosts specifics, because some vhosts (or RewriteRules)
> may use the default ones.
> But then why would they share (at runtime) the same lbmethod, members
> list (dynamic with balancer-manager), backend connections (reslist,
> same timeouts, reusability, states, any worker parameter).
>
> IMHO there should be a deep copy here, that's another balancer, with
> its own mutexes, own members having their own parameters, own
> sockets...
 >
> Regards,
> Yann.
>


Mime
View raw message