On 12/12/2014 12:08 PM, Yann Ylavic wrote:
Hi Jan,
On Fri, Dec 12, 2014 at 9:44 AM, Jan Kaluža <jkal...@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.
