On 08/25/2015 11:39 AM, Plüm, Rüdiger, Vodafone Group wrote:
How about the following patch? It uses the server_rec of the server that
originally created the configuration item.
This looks like good strategy. I've verified that the patch fixes this
issue and does not break anything when testing briefly.
What do you think, Yann?
Note that "id_server" declaration is missing in the patch. Otherwise
it's OK.
Regards,
Jan Kaluza
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: dev@httpd.apache.org
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:jkal...@redhat.com]
Sent: Dienstag, 25. August 2015 10:23
To: dev@httpd.apache.org
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 <ylavic....@gmail.com>
wrote:
On Mon, Aug 24, 2015 at 4:47 PM, Jan Kaluža <jkal...@redhat.com>
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
