Re: svn commit: r1387603 - /httpd/httpd/trunk/modules/proxy/mod_proxy.c

[Jim Jagielski <ji...@jaguNET.com>]

I cannot think of one good reason why we've been doing the below...
I any case, I think vhosts inheriting these structs is a pretty
nasty bug, as well as memory hog...

On Sep 19, 2012, at 10:17 AM, jim@apache.org wrote:

> Author: jim
> Date: Wed Sep 19 14:17:03 2012
> New Revision: 1387603
> 
> URL: http://svn.apache.org/viewvc?rev=1387603&view=rev
> Log:
> wtf are we doing merging in these from the parent??
> These are server specific!
> 
> Modified:
>    httpd/httpd/trunk/modules/proxy/mod_proxy.c
> 
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c?rev=1387603&r1=1387602&r2=1387603&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Wed Sep 19 14:17:03 2012
> @@ -1173,13 +1173,13 @@ static void * merge_proxy_config(apr_poo
>     proxy_server_conf *base = (proxy_server_conf *) basev;
>     proxy_server_conf *overrides = (proxy_server_conf *) overridesv;
> 
> -    ps->proxies = apr_array_append(p, base->proxies, overrides->proxies);
> -    ps->sec_proxy = apr_array_append(p, base->sec_proxy, overrides->sec_proxy);
> -    ps->aliases = apr_array_append(p, base->aliases, overrides->aliases);
> -    ps->noproxies = apr_array_append(p, base->noproxies, overrides->noproxies);
> -    ps->dirconn = apr_array_append(p, base->dirconn, overrides->dirconn);
> -    ps->workers = apr_array_append(p, base->workers, overrides->workers);
> -    ps->balancers = apr_array_append(p, base->balancers, overrides->balancers);
> +    ps->proxies = overrides->proxies;
> +    ps->sec_proxy = overrides->sec_proxy;
> +    ps->aliases = overrides->aliases;
> +    ps->noproxies = overrides->noproxies;
> +    ps->dirconn = overrides->dirconn;
> +    ps->workers = overrides->workers;
> +    ps->balancers = overrides->balancers;
>     ps->forward = overrides->forward ? overrides->forward : base->forward;
>     ps->reverse = overrides->reverse ? overrides->reverse : base->reverse;
> 
> 
> 

RE: svn commit: r1387603 - /httpd/httpd/trunk/modules/proxy/mod_proxy.c

[Plüm, Rüdiger, Vodafone Group <ru...@vodafone.com>]

Not having looked into this in detail, but I believe you need this if
you want to have the balancer manager running in a different virtual host then your balancer
e.g. to expose this management interface only on a different port that might not be open to everyone.
At least with 2.2.x the way to do this is to define the balancers on global level and
use them via inheritance in the virtual hosts (one that uses the balancer and one that has the balancer
manager).
Having the balancer manager turned on in the "production" virtual host is likely a non-starter
for many people due to security concerns.

Regards

Rüdiger


> -----Original Message-----
> From: Jim Jagielski [mailto:jim@jaguNET.com]
> Sent: Mittwoch, 19. September 2012 16:34
> To: dev@httpd.apache.org
> Subject: Re: svn commit: r1387603 -
> /httpd/httpd/trunk/modules/proxy/mod_proxy.c
> 
> I cannot think of one good reason why we've been doing the below...
> I any case, I think vhosts inheriting these structs is a pretty
> nasty bug, as well as memory hog...
> 
> On Sep 19, 2012, at 10:17 AM, jim@apache.org wrote:
> 
> > Author: jim
> > Date: Wed Sep 19 14:17:03 2012
> > New Revision: 1387603
> >
> > URL: http://svn.apache.org/viewvc?rev=1387603&view=rev
> > Log:
> > wtf are we doing merging in these from the parent??
> > These are server specific!
> >
> > Modified:
> >    httpd/httpd/trunk/modules/proxy/mod_proxy.c
> >
> > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy.c
> > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy.c
> ?rev=1387603&r1=1387602&r2=1387603&view=diff
> >
> ========================================================================
> ======
> > --- httpd/httpd/trunk/modules/proxy/mod_proxy.c (original)
> > +++ httpd/httpd/trunk/modules/proxy/mod_proxy.c Wed Sep 19 14:17:03
> 2012
> > @@ -1173,13 +1173,13 @@ static void * merge_proxy_config(apr_poo
> >     proxy_server_conf *base = (proxy_server_conf *) basev;
> >     proxy_server_conf *overrides = (proxy_server_conf *) overridesv;
> >
> > -    ps->proxies = apr_array_append(p, base->proxies, overrides-
> >proxies);
> > -    ps->sec_proxy = apr_array_append(p, base->sec_proxy, overrides-
> >sec_proxy);
> > -    ps->aliases = apr_array_append(p, base->aliases, overrides-
> >aliases);
> > -    ps->noproxies = apr_array_append(p, base->noproxies, overrides-
> >noproxies);
> > -    ps->dirconn = apr_array_append(p, base->dirconn, overrides-
> >dirconn);
> > -    ps->workers = apr_array_append(p, base->workers, overrides-
> >workers);
> > -    ps->balancers = apr_array_append(p, base->balancers, overrides-
> >balancers);
> > +    ps->proxies = overrides->proxies;
> > +    ps->sec_proxy = overrides->sec_proxy;
> > +    ps->aliases = overrides->aliases;
> > +    ps->noproxies = overrides->noproxies;
> > +    ps->dirconn = overrides->dirconn;
> > +    ps->workers = overrides->workers;
> > +    ps->balancers = overrides->balancers;
> >     ps->forward = overrides->forward ? overrides->forward : base-
> >forward;
> >     ps->reverse = overrides->reverse ? overrides->reverse : base-
> >reverse;
> >
> >
> >

Re: svn commit: r1387603 - /httpd/httpd/trunk/modules/proxy/mod_proxy.c

[Jim Jagielski <ji...@jaguNET.com>]

Yeah, ever since we started moving many of these params to
shared memory, this has been broken. As such, even without
the persist, using the balancer-manager to change params
causes some unknown and un-seen interactions.

I propose that we close this bug in 2.4.x.

On Sep 19, 2012, at 11:45 AM, Jim Jagielski <ji...@jagunet.com> wrote:

> The issue is that muddies the waters as far as "whose balancer
> is this"... For example, let's assume you define balancer://foo
> at the top level and it is inherited by vhost1 and vhost2.
> 
> If you change a balancer setting in vhost1, should that change
> be automatically made to the one in vhost2? Or is it specific
> to vhost1?
> 
> Personally, I feel that the inheritance is majorly wrong and a
> major bug. Now that we are allowing realtime changes to these
> params, we need to restrict them to per-server, which means
> no inheritance.
> 
> On Sep 19, 2012, at 10:45 AM, Tom Evans <te...@googlemail.com> wrote:
> 
>> On Wed, Sep 19, 2012 at 3:34 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>> I cannot think of one good reason why we've been doing the below...
>>> I any case, I think vhosts inheriting these structs is a pretty
>>> nasty bug, as well as memory hog...
>> 
>> Hi Jim
>> 
>> Is one possible use case for this defining balancers outside of
>> vhosts, and then using them in multiple vhosts? (Personally, I hate
>> this feature, it clutters up unrelated balancer sets on
>> balancer-manager).
>> 
>> 
>> Cheers
>> 
>> Tom
>> 
> 

Re: svn commit: r1387603 - /httpd/httpd/trunk/modules/proxy/mod_proxy.c

[Jim Jagielski <ji...@jaguNET.com>]

The issue is that muddies the waters as far as "whose balancer
is this"... For example, let's assume you define balancer://foo
at the top level and it is inherited by vhost1 and vhost2.

If you change a balancer setting in vhost1, should that change
be automatically made to the one in vhost2? Or is it specific
to vhost1?

Personally, I feel that the inheritance is majorly wrong and a
major bug. Now that we are allowing realtime changes to these
params, we need to restrict them to per-server, which means
no inheritance.

On Sep 19, 2012, at 10:45 AM, Tom Evans <te...@googlemail.com> wrote:

> On Wed, Sep 19, 2012 at 3:34 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>> I cannot think of one good reason why we've been doing the below...
>> I any case, I think vhosts inheriting these structs is a pretty
>> nasty bug, as well as memory hog...
> 
> Hi Jim
> 
> Is one possible use case for this defining balancers outside of
> vhosts, and then using them in multiple vhosts? (Personally, I hate
> this feature, it clutters up unrelated balancer sets on
> balancer-manager).
> 
> 
> Cheers
> 
> Tom
> 

Re: svn commit: r1387603 - /httpd/httpd/trunk/modules/proxy/mod_proxy.c

[Tom Evans <te...@googlemail.com>]

On Wed, Sep 19, 2012 at 3:34 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> I cannot think of one good reason why we've been doing the below...
> I any case, I think vhosts inheriting these structs is a pretty
> nasty bug, as well as memory hog...

Hi Jim

Is one possible use case for this defining balancers outside of
vhosts, and then using them in multiple vhosts? (Personally, I hate
this feature, it clutters up unrelated balancer sets on
balancer-manager).


Cheers

Tom