Of course it requires a minor bump. Regards
Rüdiger > -----Original Message----- > From: Plüm, Rüdiger, Vodafone Group > Sent: Dienstag, 25. August 2015 11:39 > To: [email protected] > Subject: RE: PR 58267: Regression in 2.2.31 caused by r1680920 > > How about the following patch? It uses the server_rec of the server that > originally created the configuration item. > > Index: modules/proxy/proxy_util.c > =================================================================== > --- modules/proxy/proxy_util.c (revision 1697578) > +++ modules/proxy/proxy_util.c (working copy) > @@ -1460,6 +1460,7 @@ > (*worker)->flush_packets = flush_off; > (*worker)->flush_wait = PROXY_FLUSH_WAIT; > (*worker)->smax = -1; > + (*worker)->server = conf->s; > /* Increase the total worker count */ > proxy_lb_workers++; > init_conn_pool(p, *worker); > @@ -1824,14 +1829,20 @@ > apr_md5_update(&ctx, (unsigned char *)balancer->name, > strlen(balancer->name)); > } > - if (server) { > + if (worker->server) { > + id_server = worker->server; > + } > + else { > + id_server = server; > + } > + if (id_server) { > server_addr_rec *addr; > /* Assumes the unique identifier of a vhost is its > address(es) > * plus the ServerName:Port. Should two or more vhosts have > this > * same identifier, the first one would always be elected to > * handle the requests, so this shouldn't be an issue... > */ > - for (addr = server->addrs; addr; addr = addr->next) { > + for (addr = id_server->addrs; addr; addr = addr->next) { > char host_ip[64]; /* for any IPv[46] string */ > apr_sockaddr_ip_getbuf(host_ip, sizeof host_ip, > addr->host_addr); > @@ -1840,10 +1851,10 @@ > apr_md5_update(&ctx, (unsigned char *)&addr->host_port, > sizeof(addr->host_port)); > } > - apr_md5_update(&ctx, (unsigned char *)server- > >server_hostname, > - strlen(server->server_hostname)); > - apr_md5_update(&ctx, (unsigned char *)&server->port, > - sizeof(server->port)); > + apr_md5_update(&ctx, (unsigned char *)id_server- > >server_hostname, > + strlen(id_server->server_hostname)); > + apr_md5_update(&ctx, (unsigned char *)&id_server->port, > + sizeof(id_server->port)); > } > apr_md5_final(digest, &ctx); > > Index: modules/proxy/mod_proxy.c > =================================================================== > --- modules/proxy/mod_proxy.c (revision 1697578) > +++ modules/proxy/mod_proxy.c (working copy) > @@ -1129,6 +1129,7 @@ > ps->badopt = bad_error; > ps->badopt_set = 0; > ps->pool = p; > + ps->s = s; > > return ps; > } > @@ -1172,6 +1173,7 @@ > ps->proxy_status = (overrides->proxy_status_set == 0) ? base- > >proxy_status : overrides->proxy_status; > ps->proxy_status_set = overrides->proxy_status_set || base- > >proxy_status_set; > ps->pool = p; > + ps->s = overrides->s; > return ps; > } > > Index: modules/proxy/mod_proxy.h > =================================================================== > --- modules/proxy/mod_proxy.h (revision 1697578) > +++ modules/proxy/mod_proxy.h (working copy) > @@ -193,6 +193,7 @@ > } proxy_status; /* Status display options */ > char proxy_status_set; > apr_pool_t *pool; /* Pool used for allocating this struct > */ > + server_rec *s; /* The server_rec where this > configuration was created in */ > } proxy_server_conf; > > > @@ -369,6 +370,7 @@ > char disablereuse_set; > apr_interval_time_t conn_timeout; > char conn_timeout_set; > + server_rec *server; /* The server_rec where this > configuration was created in */ > }; > > /* > > > Regards > > Rüdiger > > > -----Original Message----- > > From: Plüm, Rüdiger, Vodafone Group > > Sent: Dienstag, 25. August 2015 10:48 > > To: [email protected] > > Subject: RE: PR 58267: Regression in 2.2.31 caused by r1680920 > > > > I think the current state of 2.2.31 breaks existing 2.2.x configuration > > prior to 2.2.31. > > Prior to 2.2.31 you could do the following: > > > > <Proxy Balancer://proxy1> > > BalancerMember ajp://127.0.0.1:7001 > > BalancerMember ajp://127.0.0.2:7002 > > </Proxy> > > > > <virtualhost *:80> > > ProxyPass / balancer://proxy1/ > > </virtualhost> > > > > <virtualhost 127.0.0.1:8080> > > > > <Location /bmanager> > > sethandler balancer-manager > > </Location> > > > > </virtualhost> > > > > With this configuration you could provide your reverse proxy to the > > outside world while having the > > balancermanager managing the balancer only listening to localhost. This > is > > not possible any longer with 2.2.31 > > as the worker->s is now different in each virtualhost whereas before it > > was the same as the worker->id was used > > to identify the scoreboard slot. > > The patches proposed so far would not change this. I think in order to > > revert to the old behaviour we need to > > store with each worker in which server_rec context it was created. e.g. > > adding a const char * field to the worker that would be filled with > > server->server_hostname. Then we could use this value for creating the > > md5. > > > > Regards > > > > Rüdiger > > > > > -----Original Message----- > > > From: Jan Kaluža [mailto:[email protected]] > > > Sent: Dienstag, 25. August 2015 10:23 > > > To: [email protected] > > > Subject: Re: PR 58267: Regression in 2.2.31 caused by r1680920 > > > > > > On 08/24/2015 11:12 PM, Yann Ylavic wrote: > > > > On Mon, Aug 24, 2015 at 5:51 PM, Yann Ylavic <[email protected]> > > > wrote: > > > >> > > > >> On Mon, Aug 24, 2015 at 4:47 PM, Jan Kaluža <[email protected]> > > wrote: > > > >>> > > > >>> 2) Increment proxy_lb_workers according to number of workers in > > > balancer > > > >>> when using "ProxyPass /foobar/ Balancer://foobar/" in the > > VirtualHost. > > > The > > > >>> scoreboard would have right size and ap_proxy_set_scoreboard_lb > > would > > > not > > > >>> fail then. > > > >> > > > >> I think we can do this quite easily in merge_proxy_config(), by > > > >> incrementing proxy_lb_workers for each base->balancers->workers. I > > did > > > >> not test it yet though. > > > > > > > > I tested the below which seems to work. > > > > > > Hm, this reserves the slots in scoreboard even when the balancers are > > > not used in the virtualhost, or am I wrong? > > > > > > I originally thought about increasing proxy_lb_workers only when they > > > are used in the ProxyPass* in the vhost. > > > > > > > Index: modules/proxy/mod_proxy.c > > > > =================================================================== > > > > --- modules/proxy/mod_proxy.c (revision 1697358) > > > > +++ modules/proxy/mod_proxy.c (working copy) > > > > @@ -1135,6 +1135,7 @@ static void * create_proxy_config(apr_pool_t > *p, > > s > > > > > > > > static void * merge_proxy_config(apr_pool_t *p, void *basev, void > > > *overridesv) > > > > { > > > > + int i; > > > > proxy_server_conf *ps = apr_pcalloc(p, > > sizeof(proxy_server_conf)); > > > > proxy_server_conf *base = (proxy_server_conf *) basev; > > > > proxy_server_conf *overrides = (proxy_server_conf *) > overridesv; > > > > @@ -1147,6 +1148,13 @@ static void * merge_proxy_config(apr_pool_t > *p, > > > vo > > > > ps->allowed_connect_ports = apr_array_append(p, > > > > base->allowed_connect_ports, overrides->allowed_connect_ports); > > > > ps->workers = apr_array_append(p, base->workers, overrides- > > > >workers); > > > > ps->balancers = apr_array_append(p, base->balancers, > overrides- > > > >balancers); > > > > + /* The balancers inherited from base don't have their members > > > reserved on > > > > + * the scorebord_lb for this server, account for them now. > > > > + */ > > > > + for (i = 0; i < base->balancers->nelts; ++i) { > > > > + proxy_balancer *balancer = (proxy_balancer *)base- > >balancers- > > > >elts + i; > > > > + proxy_lb_workers += balancer->workers->nelts; > > > > + } > > > > ps->forward = overrides->forward ? overrides->forward : base- > > > >forward; > > > > ps->reverse = overrides->reverse ? overrides->reverse : base- > > > >reverse; > > > > > > > > -- > > > > > > > > Please note that since all the workers would really be accounted in > > > > the scoreboard, configurations like the one of PR 58267 (with > > > > inherited balancers) would also need bigger SHMs (but no more) than > > > > currently... > > > > > > > > WDYT? > > > > > > > > > > Regards, > > > Jan Kaluza > > >
