You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by yl...@apache.org on 2015/08/21 14:34:03 UTC
svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Author: ylavic
Date: Fri Aug 21 12:34:02 2015
New Revision: 1696960
URL: http://svn.apache.org/r1696960
Log:
mod_proxy_balancer: Fix balancers and balancer members reuse on
restart when new ones are added. PR 58024.
Since slotmem_create() issues a strict check on the size of an existing
slot before reusing it, it won't reuse existing balancers and members when
new ones are added during restart (whereas growth margins would allow it).
Fix this by using slotmem_attach() first and if it succeeds do the checks
based on the returned previous number of existing entries.
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/docs/log-message-tags/next-number
httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
@@ -1,6 +1,9 @@
-*- coding: utf-8 -*-
Changes with Apache 2.5.0
+ *) mod_proxy_balancer: Fix balancers and balancer members reuse on
+ restart when new ones are added. PR 58024. [Yann Ylavic]
+
*) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
long to keep idle connections with the memcache server(s).
Change default value from 600 usec (!) to 15 sec. PR 58091
Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
+++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
@@ -1 +1 @@
-2964
+2966
Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
@@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
char *id;
proxy_balancer *balancer;
ap_slotmem_type_t type;
+ apr_size_t attached_size;
+ unsigned int attached_num;
void *sconf = s->module_config;
conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
+
/*
* During create_proxy_config() we created a dummy id. Now that
* we have identifying info, we can create the real id
@@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
(int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
(int)conf->balancers->nelts, conf->max_balancers);
- rv = storage->create(&new, conf->id,
- ALIGNED_PROXY_BALANCER_SHARED_SIZE,
- conf->max_balancers, type, pconf);
+ /* First try to attach() since the number of configured balancers
+ * may have changed during restart, and we don't want create() to
+ * fail because the overall size * number of entries is not stricly
+ * identical to the previous run. There may still be enough room
+ * for this new run thanks to bgrowth margin, so if attach()
+ * succeeds we can only check for the number of available entries
+ * to be *greater or* equal to what we need now. If attach() fails
+ * we simply fall back to create().
+ */
+ rv = storage->attach(&new, conf->id,
+ &attached_size, &attached_num,
+ pconf);
+ if (rv != APR_SUCCESS) {
+ rv = storage->create(&new, conf->id,
+ ALIGNED_PROXY_BALANCER_SHARED_SIZE,
+ conf->max_balancers, type, pconf);
+ }
+ else {
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
+ "Balancers attached: %d, %d (%d)",
+ (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
+ (int)attached_num, conf->max_balancers);
+ if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
+ && attached_num >= conf->balancers->nelts) {
+ conf->max_balancers = attached_num;
+ }
+ else {
+ rv = APR_ENOSPC;
+ }
+ }
if (rv != APR_SUCCESS) {
- ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
+ ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
+ "balancer slotmem create or attach failed");
return !OK;
}
conf->bslot = new;
@@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
(int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
(int)balancer->max_workers, i);
- rv = storage->create(&new, balancer->s->sname,
- ALIGNED_PROXY_WORKER_SHARED_SIZE,
- balancer->max_workers, type, pconf);
+ /* try to attach first (see rationale from balancers above) */
+ rv = storage->attach(&new, balancer->s->sname,
+ &attached_size, &attached_num,
+ pconf);
+ if (rv != APR_SUCCESS) {
+ rv = storage->create(&new, balancer->s->sname,
+ ALIGNED_PROXY_WORKER_SHARED_SIZE,
+ balancer->max_workers, type, pconf);
+ }
+ else {
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
+ "Workers attached: %s (%s), %d, %d (%d) [%u]",
+ balancer->s->name, balancer->s->sname,
+ (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
+ (int)attached_num, balancer->max_workers, i);
+ if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
+ && attached_num >= balancer->workers->nelts) {
+ balancer->max_workers = attached_num;
+ }
+ else {
+ rv = APR_ENOSPC;
+ }
+ }
if (rv != APR_SUCCESS) {
- ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
+ ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
+ "worker slotmem create or attach failed");
return !OK;
}
balancer->wslot = new;
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Aug 21, 2015 at 8:07 PM, Ruediger Pluem <rp...@apache.org> wrote:
>
>
> On 08/21/2015 07:02 PM, Yann Ylavic wrote:
>> On Fri, Aug 21, 2015 at 4:15 PM, Ruediger Pluem <rp...@apache.org> wrote:
>>>
>>> Hm. Doesn't that mean that if I keep on adding stuff and do graceful restarts that at one point in time I will have used
>>> up all my growth and need to do a hard restart?
>>
>> Yes, the last restart would fail.
>>
>>> If yes, it should be probably documented to avoid some unexpected surprises.
>>
>> I agree it should be documented (will do), currently we already have
>> this issue from the first restart though.
>
> So it is an improvement over what we have :-)
Yes, including the doc :)
Added in r1697352, and to the backport proposal.
Thanks,
Yann.
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 08/21/2015 07:02 PM, Yann Ylavic wrote:
> On Fri, Aug 21, 2015 at 4:15 PM, Ruediger Pluem <rp...@apache.org> wrote:
>>
>> Hm. Doesn't that mean that if I keep on adding stuff and do graceful restarts that at one point in time I will have used
>> up all my growth and need to do a hard restart?
>
> Yes, the last restart would fail.
>
>> If yes, it should be probably documented to avoid some unexpected surprises.
>
> I agree it should be documented (will do), currently we already have
> this issue from the first restart though.
So it is an improvement over what we have :-)
Regards
Rüdiger
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Aug 21, 2015 at 4:15 PM, Ruediger Pluem <rp...@apache.org> wrote:
>
> Hm. Doesn't that mean that if I keep on adding stuff and do graceful restarts that at one point in time I will have used
> up all my growth and need to do a hard restart?
Yes, the last restart would fail.
> If yes, it should be probably documented to avoid some unexpected surprises.
I agree it should be documented (will do), currently we already have
this issue from the first restart though.
Regards,
Yann.
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 08/21/2015 02:34 PM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Fri Aug 21 12:34:02 2015
> New Revision: 1696960
>
> URL: http://svn.apache.org/r1696960
> Log:
> mod_proxy_balancer: Fix balancers and balancer members reuse on
> restart when new ones are added. PR 58024.
>
> Since slotmem_create() issues a strict check on the size of an existing
> slot before reusing it, it won't reuse existing balancers and members when
> new ones are added during restart (whereas growth margins would allow it).
> Fix this by using slotmem_attach() first and if it succeeds do the checks
> based on the returned previous number of existing entries.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/docs/log-message-tags/next-number
> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
> @@ -1,6 +1,9 @@
> -*- coding: utf-8 -*-
> Changes with Apache 2.5.0
>
> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
> + restart when new ones are added. PR 58024. [Yann Ylavic]
> +
> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
> long to keep idle connections with the memcache server(s).
> Change default value from 600 usec (!) to 15 sec. PR 58091
>
> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
> @@ -1 +1 @@
> -2964
> +2966
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
> char *id;
> proxy_balancer *balancer;
> ap_slotmem_type_t type;
> + apr_size_t attached_size;
> + unsigned int attached_num;
> void *sconf = s->module_config;
> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
> +
> /*
> * During create_proxy_config() we created a dummy id. Now that
> * we have identifying info, we can create the real id
> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> (int)conf->balancers->nelts, conf->max_balancers);
>
> - rv = storage->create(&new, conf->id,
> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> - conf->max_balancers, type, pconf);
> + /* First try to attach() since the number of configured balancers
> + * may have changed during restart, and we don't want create() to
> + * fail because the overall size * number of entries is not stricly
> + * identical to the previous run. There may still be enough room
> + * for this new run thanks to bgrowth margin, so if attach()
> + * succeeds we can only check for the number of available entries
> + * to be *greater or* equal to what we need now. If attach() fails
> + * we simply fall back to create().
> + */
> + rv = storage->attach(&new, conf->id,
> + &attached_size, &attached_num,
> + pconf);
> + if (rv != APR_SUCCESS) {
> + rv = storage->create(&new, conf->id,
> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> + conf->max_balancers, type, pconf);
> + }
> + else {
> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
> + "Balancers attached: %d, %d (%d)",
> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> + (int)attached_num, conf->max_balancers);
> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
> + && attached_num >= conf->balancers->nelts) {
> + conf->max_balancers = attached_num;
> + }
> + else {
> + rv = APR_ENOSPC;
Hm. Doesn't that mean that if I keep on adding stuff and do graceful restarts that at one point in time I will have used
up all my growth and need to do a hard restart?
If yes, it should be probably documented to avoid some unexpected surprises.
Regards
Rüdiger
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
I don't understand this at all... I add new balancers and members
all the time and do restarts and power cycles and never lose anything.
What do you mean by some sort of size check failing?? The size of
the slot should *never* change.
> On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
>
> Author: ylavic
> Date: Fri Aug 21 12:34:02 2015
> New Revision: 1696960
>
> URL: http://svn.apache.org/r1696960
> Log:
> mod_proxy_balancer: Fix balancers and balancer members reuse on
> restart when new ones are added. PR 58024.
>
> Since slotmem_create() issues a strict check on the size of an existing
> slot before reusing it, it won't reuse existing balancers and members when
> new ones are added during restart (whereas growth margins would allow it).
> Fix this by using slotmem_attach() first and if it succeeds do the checks
> based on the returned previous number of existing entries.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/docs/log-message-tags/next-number
> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
> @@ -1,6 +1,9 @@
> -*- coding: utf-8 -*-
> Changes with Apache 2.5.0
>
> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
> + restart when new ones are added. PR 58024. [Yann Ylavic]
> +
> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
> long to keep idle connections with the memcache server(s).
> Change default value from 600 usec (!) to 15 sec. PR 58091
>
> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
> @@ -1 +1 @@
> -2964
> +2966
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
> char *id;
> proxy_balancer *balancer;
> ap_slotmem_type_t type;
> + apr_size_t attached_size;
> + unsigned int attached_num;
> void *sconf = s->module_config;
> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
> +
> /*
> * During create_proxy_config() we created a dummy id. Now that
> * we have identifying info, we can create the real id
> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> (int)conf->balancers->nelts, conf->max_balancers);
>
> - rv = storage->create(&new, conf->id,
> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> - conf->max_balancers, type, pconf);
> + /* First try to attach() since the number of configured balancers
> + * may have changed during restart, and we don't want create() to
> + * fail because the overall size * number of entries is not stricly
> + * identical to the previous run. There may still be enough room
> + * for this new run thanks to bgrowth margin, so if attach()
> + * succeeds we can only check for the number of available entries
> + * to be *greater or* equal to what we need now. If attach() fails
> + * we simply fall back to create().
> + */
> + rv = storage->attach(&new, conf->id,
> + &attached_size, &attached_num,
> + pconf);
> + if (rv != APR_SUCCESS) {
> + rv = storage->create(&new, conf->id,
> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> + conf->max_balancers, type, pconf);
> + }
> + else {
> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
> + "Balancers attached: %d, %d (%d)",
> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> + (int)attached_num, conf->max_balancers);
> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
> + && attached_num >= conf->balancers->nelts) {
> + conf->max_balancers = attached_num;
> + }
> + else {
> + rv = APR_ENOSPC;
> + }
> + }
> if (rv != APR_SUCCESS) {
> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
> + "balancer slotmem create or attach failed");
> return !OK;
> }
> conf->bslot = new;
> @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
> (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
> (int)balancer->max_workers, i);
>
> - rv = storage->create(&new, balancer->s->sname,
> - ALIGNED_PROXY_WORKER_SHARED_SIZE,
> - balancer->max_workers, type, pconf);
> + /* try to attach first (see rationale from balancers above) */
> + rv = storage->attach(&new, balancer->s->sname,
> + &attached_size, &attached_num,
> + pconf);
> + if (rv != APR_SUCCESS) {
> + rv = storage->create(&new, balancer->s->sname,
> + ALIGNED_PROXY_WORKER_SHARED_SIZE,
> + balancer->max_workers, type, pconf);
> + }
> + else {
> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
> + "Workers attached: %s (%s), %d, %d (%d) [%u]",
> + balancer->s->name, balancer->s->sname,
> + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
> + (int)attached_num, balancer->max_workers, i);
> + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
> + && attached_num >= balancer->workers->nelts) {
> + balancer->max_workers = attached_num;
> + }
> + else {
> + rv = APR_ENOSPC;
> + }
> + }
> if (rv != APR_SUCCESS) {
> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
> + "worker slotmem create or attach failed");
> return !OK;
> }
> balancer->wslot = new;
>
>
RE: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Plüm,
Rüdiger,
Vodafone Group <ru...@vodafone.com>.
Is it possible that Jim is talking about adding new stuff via the balancer manager and Yann about adding new stuff to the config file and doing a graceful restart?
Regards
Rüdiger
> -----Original Message-----
> From: Jim Jagielski [mailto:jim@jaguNET.com]
> Sent: Dienstag, 25. August 2015 14:12
> To: dev@httpd.apache.org
> Subject: Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
> docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
>
> How and when do ALIGNED_PROXY_BALANCER_SHARED_SIZE and conf->max_balancers
> change???
>
> > On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
> >
> > Author: ylavic
> > Date: Fri Aug 21 12:34:02 2015
> > New Revision: 1696960
> >
> > URL: http://svn.apache.org/r1696960
> > Log:
> > mod_proxy_balancer: Fix balancers and balancer members reuse on
> > restart when new ones are added. PR 58024.
> >
> > Since slotmem_create() issues a strict check on the size of an existing
> > slot before reusing it, it won't reuse existing balancers and members
> when
> > new ones are added during restart (whereas growth margins would allow
> it).
> > Fix this by using slotmem_attach() first and if it succeeds do the
> checks
> > based on the returned previous number of existing entries.
> >
> > Modified:
> > httpd/httpd/trunk/CHANGES
> > httpd/httpd/trunk/docs/log-message-tags/next-number
> > httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
> >
> > Modified: httpd/httpd/trunk/CHANGES
> > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696
> 959&r2=1696960&view=diff
> >
> ==========================================================================
> ====
> > --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> > +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
> > @@ -1,6 +1,9 @@
> > -*- coding:
> utf-8 -*-
> > Changes with Apache 2.5.0
> >
> > + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
> > + restart when new ones are added. PR 58024. [Yann Ylavic]
> > +
> > *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to
> control how
> > long to keep idle connections with the memcache server(s).
> > Change default value from 600 usec (!) to 15 sec. PR 58091
> >
> > Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
> > URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-
> tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
> >
> ==========================================================================
> ====
> > --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
> > +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21
> 12:34:02 2015
> > @@ -1 +1 @@
> > -2964
> > +2966
> >
> > Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
> > URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_bal
> ancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
> >
> ==========================================================================
> ====
> > --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
> > +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21
> 12:34:02 2015
> > @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
> > char *id;
> > proxy_balancer *balancer;
> > ap_slotmem_type_t type;
> > + apr_size_t attached_size;
> > + unsigned int attached_num;
> > void *sconf = s->module_config;
> > conf = (proxy_server_conf *)ap_get_module_config(sconf,
> &proxy_module);
> > +
> > /*
> > * During create_proxy_config() we created a dummy id. Now that
> > * we have identifying info, we can create the real id
> > @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
> > (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> > (int)conf->balancers->nelts, conf-
> >max_balancers);
> >
> > - rv = storage->create(&new, conf->id,
> > - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> > - conf->max_balancers, type, pconf);
> > + /* First try to attach() since the number of configured
> balancers
> > + * may have changed during restart, and we don't want
> create() to
> > + * fail because the overall size * number of entries is not
> stricly
> > + * identical to the previous run. There may still be
> enough room
> > + * for this new run thanks to bgrowth margin, so if
> attach()
> > + * succeeds we can only check for the number of available
> entries
> > + * to be *greater or* equal to what we need now. If
> attach() fails
> > + * we simply fall back to create().
> > + */
> > + rv = storage->attach(&new, conf->id,
> > + &attached_size, &attached_num,
> > + pconf);
> > + if (rv != APR_SUCCESS) {
> > + rv = storage->create(&new, conf->id,
> > +
> ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> > + conf->max_balancers, type, pconf);
> > + }
> > + else {
> > + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> APLOGNO(02964)
> > + "Balancers attached: %d, %d (%d)",
> > + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> > + (int)attached_num, conf->max_balancers);
> > + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
> > + && attached_num >= conf->balancers->nelts) {
> > + conf->max_balancers = attached_num;
> > + }
> > + else {
> > + rv = APR_ENOSPC;
> > + }
> > + }
> > if (rv != APR_SUCCESS) {
> > - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s,
> APLOGNO(01179) "balancer slotmem_create failed");
> > + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s,
> APLOGNO(01179)
> > + "balancer slotmem create or attach
> failed");
> > return !OK;
> > }
> > conf->bslot = new;
> > @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
> > (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
> > (int)balancer->max_workers, i);
> >
> > - rv = storage->create(&new, balancer->s->sname,
> > - ALIGNED_PROXY_WORKER_SHARED_SIZE,
> > - balancer->max_workers, type, pconf);
> > + /* try to attach first (see rationale from balancers above)
> */
> > + rv = storage->attach(&new, balancer->s->sname,
> > + &attached_size, &attached_num,
> > + pconf);
> > + if (rv != APR_SUCCESS) {
> > + rv = storage->create(&new, balancer->s->sname,
> > + ALIGNED_PROXY_WORKER_SHARED_SIZE,
> > + balancer->max_workers, type,
> pconf);
> > + }
> > + else {
> > + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s,
> APLOGNO(02965)
> > + "Workers attached: %s (%s), %d, %d (%d)
> [%u]",
> > + balancer->s->name, balancer->s->sname,
> > + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
> > + (int)attached_num, balancer->max_workers,
> i);
> > + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
> > + && attached_num >= balancer->workers->nelts) {
> > + balancer->max_workers = attached_num;
> > + }
> > + else {
> > + rv = APR_ENOSPC;
> > + }
> > + }
> > if (rv != APR_SUCCESS) {
> > - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s,
> APLOGNO(01185) "worker slotmem_create failed");
> > + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s,
> APLOGNO(01185)
> > + "worker slotmem create or attach failed");
> > return !OK;
> > }
> > balancer->wslot = new;
> >
> >
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Aug 26, 2015 at 3:32 AM, Daniel Ruggeri <DR...@primary.net> wrote:
> On 8/25/2015 10:11 AM, Jim Jagielski wrote:
>> Again, if the slotmem exists and is persisted, the assumption
>> is that THAT is what the admin wants, and when Apache restarts,
>> THAT is the running config they desire. If there are changes
>> to the reverse proxy setup, the assumption must be we are
>> starting from scratch; There are, iirc, a number of places where
>> these kinds of checks are done. Trying to somehow "merge" a just-changed
>> file config and a running config (based on an older file config
>> with dynamic changes) is nasty and tough to do correctly.
>
> This is a common problem with serialization. The persisted config is
> serialized data of what is in memory. It can only represent the
> "version" of the conf file that created it. If you change the conf,
> deserialization should fail because there could be other material
> changes that might get in the way.
>
> I'm +1 on the idea that using bgrowth space to add stuff via
> conf+graceful restart should be avoided.
Just to be precise ( and maybe give this change a very last chance :p
), this commit does not use arbitrary bgrowth (or growth) space for
conf+graceful restarts.
The reused slots are still strictly checked against the item_size
(unchanged), such that they are garanteed to contain balancers (or
members) of the vhost, not any other slot-able object.
That's only the check on item_num which is relaxed to verify that it
is *greater or equal* to the number of newly loaded balancers (or
members), whereas the original check was *strictly equal* here too.
My point is that this commit doesn't introduce any issue wrt
arbitrary/unrelated/corrupted slots' reuse on restart, we are really
talking about free space available strictly for balancers (or members)
via the balancer-manager (and the config-file, hopefully).
This means that an admin using the balancer-manager (UI) sees the
changes, is able to manage all the original or reloaded balancers (or
members), and can still add new ones if the slots are not full.
IOW, this commit looks compatible (and useful), to me...
OTOH, forbidding "compatible" changes via the config-file looks quite
unusual (most people configure httpd via the config-file only, I
guess).
But hey, I'm not the community :)
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Daniel Ruggeri <DR...@primary.net>.
On 8/25/2015 10:11 AM, Jim Jagielski wrote:
> Again, if the slotmem exists and is persisted, the assumption
> is that THAT is what the admin wants, and when Apache restarts,
> THAT is the running config they desire. If there are changes
> to the reverse proxy setup, the assumption must be we are
> starting from scratch; There are, iirc, a number of places where
> these kinds of checks are done. Trying to somehow "merge" a just-changed
> file config and a running config (based on an older file config
> with dynamic changes) is nasty and tough to do correctly.
This is a common problem with serialization. The persisted config is
serialized data of what is in memory. It can only represent the
"version" of the conf file that created it. If you change the conf,
deserialization should fail because there could be other material
changes that might get in the way.
I'm +1 on the idea that using bgrowth space to add stuff via
conf+graceful restart should be avoided.
--
Daniel Ruggeri
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
> On Aug 25, 2015, at 10:41 AM, Yann Ylavic <yl...@gmail.com> wrote:
>
> I don't see the difference between the configuration via
> balancer-manager or file+graceful-restart, both can change the number
> of balancers/members AND the settings while children are running.
> I'd kind of agree if we were talking about non-graceful restarts or
> first startup, we could add a new storage->destroy() method to do the
> job depending on the slotmem provider (though what about remote
> storage shared between multiple httpds?).
> Currently any restart (graceful or not) reuses the existing slots,
As it should, and as I designed it, yeah. The whole assumption
is that restarts DO NOT CHANGE THE REVERSE PROXY CONFIG.
> even if the settings changed (but the size of the slot prior to this
> commit), so your proposed changes may break compatibility too...
>
Please note how the runtime state from a persisted slotmem
is re-used and how/when/if it is over-written or over-writes.
Please don't change the behavior of what it is currently doing
based on the feedback and insights and discussions of years
to what you think it should be doing, or what you want it to
do. If it's broke, fix it. But don't just change it. :)
Again, if the slotmem exists and is persisted, the assumption
is that THAT is what the admin wants, and when Apache restarts,
THAT is the running config they desire. If there are changes
to the reverse proxy setup, the assumption must be we are
starting from scratch; There are, iirc, a number of places where
these kinds of checks are done. Trying to somehow "merge" a just-changed
file config and a running config (based on an older file config
with dynamic changes) is nasty and tough to do correctly.
>
> On Tue, Aug 25, 2015 at 3:59 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>> When 1st configured, Apache sees how many balancers there are
>> set in the config file. It then pads that amount by growth to
>> allow for the to-be-added dynamic addition of new balancers via
>> the balancer manager (ala adding members).
>>
>> The assumption is that if the actual config file is changed,
>> then the runtime configuration of balancers and members
>> is no longer valid; after all, if someone is changing the
>> config file, they could have done the same to add/change
>> bal/mem settings. So a config file change means something
>> more "serious" at which point the dynamic config should be
>> considered void and Apache starts with a clean (dynamic) slate
>> and just uses what's in the config only.
>>
>>> On Aug 25, 2015, at 9:48 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>
>>> Hm, what's the point of the balancers and balancer->workers growth margin then?
>>> The first time the configration is loaded we create enough room for
>>> adding new balancers/workers, and on (graceful) restart we wouldn't
>>> use this space?
>>> That's also how the balancer-manager works when adding a new worker to
>>> a balancer, AFAICT.
>>>
>>>
>>> On Tue, Aug 25, 2015 at 3:43 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>> You mean via changing the config file? If so, then all bets
>>>> are off. You change the config file and restart then Apache
>>>> should start off w/ a clean slate. Even so, you need to handle
>>>> freeing the old shared mem segment and creating a brand new one
>>>> with the "correct" size... Trying to "use" the old one is
>>>> just fraught with issues.
>>>>
>>>>
>>>>> On Aug 25, 2015, at 9:15 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>
>>>>> Well, if one adds a new balancer before restarting, at post_config
>>>>> time conf->balancers->nelts is +1 wrt the previous load...
>>>>>
>>>>> On Tue, Aug 25, 2015 at 2:46 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>>> But the number of balancers does not change yet... And when it
>>>>>> is implemented the overall size of the storage slot should *never*
>>>>>> change. It must be constant, ala the scoreboard.
>>>>>>
>>>>>>> On Aug 25, 2015, at 8:39 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>>>
>>>>>>> At every startup, in balancer_post_config:
>>>>>>> conf->max_balancers = conf->balancers->nelts + conf->bgrowth;
>>>>>>>
>>>>>>> which means that if you add a balancer and restart, storage->create()
>>>>>>> (eg. slotmem_create) will try to find an existing slot with this ID
>>>>>>> and the exact ALIGNED_PROXY_BALANCER_SHARED_SIZE * conf->max_balancers
>>>>>>> size, and fail:
>>>>>>> if (apr_shm_size_get(shm) != size) {
>>>>>>> apr_shm_detach(shm);
>>>>>>> ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(02599)
>>>>>>> "existing shared memory for %s could not be used
>>>>>>> (failed size check)",
>>>>>>> fname);
>>>>>>> return APR_EINVAL;
>>>>>>> }
>>>>>>>
>>>>>>> By using storage->attach() first (which is based on the ID only and
>>>>>>> returns the existing item_size and item_num), we can do the check
>>>>>>> ourselves and fail only if conf->max_balancers > item_num, allowing
>>>>>>> for balancers to be added thanks to growth margin used at startup.
>>>>>>>
>>>>>>> On Tue, Aug 25, 2015 at 2:12 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>>>>> How and when do ALIGNED_PROXY_BALANCER_SHARED_SIZE and conf->max_balancers
>>>>>>>> change???
>>>>>>>>
>>>>>>>>> On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
>>>>>>>>>
>>>>>>>>> Author: ylavic
>>>>>>>>> Date: Fri Aug 21 12:34:02 2015
>>>>>>>>> New Revision: 1696960
>>>>>>>>>
>>>>>>>>> URL: http://svn.apache.org/r1696960
>>>>>>>>> Log:
>>>>>>>>> mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>>>>>> restart when new ones are added. PR 58024.
>>>>>>>>>
>>>>>>>>> Since slotmem_create() issues a strict check on the size of an existing
>>>>>>>>> slot before reusing it, it won't reuse existing balancers and members when
>>>>>>>>> new ones are added during restart (whereas growth margins would allow it).
>>>>>>>>> Fix this by using slotmem_attach() first and if it succeeds do the checks
>>>>>>>>> based on the returned previous number of existing entries.
>>>>>>>>>
>>>>>>>>> Modified:
>>>>>>>>> httpd/httpd/trunk/CHANGES
>>>>>>>>> httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>>>>>
>>>>>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>>>> ==============================================================================
>>>>>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
>>>>>>>>> @@ -1,6 +1,9 @@
>>>>>>>>> -*- coding: utf-8 -*-
>>>>>>>>> Changes with Apache 2.5.0
>>>>>>>>>
>>>>>>>>> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>>>>>> + restart when new ones are added. PR 58024. [Yann Ylavic]
>>>>>>>>> +
>>>>>>>>> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
>>>>>>>>> long to keep idle connections with the memcache server(s).
>>>>>>>>> Change default value from 600 usec (!) to 15 sec. PR 58091
>>>>>>>>>
>>>>>>>>> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>>>> ==============================================================================
>>>>>>>>> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
>>>>>>>>> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
>>>>>>>>> @@ -1 +1 @@
>>>>>>>>> -2964
>>>>>>>>> +2966
>>>>>>>>>
>>>>>>>>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>>>> ==============================================================================
>>>>>>>>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
>>>>>>>>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
>>>>>>>>> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
>>>>>>>>> char *id;
>>>>>>>>> proxy_balancer *balancer;
>>>>>>>>> ap_slotmem_type_t type;
>>>>>>>>> + apr_size_t attached_size;
>>>>>>>>> + unsigned int attached_num;
>>>>>>>>> void *sconf = s->module_config;
>>>>>>>>> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
>>>>>>>>> +
>>>>>>>>> /*
>>>>>>>>> * During create_proxy_config() we created a dummy id. Now that
>>>>>>>>> * we have identifying info, we can create the real id
>>>>>>>>> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
>>>>>>>>> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>>> (int)conf->balancers->nelts, conf->max_balancers);
>>>>>>>>>
>>>>>>>>> - rv = storage->create(&new, conf->id,
>>>>>>>>> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>>> - conf->max_balancers, type, pconf);
>>>>>>>>> + /* First try to attach() since the number of configured balancers
>>>>>>>>> + * may have changed during restart, and we don't want create() to
>>>>>>>>> + * fail because the overall size * number of entries is not stricly
>>>>>>>>> + * identical to the previous run. There may still be enough room
>>>>>>>>> + * for this new run thanks to bgrowth margin, so if attach()
>>>>>>>>> + * succeeds we can only check for the number of available entries
>>>>>>>>> + * to be *greater or* equal to what we need now. If attach() fails
>>>>>>>>> + * we simply fall back to create().
>>>>>>>>> + */
>>>>>>>>> + rv = storage->attach(&new, conf->id,
>>>>>>>>> + &attached_size, &attached_num,
>>>>>>>>> + pconf);
>>>>>>>>> + if (rv != APR_SUCCESS) {
>>>>>>>>> + rv = storage->create(&new, conf->id,
>>>>>>>>> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>>> + conf->max_balancers, type, pconf);
>>>>>>>>> + }
>>>>>>>>> + else {
>>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
>>>>>>>>> + "Balancers attached: %d, %d (%d)",
>>>>>>>>> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>>> + (int)attached_num, conf->max_balancers);
>>>>>>>>> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
>>>>>>>>> + && attached_num >= conf->balancers->nelts) {
>>>>>>>>> + conf->max_balancers = attached_num;
>>>>>>>>> + }
>>>>>>>>> + else {
>>>>>>>>> + rv = APR_ENOSPC;
>>>>>>>>> + }
>>>>>>>>> + }
>>>>>>>>> if (rv != APR_SUCCESS) {
>>>>>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
>>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
>>>>>>>>> + "balancer slotmem create or attach failed");
>>>>>>>>> return !OK;
>>>>>>>>> }
>>>>>>>>> conf->bslot = new;
>>>>>>>>> @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
>>>>>>>>> (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>>> (int)balancer->max_workers, i);
>>>>>>>>>
>>>>>>>>> - rv = storage->create(&new, balancer->s->sname,
>>>>>>>>> - ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>>> - balancer->max_workers, type, pconf);
>>>>>>>>> + /* try to attach first (see rationale from balancers above) */
>>>>>>>>> + rv = storage->attach(&new, balancer->s->sname,
>>>>>>>>> + &attached_size, &attached_num,
>>>>>>>>> + pconf);
>>>>>>>>> + if (rv != APR_SUCCESS) {
>>>>>>>>> + rv = storage->create(&new, balancer->s->sname,
>>>>>>>>> + ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>>> + balancer->max_workers, type, pconf);
>>>>>>>>> + }
>>>>>>>>> + else {
>>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
>>>>>>>>> + "Workers attached: %s (%s), %d, %d (%d) [%u]",
>>>>>>>>> + balancer->s->name, balancer->s->sname,
>>>>>>>>> + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>>> + (int)attached_num, balancer->max_workers, i);
>>>>>>>>> + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
>>>>>>>>> + && attached_num >= balancer->workers->nelts) {
>>>>>>>>> + balancer->max_workers = attached_num;
>>>>>>>>> + }
>>>>>>>>> + else {
>>>>>>>>> + rv = APR_ENOSPC;
>>>>>>>>> + }
>>>>>>>>> + }
>>>>>>>>> if (rv != APR_SUCCESS) {
>>>>>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
>>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
>>>>>>>>> + "worker slotmem create or attach failed");
>>>>>>>>> return !OK;
>>>>>>>>> }
>>>>>>>>> balancer->wslot = new;
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
The whole intent of the dynamic capability of balancer manage,
plus the persistence of it, is to allow changes to be done
to the reverse proxy setup w/o requiring any changes to
the config at all. It allows it to be reconfigured dynamically,
either via the manager or some other mechanism. It's to
*avoid* changing the file config. It's been a long-requested
change and something which *completely* distinguishes httpd
from others. I am very -1 on any changes that reduce that
capability or any that somehow "encourages" going back to
a preference for changes to the config file.
> On Aug 25, 2015, at 10:43 AM, Yann Ylavic <yl...@gmail.com> wrote:
>
> The advantage of using the balancer-manager to add a member is that
> failures won't kill httpd, whereas failing to add member or balancer
> via the configuration file (no space left) stops httpd (now
> documented).
>
> To avoid that, maybe could we use non-shared (per child) memory in
> this case, like in 2.2.x, so that httpd can continue to ~work~?
>
>
> On Tue, Aug 25, 2015 at 4:41 PM, Yann Ylavic <yl...@gmail.com> wrote:
>> I don't see the difference between the configuration via
>> balancer-manager or file+graceful-restart, both can change the number
>> of balancers/members AND the settings while children are running.
>> I'd kind of agree if we were talking about non-graceful restarts or
>> first startup, we could add a new storage->destroy() method to do the
>> job depending on the slotmem provider (though what about remote
>> storage shared between multiple httpds?).
>> Currently any restart (graceful or not) reuses the existing slots,
>> even if the settings changed (but the size of the slot prior to this
>> commit), so your proposed changes may break compatibility too...
>>
>>
>> On Tue, Aug 25, 2015 at 3:59 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>> When 1st configured, Apache sees how many balancers there are
>>> set in the config file. It then pads that amount by growth to
>>> allow for the to-be-added dynamic addition of new balancers via
>>> the balancer manager (ala adding members).
>>>
>>> The assumption is that if the actual config file is changed,
>>> then the runtime configuration of balancers and members
>>> is no longer valid; after all, if someone is changing the
>>> config file, they could have done the same to add/change
>>> bal/mem settings. So a config file change means something
>>> more "serious" at which point the dynamic config should be
>>> considered void and Apache starts with a clean (dynamic) slate
>>> and just uses what's in the config only.
>>>
>>>> On Aug 25, 2015, at 9:48 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>
>>>> Hm, what's the point of the balancers and balancer->workers growth margin then?
>>>> The first time the configration is loaded we create enough room for
>>>> adding new balancers/workers, and on (graceful) restart we wouldn't
>>>> use this space?
>>>> That's also how the balancer-manager works when adding a new worker to
>>>> a balancer, AFAICT.
>>>>
>>>>
>>>> On Tue, Aug 25, 2015 at 3:43 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>> You mean via changing the config file? If so, then all bets
>>>>> are off. You change the config file and restart then Apache
>>>>> should start off w/ a clean slate. Even so, you need to handle
>>>>> freeing the old shared mem segment and creating a brand new one
>>>>> with the "correct" size... Trying to "use" the old one is
>>>>> just fraught with issues.
>>>>>
>>>>>
>>>>>> On Aug 25, 2015, at 9:15 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>>
>>>>>> Well, if one adds a new balancer before restarting, at post_config
>>>>>> time conf->balancers->nelts is +1 wrt the previous load...
>>>>>>
>>>>>> On Tue, Aug 25, 2015 at 2:46 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>>>> But the number of balancers does not change yet... And when it
>>>>>>> is implemented the overall size of the storage slot should *never*
>>>>>>> change. It must be constant, ala the scoreboard.
>>>>>>>
>>>>>>>> On Aug 25, 2015, at 8:39 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>>>>
>>>>>>>> At every startup, in balancer_post_config:
>>>>>>>> conf->max_balancers = conf->balancers->nelts + conf->bgrowth;
>>>>>>>>
>>>>>>>> which means that if you add a balancer and restart, storage->create()
>>>>>>>> (eg. slotmem_create) will try to find an existing slot with this ID
>>>>>>>> and the exact ALIGNED_PROXY_BALANCER_SHARED_SIZE * conf->max_balancers
>>>>>>>> size, and fail:
>>>>>>>> if (apr_shm_size_get(shm) != size) {
>>>>>>>> apr_shm_detach(shm);
>>>>>>>> ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(02599)
>>>>>>>> "existing shared memory for %s could not be used
>>>>>>>> (failed size check)",
>>>>>>>> fname);
>>>>>>>> return APR_EINVAL;
>>>>>>>> }
>>>>>>>>
>>>>>>>> By using storage->attach() first (which is based on the ID only and
>>>>>>>> returns the existing item_size and item_num), we can do the check
>>>>>>>> ourselves and fail only if conf->max_balancers > item_num, allowing
>>>>>>>> for balancers to be added thanks to growth margin used at startup.
>>>>>>>>
>>>>>>>> On Tue, Aug 25, 2015 at 2:12 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>>>>>> How and when do ALIGNED_PROXY_BALANCER_SHARED_SIZE and conf->max_balancers
>>>>>>>>> change???
>>>>>>>>>
>>>>>>>>>> On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
>>>>>>>>>>
>>>>>>>>>> Author: ylavic
>>>>>>>>>> Date: Fri Aug 21 12:34:02 2015
>>>>>>>>>> New Revision: 1696960
>>>>>>>>>>
>>>>>>>>>> URL: http://svn.apache.org/r1696960
>>>>>>>>>> Log:
>>>>>>>>>> mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>>>>>>> restart when new ones are added. PR 58024.
>>>>>>>>>>
>>>>>>>>>> Since slotmem_create() issues a strict check on the size of an existing
>>>>>>>>>> slot before reusing it, it won't reuse existing balancers and members when
>>>>>>>>>> new ones are added during restart (whereas growth margins would allow it).
>>>>>>>>>> Fix this by using slotmem_attach() first and if it succeeds do the checks
>>>>>>>>>> based on the returned previous number of existing entries.
>>>>>>>>>>
>>>>>>>>>> Modified:
>>>>>>>>>> httpd/httpd/trunk/CHANGES
>>>>>>>>>> httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>>>>>>
>>>>>>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>>>>> ==============================================================================
>>>>>>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>>>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
>>>>>>>>>> @@ -1,6 +1,9 @@
>>>>>>>>>> -*- coding: utf-8 -*-
>>>>>>>>>> Changes with Apache 2.5.0
>>>>>>>>>>
>>>>>>>>>> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>>>>>>> + restart when new ones are added. PR 58024. [Yann Ylavic]
>>>>>>>>>> +
>>>>>>>>>> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
>>>>>>>>>> long to keep idle connections with the memcache server(s).
>>>>>>>>>> Change default value from 600 usec (!) to 15 sec. PR 58091
>>>>>>>>>>
>>>>>>>>>> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>>>>> ==============================================================================
>>>>>>>>>> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
>>>>>>>>>> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
>>>>>>>>>> @@ -1 +1 @@
>>>>>>>>>> -2964
>>>>>>>>>> +2966
>>>>>>>>>>
>>>>>>>>>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>>>>> ==============================================================================
>>>>>>>>>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
>>>>>>>>>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
>>>>>>>>>> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
>>>>>>>>>> char *id;
>>>>>>>>>> proxy_balancer *balancer;
>>>>>>>>>> ap_slotmem_type_t type;
>>>>>>>>>> + apr_size_t attached_size;
>>>>>>>>>> + unsigned int attached_num;
>>>>>>>>>> void *sconf = s->module_config;
>>>>>>>>>> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
>>>>>>>>>> +
>>>>>>>>>> /*
>>>>>>>>>> * During create_proxy_config() we created a dummy id. Now that
>>>>>>>>>> * we have identifying info, we can create the real id
>>>>>>>>>> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
>>>>>>>>>> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>>>> (int)conf->balancers->nelts, conf->max_balancers);
>>>>>>>>>>
>>>>>>>>>> - rv = storage->create(&new, conf->id,
>>>>>>>>>> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>>>> - conf->max_balancers, type, pconf);
>>>>>>>>>> + /* First try to attach() since the number of configured balancers
>>>>>>>>>> + * may have changed during restart, and we don't want create() to
>>>>>>>>>> + * fail because the overall size * number of entries is not stricly
>>>>>>>>>> + * identical to the previous run. There may still be enough room
>>>>>>>>>> + * for this new run thanks to bgrowth margin, so if attach()
>>>>>>>>>> + * succeeds we can only check for the number of available entries
>>>>>>>>>> + * to be *greater or* equal to what we need now. If attach() fails
>>>>>>>>>> + * we simply fall back to create().
>>>>>>>>>> + */
>>>>>>>>>> + rv = storage->attach(&new, conf->id,
>>>>>>>>>> + &attached_size, &attached_num,
>>>>>>>>>> + pconf);
>>>>>>>>>> + if (rv != APR_SUCCESS) {
>>>>>>>>>> + rv = storage->create(&new, conf->id,
>>>>>>>>>> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>>>> + conf->max_balancers, type, pconf);
>>>>>>>>>> + }
>>>>>>>>>> + else {
>>>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
>>>>>>>>>> + "Balancers attached: %d, %d (%d)",
>>>>>>>>>> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>>>> + (int)attached_num, conf->max_balancers);
>>>>>>>>>> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
>>>>>>>>>> + && attached_num >= conf->balancers->nelts) {
>>>>>>>>>> + conf->max_balancers = attached_num;
>>>>>>>>>> + }
>>>>>>>>>> + else {
>>>>>>>>>> + rv = APR_ENOSPC;
>>>>>>>>>> + }
>>>>>>>>>> + }
>>>>>>>>>> if (rv != APR_SUCCESS) {
>>>>>>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
>>>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
>>>>>>>>>> + "balancer slotmem create or attach failed");
>>>>>>>>>> return !OK;
>>>>>>>>>> }
>>>>>>>>>> conf->bslot = new;
>>>>>>>>>> @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
>>>>>>>>>> (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>>>> (int)balancer->max_workers, i);
>>>>>>>>>>
>>>>>>>>>> - rv = storage->create(&new, balancer->s->sname,
>>>>>>>>>> - ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>>>> - balancer->max_workers, type, pconf);
>>>>>>>>>> + /* try to attach first (see rationale from balancers above) */
>>>>>>>>>> + rv = storage->attach(&new, balancer->s->sname,
>>>>>>>>>> + &attached_size, &attached_num,
>>>>>>>>>> + pconf);
>>>>>>>>>> + if (rv != APR_SUCCESS) {
>>>>>>>>>> + rv = storage->create(&new, balancer->s->sname,
>>>>>>>>>> + ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>>>> + balancer->max_workers, type, pconf);
>>>>>>>>>> + }
>>>>>>>>>> + else {
>>>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
>>>>>>>>>> + "Workers attached: %s (%s), %d, %d (%d) [%u]",
>>>>>>>>>> + balancer->s->name, balancer->s->sname,
>>>>>>>>>> + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>>>> + (int)attached_num, balancer->max_workers, i);
>>>>>>>>>> + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
>>>>>>>>>> + && attached_num >= balancer->workers->nelts) {
>>>>>>>>>> + balancer->max_workers = attached_num;
>>>>>>>>>> + }
>>>>>>>>>> + else {
>>>>>>>>>> + rv = APR_ENOSPC;
>>>>>>>>>> + }
>>>>>>>>>> + }
>>>>>>>>>> if (rv != APR_SUCCESS) {
>>>>>>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
>>>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
>>>>>>>>>> + "worker slotmem create or attach failed");
>>>>>>>>>> return !OK;
>>>>>>>>>> }
>>>>>>>>>> balancer->wslot = new;
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>
>>>
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Yann Ylavic <yl...@gmail.com>.
The advantage of using the balancer-manager to add a member is that
failures won't kill httpd, whereas failing to add member or balancer
via the configuration file (no space left) stops httpd (now
documented).
To avoid that, maybe could we use non-shared (per child) memory in
this case, like in 2.2.x, so that httpd can continue to ~work~?
On Tue, Aug 25, 2015 at 4:41 PM, Yann Ylavic <yl...@gmail.com> wrote:
> I don't see the difference between the configuration via
> balancer-manager or file+graceful-restart, both can change the number
> of balancers/members AND the settings while children are running.
> I'd kind of agree if we were talking about non-graceful restarts or
> first startup, we could add a new storage->destroy() method to do the
> job depending on the slotmem provider (though what about remote
> storage shared between multiple httpds?).
> Currently any restart (graceful or not) reuses the existing slots,
> even if the settings changed (but the size of the slot prior to this
> commit), so your proposed changes may break compatibility too...
>
>
> On Tue, Aug 25, 2015 at 3:59 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>> When 1st configured, Apache sees how many balancers there are
>> set in the config file. It then pads that amount by growth to
>> allow for the to-be-added dynamic addition of new balancers via
>> the balancer manager (ala adding members).
>>
>> The assumption is that if the actual config file is changed,
>> then the runtime configuration of balancers and members
>> is no longer valid; after all, if someone is changing the
>> config file, they could have done the same to add/change
>> bal/mem settings. So a config file change means something
>> more "serious" at which point the dynamic config should be
>> considered void and Apache starts with a clean (dynamic) slate
>> and just uses what's in the config only.
>>
>>> On Aug 25, 2015, at 9:48 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>
>>> Hm, what's the point of the balancers and balancer->workers growth margin then?
>>> The first time the configration is loaded we create enough room for
>>> adding new balancers/workers, and on (graceful) restart we wouldn't
>>> use this space?
>>> That's also how the balancer-manager works when adding a new worker to
>>> a balancer, AFAICT.
>>>
>>>
>>> On Tue, Aug 25, 2015 at 3:43 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>> You mean via changing the config file? If so, then all bets
>>>> are off. You change the config file and restart then Apache
>>>> should start off w/ a clean slate. Even so, you need to handle
>>>> freeing the old shared mem segment and creating a brand new one
>>>> with the "correct" size... Trying to "use" the old one is
>>>> just fraught with issues.
>>>>
>>>>
>>>>> On Aug 25, 2015, at 9:15 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>
>>>>> Well, if one adds a new balancer before restarting, at post_config
>>>>> time conf->balancers->nelts is +1 wrt the previous load...
>>>>>
>>>>> On Tue, Aug 25, 2015 at 2:46 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>>> But the number of balancers does not change yet... And when it
>>>>>> is implemented the overall size of the storage slot should *never*
>>>>>> change. It must be constant, ala the scoreboard.
>>>>>>
>>>>>>> On Aug 25, 2015, at 8:39 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>>>
>>>>>>> At every startup, in balancer_post_config:
>>>>>>> conf->max_balancers = conf->balancers->nelts + conf->bgrowth;
>>>>>>>
>>>>>>> which means that if you add a balancer and restart, storage->create()
>>>>>>> (eg. slotmem_create) will try to find an existing slot with this ID
>>>>>>> and the exact ALIGNED_PROXY_BALANCER_SHARED_SIZE * conf->max_balancers
>>>>>>> size, and fail:
>>>>>>> if (apr_shm_size_get(shm) != size) {
>>>>>>> apr_shm_detach(shm);
>>>>>>> ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(02599)
>>>>>>> "existing shared memory for %s could not be used
>>>>>>> (failed size check)",
>>>>>>> fname);
>>>>>>> return APR_EINVAL;
>>>>>>> }
>>>>>>>
>>>>>>> By using storage->attach() first (which is based on the ID only and
>>>>>>> returns the existing item_size and item_num), we can do the check
>>>>>>> ourselves and fail only if conf->max_balancers > item_num, allowing
>>>>>>> for balancers to be added thanks to growth margin used at startup.
>>>>>>>
>>>>>>> On Tue, Aug 25, 2015 at 2:12 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>>>>> How and when do ALIGNED_PROXY_BALANCER_SHARED_SIZE and conf->max_balancers
>>>>>>>> change???
>>>>>>>>
>>>>>>>>> On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
>>>>>>>>>
>>>>>>>>> Author: ylavic
>>>>>>>>> Date: Fri Aug 21 12:34:02 2015
>>>>>>>>> New Revision: 1696960
>>>>>>>>>
>>>>>>>>> URL: http://svn.apache.org/r1696960
>>>>>>>>> Log:
>>>>>>>>> mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>>>>>> restart when new ones are added. PR 58024.
>>>>>>>>>
>>>>>>>>> Since slotmem_create() issues a strict check on the size of an existing
>>>>>>>>> slot before reusing it, it won't reuse existing balancers and members when
>>>>>>>>> new ones are added during restart (whereas growth margins would allow it).
>>>>>>>>> Fix this by using slotmem_attach() first and if it succeeds do the checks
>>>>>>>>> based on the returned previous number of existing entries.
>>>>>>>>>
>>>>>>>>> Modified:
>>>>>>>>> httpd/httpd/trunk/CHANGES
>>>>>>>>> httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>>>>>
>>>>>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>>>> ==============================================================================
>>>>>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
>>>>>>>>> @@ -1,6 +1,9 @@
>>>>>>>>> -*- coding: utf-8 -*-
>>>>>>>>> Changes with Apache 2.5.0
>>>>>>>>>
>>>>>>>>> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>>>>>> + restart when new ones are added. PR 58024. [Yann Ylavic]
>>>>>>>>> +
>>>>>>>>> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
>>>>>>>>> long to keep idle connections with the memcache server(s).
>>>>>>>>> Change default value from 600 usec (!) to 15 sec. PR 58091
>>>>>>>>>
>>>>>>>>> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>>>> ==============================================================================
>>>>>>>>> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
>>>>>>>>> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
>>>>>>>>> @@ -1 +1 @@
>>>>>>>>> -2964
>>>>>>>>> +2966
>>>>>>>>>
>>>>>>>>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>>>> ==============================================================================
>>>>>>>>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
>>>>>>>>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
>>>>>>>>> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
>>>>>>>>> char *id;
>>>>>>>>> proxy_balancer *balancer;
>>>>>>>>> ap_slotmem_type_t type;
>>>>>>>>> + apr_size_t attached_size;
>>>>>>>>> + unsigned int attached_num;
>>>>>>>>> void *sconf = s->module_config;
>>>>>>>>> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
>>>>>>>>> +
>>>>>>>>> /*
>>>>>>>>> * During create_proxy_config() we created a dummy id. Now that
>>>>>>>>> * we have identifying info, we can create the real id
>>>>>>>>> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
>>>>>>>>> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>>> (int)conf->balancers->nelts, conf->max_balancers);
>>>>>>>>>
>>>>>>>>> - rv = storage->create(&new, conf->id,
>>>>>>>>> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>>> - conf->max_balancers, type, pconf);
>>>>>>>>> + /* First try to attach() since the number of configured balancers
>>>>>>>>> + * may have changed during restart, and we don't want create() to
>>>>>>>>> + * fail because the overall size * number of entries is not stricly
>>>>>>>>> + * identical to the previous run. There may still be enough room
>>>>>>>>> + * for this new run thanks to bgrowth margin, so if attach()
>>>>>>>>> + * succeeds we can only check for the number of available entries
>>>>>>>>> + * to be *greater or* equal to what we need now. If attach() fails
>>>>>>>>> + * we simply fall back to create().
>>>>>>>>> + */
>>>>>>>>> + rv = storage->attach(&new, conf->id,
>>>>>>>>> + &attached_size, &attached_num,
>>>>>>>>> + pconf);
>>>>>>>>> + if (rv != APR_SUCCESS) {
>>>>>>>>> + rv = storage->create(&new, conf->id,
>>>>>>>>> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>>> + conf->max_balancers, type, pconf);
>>>>>>>>> + }
>>>>>>>>> + else {
>>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
>>>>>>>>> + "Balancers attached: %d, %d (%d)",
>>>>>>>>> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>>> + (int)attached_num, conf->max_balancers);
>>>>>>>>> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
>>>>>>>>> + && attached_num >= conf->balancers->nelts) {
>>>>>>>>> + conf->max_balancers = attached_num;
>>>>>>>>> + }
>>>>>>>>> + else {
>>>>>>>>> + rv = APR_ENOSPC;
>>>>>>>>> + }
>>>>>>>>> + }
>>>>>>>>> if (rv != APR_SUCCESS) {
>>>>>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
>>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
>>>>>>>>> + "balancer slotmem create or attach failed");
>>>>>>>>> return !OK;
>>>>>>>>> }
>>>>>>>>> conf->bslot = new;
>>>>>>>>> @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
>>>>>>>>> (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>>> (int)balancer->max_workers, i);
>>>>>>>>>
>>>>>>>>> - rv = storage->create(&new, balancer->s->sname,
>>>>>>>>> - ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>>> - balancer->max_workers, type, pconf);
>>>>>>>>> + /* try to attach first (see rationale from balancers above) */
>>>>>>>>> + rv = storage->attach(&new, balancer->s->sname,
>>>>>>>>> + &attached_size, &attached_num,
>>>>>>>>> + pconf);
>>>>>>>>> + if (rv != APR_SUCCESS) {
>>>>>>>>> + rv = storage->create(&new, balancer->s->sname,
>>>>>>>>> + ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>>> + balancer->max_workers, type, pconf);
>>>>>>>>> + }
>>>>>>>>> + else {
>>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
>>>>>>>>> + "Workers attached: %s (%s), %d, %d (%d) [%u]",
>>>>>>>>> + balancer->s->name, balancer->s->sname,
>>>>>>>>> + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>>> + (int)attached_num, balancer->max_workers, i);
>>>>>>>>> + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
>>>>>>>>> + && attached_num >= balancer->workers->nelts) {
>>>>>>>>> + balancer->max_workers = attached_num;
>>>>>>>>> + }
>>>>>>>>> + else {
>>>>>>>>> + rv = APR_ENOSPC;
>>>>>>>>> + }
>>>>>>>>> + }
>>>>>>>>> if (rv != APR_SUCCESS) {
>>>>>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
>>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
>>>>>>>>> + "worker slotmem create or attach failed");
>>>>>>>>> return !OK;
>>>>>>>>> }
>>>>>>>>> balancer->wslot = new;
>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>
>>
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Yann Ylavic <yl...@gmail.com>.
I don't see the difference between the configuration via
balancer-manager or file+graceful-restart, both can change the number
of balancers/members AND the settings while children are running.
I'd kind of agree if we were talking about non-graceful restarts or
first startup, we could add a new storage->destroy() method to do the
job depending on the slotmem provider (though what about remote
storage shared between multiple httpds?).
Currently any restart (graceful or not) reuses the existing slots,
even if the settings changed (but the size of the slot prior to this
commit), so your proposed changes may break compatibility too...
On Tue, Aug 25, 2015 at 3:59 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> When 1st configured, Apache sees how many balancers there are
> set in the config file. It then pads that amount by growth to
> allow for the to-be-added dynamic addition of new balancers via
> the balancer manager (ala adding members).
>
> The assumption is that if the actual config file is changed,
> then the runtime configuration of balancers and members
> is no longer valid; after all, if someone is changing the
> config file, they could have done the same to add/change
> bal/mem settings. So a config file change means something
> more "serious" at which point the dynamic config should be
> considered void and Apache starts with a clean (dynamic) slate
> and just uses what's in the config only.
>
>> On Aug 25, 2015, at 9:48 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>
>> Hm, what's the point of the balancers and balancer->workers growth margin then?
>> The first time the configration is loaded we create enough room for
>> adding new balancers/workers, and on (graceful) restart we wouldn't
>> use this space?
>> That's also how the balancer-manager works when adding a new worker to
>> a balancer, AFAICT.
>>
>>
>> On Tue, Aug 25, 2015 at 3:43 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>> You mean via changing the config file? If so, then all bets
>>> are off. You change the config file and restart then Apache
>>> should start off w/ a clean slate. Even so, you need to handle
>>> freeing the old shared mem segment and creating a brand new one
>>> with the "correct" size... Trying to "use" the old one is
>>> just fraught with issues.
>>>
>>>
>>>> On Aug 25, 2015, at 9:15 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>
>>>> Well, if one adds a new balancer before restarting, at post_config
>>>> time conf->balancers->nelts is +1 wrt the previous load...
>>>>
>>>> On Tue, Aug 25, 2015 at 2:46 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>> But the number of balancers does not change yet... And when it
>>>>> is implemented the overall size of the storage slot should *never*
>>>>> change. It must be constant, ala the scoreboard.
>>>>>
>>>>>> On Aug 25, 2015, at 8:39 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>>
>>>>>> At every startup, in balancer_post_config:
>>>>>> conf->max_balancers = conf->balancers->nelts + conf->bgrowth;
>>>>>>
>>>>>> which means that if you add a balancer and restart, storage->create()
>>>>>> (eg. slotmem_create) will try to find an existing slot with this ID
>>>>>> and the exact ALIGNED_PROXY_BALANCER_SHARED_SIZE * conf->max_balancers
>>>>>> size, and fail:
>>>>>> if (apr_shm_size_get(shm) != size) {
>>>>>> apr_shm_detach(shm);
>>>>>> ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(02599)
>>>>>> "existing shared memory for %s could not be used
>>>>>> (failed size check)",
>>>>>> fname);
>>>>>> return APR_EINVAL;
>>>>>> }
>>>>>>
>>>>>> By using storage->attach() first (which is based on the ID only and
>>>>>> returns the existing item_size and item_num), we can do the check
>>>>>> ourselves and fail only if conf->max_balancers > item_num, allowing
>>>>>> for balancers to be added thanks to growth margin used at startup.
>>>>>>
>>>>>> On Tue, Aug 25, 2015 at 2:12 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>>>> How and when do ALIGNED_PROXY_BALANCER_SHARED_SIZE and conf->max_balancers
>>>>>>> change???
>>>>>>>
>>>>>>>> On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
>>>>>>>>
>>>>>>>> Author: ylavic
>>>>>>>> Date: Fri Aug 21 12:34:02 2015
>>>>>>>> New Revision: 1696960
>>>>>>>>
>>>>>>>> URL: http://svn.apache.org/r1696960
>>>>>>>> Log:
>>>>>>>> mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>>>>> restart when new ones are added. PR 58024.
>>>>>>>>
>>>>>>>> Since slotmem_create() issues a strict check on the size of an existing
>>>>>>>> slot before reusing it, it won't reuse existing balancers and members when
>>>>>>>> new ones are added during restart (whereas growth margins would allow it).
>>>>>>>> Fix this by using slotmem_attach() first and if it succeeds do the checks
>>>>>>>> based on the returned previous number of existing entries.
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>> httpd/httpd/trunk/CHANGES
>>>>>>>> httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>>>>
>>>>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>>> ==============================================================================
>>>>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
>>>>>>>> @@ -1,6 +1,9 @@
>>>>>>>> -*- coding: utf-8 -*-
>>>>>>>> Changes with Apache 2.5.0
>>>>>>>>
>>>>>>>> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>>>>> + restart when new ones are added. PR 58024. [Yann Ylavic]
>>>>>>>> +
>>>>>>>> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
>>>>>>>> long to keep idle connections with the memcache server(s).
>>>>>>>> Change default value from 600 usec (!) to 15 sec. PR 58091
>>>>>>>>
>>>>>>>> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>>> ==============================================================================
>>>>>>>> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
>>>>>>>> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
>>>>>>>> @@ -1 +1 @@
>>>>>>>> -2964
>>>>>>>> +2966
>>>>>>>>
>>>>>>>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>>> ==============================================================================
>>>>>>>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
>>>>>>>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
>>>>>>>> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
>>>>>>>> char *id;
>>>>>>>> proxy_balancer *balancer;
>>>>>>>> ap_slotmem_type_t type;
>>>>>>>> + apr_size_t attached_size;
>>>>>>>> + unsigned int attached_num;
>>>>>>>> void *sconf = s->module_config;
>>>>>>>> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
>>>>>>>> +
>>>>>>>> /*
>>>>>>>> * During create_proxy_config() we created a dummy id. Now that
>>>>>>>> * we have identifying info, we can create the real id
>>>>>>>> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
>>>>>>>> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>> (int)conf->balancers->nelts, conf->max_balancers);
>>>>>>>>
>>>>>>>> - rv = storage->create(&new, conf->id,
>>>>>>>> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>> - conf->max_balancers, type, pconf);
>>>>>>>> + /* First try to attach() since the number of configured balancers
>>>>>>>> + * may have changed during restart, and we don't want create() to
>>>>>>>> + * fail because the overall size * number of entries is not stricly
>>>>>>>> + * identical to the previous run. There may still be enough room
>>>>>>>> + * for this new run thanks to bgrowth margin, so if attach()
>>>>>>>> + * succeeds we can only check for the number of available entries
>>>>>>>> + * to be *greater or* equal to what we need now. If attach() fails
>>>>>>>> + * we simply fall back to create().
>>>>>>>> + */
>>>>>>>> + rv = storage->attach(&new, conf->id,
>>>>>>>> + &attached_size, &attached_num,
>>>>>>>> + pconf);
>>>>>>>> + if (rv != APR_SUCCESS) {
>>>>>>>> + rv = storage->create(&new, conf->id,
>>>>>>>> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>> + conf->max_balancers, type, pconf);
>>>>>>>> + }
>>>>>>>> + else {
>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
>>>>>>>> + "Balancers attached: %d, %d (%d)",
>>>>>>>> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>>> + (int)attached_num, conf->max_balancers);
>>>>>>>> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
>>>>>>>> + && attached_num >= conf->balancers->nelts) {
>>>>>>>> + conf->max_balancers = attached_num;
>>>>>>>> + }
>>>>>>>> + else {
>>>>>>>> + rv = APR_ENOSPC;
>>>>>>>> + }
>>>>>>>> + }
>>>>>>>> if (rv != APR_SUCCESS) {
>>>>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
>>>>>>>> + "balancer slotmem create or attach failed");
>>>>>>>> return !OK;
>>>>>>>> }
>>>>>>>> conf->bslot = new;
>>>>>>>> @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
>>>>>>>> (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>> (int)balancer->max_workers, i);
>>>>>>>>
>>>>>>>> - rv = storage->create(&new, balancer->s->sname,
>>>>>>>> - ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>> - balancer->max_workers, type, pconf);
>>>>>>>> + /* try to attach first (see rationale from balancers above) */
>>>>>>>> + rv = storage->attach(&new, balancer->s->sname,
>>>>>>>> + &attached_size, &attached_num,
>>>>>>>> + pconf);
>>>>>>>> + if (rv != APR_SUCCESS) {
>>>>>>>> + rv = storage->create(&new, balancer->s->sname,
>>>>>>>> + ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>> + balancer->max_workers, type, pconf);
>>>>>>>> + }
>>>>>>>> + else {
>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
>>>>>>>> + "Workers attached: %s (%s), %d, %d (%d) [%u]",
>>>>>>>> + balancer->s->name, balancer->s->sname,
>>>>>>>> + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>>> + (int)attached_num, balancer->max_workers, i);
>>>>>>>> + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
>>>>>>>> + && attached_num >= balancer->workers->nelts) {
>>>>>>>> + balancer->max_workers = attached_num;
>>>>>>>> + }
>>>>>>>> + else {
>>>>>>>> + rv = APR_ENOSPC;
>>>>>>>> + }
>>>>>>>> + }
>>>>>>>> if (rv != APR_SUCCESS) {
>>>>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
>>>>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
>>>>>>>> + "worker slotmem create or attach failed");
>>>>>>>> return !OK;
>>>>>>>> }
>>>>>>>> balancer->wslot = new;
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>
>
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
When 1st configured, Apache sees how many balancers there are
set in the config file. It then pads that amount by growth to
allow for the to-be-added dynamic addition of new balancers via
the balancer manager (ala adding members).
The assumption is that if the actual config file is changed,
then the runtime configuration of balancers and members
is no longer valid; after all, if someone is changing the
config file, they could have done the same to add/change
bal/mem settings. So a config file change means something
more "serious" at which point the dynamic config should be
considered void and Apache starts with a clean (dynamic) slate
and just uses what's in the config only.
> On Aug 25, 2015, at 9:48 AM, Yann Ylavic <yl...@gmail.com> wrote:
>
> Hm, what's the point of the balancers and balancer->workers growth margin then?
> The first time the configration is loaded we create enough room for
> adding new balancers/workers, and on (graceful) restart we wouldn't
> use this space?
> That's also how the balancer-manager works when adding a new worker to
> a balancer, AFAICT.
>
>
> On Tue, Aug 25, 2015 at 3:43 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>> You mean via changing the config file? If so, then all bets
>> are off. You change the config file and restart then Apache
>> should start off w/ a clean slate. Even so, you need to handle
>> freeing the old shared mem segment and creating a brand new one
>> with the "correct" size... Trying to "use" the old one is
>> just fraught with issues.
>>
>>
>>> On Aug 25, 2015, at 9:15 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>
>>> Well, if one adds a new balancer before restarting, at post_config
>>> time conf->balancers->nelts is +1 wrt the previous load...
>>>
>>> On Tue, Aug 25, 2015 at 2:46 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>> But the number of balancers does not change yet... And when it
>>>> is implemented the overall size of the storage slot should *never*
>>>> change. It must be constant, ala the scoreboard.
>>>>
>>>>> On Aug 25, 2015, at 8:39 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>>
>>>>> At every startup, in balancer_post_config:
>>>>> conf->max_balancers = conf->balancers->nelts + conf->bgrowth;
>>>>>
>>>>> which means that if you add a balancer and restart, storage->create()
>>>>> (eg. slotmem_create) will try to find an existing slot with this ID
>>>>> and the exact ALIGNED_PROXY_BALANCER_SHARED_SIZE * conf->max_balancers
>>>>> size, and fail:
>>>>> if (apr_shm_size_get(shm) != size) {
>>>>> apr_shm_detach(shm);
>>>>> ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(02599)
>>>>> "existing shared memory for %s could not be used
>>>>> (failed size check)",
>>>>> fname);
>>>>> return APR_EINVAL;
>>>>> }
>>>>>
>>>>> By using storage->attach() first (which is based on the ID only and
>>>>> returns the existing item_size and item_num), we can do the check
>>>>> ourselves and fail only if conf->max_balancers > item_num, allowing
>>>>> for balancers to be added thanks to growth margin used at startup.
>>>>>
>>>>> On Tue, Aug 25, 2015 at 2:12 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>>> How and when do ALIGNED_PROXY_BALANCER_SHARED_SIZE and conf->max_balancers
>>>>>> change???
>>>>>>
>>>>>>> On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
>>>>>>>
>>>>>>> Author: ylavic
>>>>>>> Date: Fri Aug 21 12:34:02 2015
>>>>>>> New Revision: 1696960
>>>>>>>
>>>>>>> URL: http://svn.apache.org/r1696960
>>>>>>> Log:
>>>>>>> mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>>>> restart when new ones are added. PR 58024.
>>>>>>>
>>>>>>> Since slotmem_create() issues a strict check on the size of an existing
>>>>>>> slot before reusing it, it won't reuse existing balancers and members when
>>>>>>> new ones are added during restart (whereas growth margins would allow it).
>>>>>>> Fix this by using slotmem_attach() first and if it succeeds do the checks
>>>>>>> based on the returned previous number of existing entries.
>>>>>>>
>>>>>>> Modified:
>>>>>>> httpd/httpd/trunk/CHANGES
>>>>>>> httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>>>
>>>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
>>>>>>> @@ -1,6 +1,9 @@
>>>>>>> -*- coding: utf-8 -*-
>>>>>>> Changes with Apache 2.5.0
>>>>>>>
>>>>>>> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>>>> + restart when new ones are added. PR 58024. [Yann Ylavic]
>>>>>>> +
>>>>>>> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
>>>>>>> long to keep idle connections with the memcache server(s).
>>>>>>> Change default value from 600 usec (!) to 15 sec. PR 58091
>>>>>>>
>>>>>>> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
>>>>>>> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
>>>>>>> @@ -1 +1 @@
>>>>>>> -2964
>>>>>>> +2966
>>>>>>>
>>>>>>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>>> ==============================================================================
>>>>>>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
>>>>>>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
>>>>>>> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
>>>>>>> char *id;
>>>>>>> proxy_balancer *balancer;
>>>>>>> ap_slotmem_type_t type;
>>>>>>> + apr_size_t attached_size;
>>>>>>> + unsigned int attached_num;
>>>>>>> void *sconf = s->module_config;
>>>>>>> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
>>>>>>> +
>>>>>>> /*
>>>>>>> * During create_proxy_config() we created a dummy id. Now that
>>>>>>> * we have identifying info, we can create the real id
>>>>>>> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
>>>>>>> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>> (int)conf->balancers->nelts, conf->max_balancers);
>>>>>>>
>>>>>>> - rv = storage->create(&new, conf->id,
>>>>>>> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>> - conf->max_balancers, type, pconf);
>>>>>>> + /* First try to attach() since the number of configured balancers
>>>>>>> + * may have changed during restart, and we don't want create() to
>>>>>>> + * fail because the overall size * number of entries is not stricly
>>>>>>> + * identical to the previous run. There may still be enough room
>>>>>>> + * for this new run thanks to bgrowth margin, so if attach()
>>>>>>> + * succeeds we can only check for the number of available entries
>>>>>>> + * to be *greater or* equal to what we need now. If attach() fails
>>>>>>> + * we simply fall back to create().
>>>>>>> + */
>>>>>>> + rv = storage->attach(&new, conf->id,
>>>>>>> + &attached_size, &attached_num,
>>>>>>> + pconf);
>>>>>>> + if (rv != APR_SUCCESS) {
>>>>>>> + rv = storage->create(&new, conf->id,
>>>>>>> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>> + conf->max_balancers, type, pconf);
>>>>>>> + }
>>>>>>> + else {
>>>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
>>>>>>> + "Balancers attached: %d, %d (%d)",
>>>>>>> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>>> + (int)attached_num, conf->max_balancers);
>>>>>>> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
>>>>>>> + && attached_num >= conf->balancers->nelts) {
>>>>>>> + conf->max_balancers = attached_num;
>>>>>>> + }
>>>>>>> + else {
>>>>>>> + rv = APR_ENOSPC;
>>>>>>> + }
>>>>>>> + }
>>>>>>> if (rv != APR_SUCCESS) {
>>>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
>>>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
>>>>>>> + "balancer slotmem create or attach failed");
>>>>>>> return !OK;
>>>>>>> }
>>>>>>> conf->bslot = new;
>>>>>>> @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
>>>>>>> (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>> (int)balancer->max_workers, i);
>>>>>>>
>>>>>>> - rv = storage->create(&new, balancer->s->sname,
>>>>>>> - ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>> - balancer->max_workers, type, pconf);
>>>>>>> + /* try to attach first (see rationale from balancers above) */
>>>>>>> + rv = storage->attach(&new, balancer->s->sname,
>>>>>>> + &attached_size, &attached_num,
>>>>>>> + pconf);
>>>>>>> + if (rv != APR_SUCCESS) {
>>>>>>> + rv = storage->create(&new, balancer->s->sname,
>>>>>>> + ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>> + balancer->max_workers, type, pconf);
>>>>>>> + }
>>>>>>> + else {
>>>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
>>>>>>> + "Workers attached: %s (%s), %d, %d (%d) [%u]",
>>>>>>> + balancer->s->name, balancer->s->sname,
>>>>>>> + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>>> + (int)attached_num, balancer->max_workers, i);
>>>>>>> + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
>>>>>>> + && attached_num >= balancer->workers->nelts) {
>>>>>>> + balancer->max_workers = attached_num;
>>>>>>> + }
>>>>>>> + else {
>>>>>>> + rv = APR_ENOSPC;
>>>>>>> + }
>>>>>>> + }
>>>>>>> if (rv != APR_SUCCESS) {
>>>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
>>>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
>>>>>>> + "worker slotmem create or attach failed");
>>>>>>> return !OK;
>>>>>>> }
>>>>>>> balancer->wslot = new;
>>>>>>>
>>>>>>>
>>>>>>
>>>>
>>
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Yann Ylavic <yl...@gmail.com>.
Hm, what's the point of the balancers and balancer->workers growth margin then?
The first time the configration is loaded we create enough room for
adding new balancers/workers, and on (graceful) restart we wouldn't
use this space?
That's also how the balancer-manager works when adding a new worker to
a balancer, AFAICT.
On Tue, Aug 25, 2015 at 3:43 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> You mean via changing the config file? If so, then all bets
> are off. You change the config file and restart then Apache
> should start off w/ a clean slate. Even so, you need to handle
> freeing the old shared mem segment and creating a brand new one
> with the "correct" size... Trying to "use" the old one is
> just fraught with issues.
>
>
>> On Aug 25, 2015, at 9:15 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>
>> Well, if one adds a new balancer before restarting, at post_config
>> time conf->balancers->nelts is +1 wrt the previous load...
>>
>> On Tue, Aug 25, 2015 at 2:46 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>> But the number of balancers does not change yet... And when it
>>> is implemented the overall size of the storage slot should *never*
>>> change. It must be constant, ala the scoreboard.
>>>
>>>> On Aug 25, 2015, at 8:39 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>
>>>> At every startup, in balancer_post_config:
>>>> conf->max_balancers = conf->balancers->nelts + conf->bgrowth;
>>>>
>>>> which means that if you add a balancer and restart, storage->create()
>>>> (eg. slotmem_create) will try to find an existing slot with this ID
>>>> and the exact ALIGNED_PROXY_BALANCER_SHARED_SIZE * conf->max_balancers
>>>> size, and fail:
>>>> if (apr_shm_size_get(shm) != size) {
>>>> apr_shm_detach(shm);
>>>> ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(02599)
>>>> "existing shared memory for %s could not be used
>>>> (failed size check)",
>>>> fname);
>>>> return APR_EINVAL;
>>>> }
>>>>
>>>> By using storage->attach() first (which is based on the ID only and
>>>> returns the existing item_size and item_num), we can do the check
>>>> ourselves and fail only if conf->max_balancers > item_num, allowing
>>>> for balancers to be added thanks to growth margin used at startup.
>>>>
>>>> On Tue, Aug 25, 2015 at 2:12 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>> How and when do ALIGNED_PROXY_BALANCER_SHARED_SIZE and conf->max_balancers
>>>>> change???
>>>>>
>>>>>> On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
>>>>>>
>>>>>> Author: ylavic
>>>>>> Date: Fri Aug 21 12:34:02 2015
>>>>>> New Revision: 1696960
>>>>>>
>>>>>> URL: http://svn.apache.org/r1696960
>>>>>> Log:
>>>>>> mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>>> restart when new ones are added. PR 58024.
>>>>>>
>>>>>> Since slotmem_create() issues a strict check on the size of an existing
>>>>>> slot before reusing it, it won't reuse existing balancers and members when
>>>>>> new ones are added during restart (whereas growth margins would allow it).
>>>>>> Fix this by using slotmem_attach() first and if it succeeds do the checks
>>>>>> based on the returned previous number of existing entries.
>>>>>>
>>>>>> Modified:
>>>>>> httpd/httpd/trunk/CHANGES
>>>>>> httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>>
>>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>> ==============================================================================
>>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
>>>>>> @@ -1,6 +1,9 @@
>>>>>> -*- coding: utf-8 -*-
>>>>>> Changes with Apache 2.5.0
>>>>>>
>>>>>> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>>> + restart when new ones are added. PR 58024. [Yann Ylavic]
>>>>>> +
>>>>>> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
>>>>>> long to keep idle connections with the memcache server(s).
>>>>>> Change default value from 600 usec (!) to 15 sec. PR 58091
>>>>>>
>>>>>> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>> ==============================================================================
>>>>>> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
>>>>>> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
>>>>>> @@ -1 +1 @@
>>>>>> -2964
>>>>>> +2966
>>>>>>
>>>>>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>> ==============================================================================
>>>>>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
>>>>>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
>>>>>> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
>>>>>> char *id;
>>>>>> proxy_balancer *balancer;
>>>>>> ap_slotmem_type_t type;
>>>>>> + apr_size_t attached_size;
>>>>>> + unsigned int attached_num;
>>>>>> void *sconf = s->module_config;
>>>>>> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
>>>>>> +
>>>>>> /*
>>>>>> * During create_proxy_config() we created a dummy id. Now that
>>>>>> * we have identifying info, we can create the real id
>>>>>> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
>>>>>> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>> (int)conf->balancers->nelts, conf->max_balancers);
>>>>>>
>>>>>> - rv = storage->create(&new, conf->id,
>>>>>> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>> - conf->max_balancers, type, pconf);
>>>>>> + /* First try to attach() since the number of configured balancers
>>>>>> + * may have changed during restart, and we don't want create() to
>>>>>> + * fail because the overall size * number of entries is not stricly
>>>>>> + * identical to the previous run. There may still be enough room
>>>>>> + * for this new run thanks to bgrowth margin, so if attach()
>>>>>> + * succeeds we can only check for the number of available entries
>>>>>> + * to be *greater or* equal to what we need now. If attach() fails
>>>>>> + * we simply fall back to create().
>>>>>> + */
>>>>>> + rv = storage->attach(&new, conf->id,
>>>>>> + &attached_size, &attached_num,
>>>>>> + pconf);
>>>>>> + if (rv != APR_SUCCESS) {
>>>>>> + rv = storage->create(&new, conf->id,
>>>>>> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>> + conf->max_balancers, type, pconf);
>>>>>> + }
>>>>>> + else {
>>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
>>>>>> + "Balancers attached: %d, %d (%d)",
>>>>>> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>> + (int)attached_num, conf->max_balancers);
>>>>>> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
>>>>>> + && attached_num >= conf->balancers->nelts) {
>>>>>> + conf->max_balancers = attached_num;
>>>>>> + }
>>>>>> + else {
>>>>>> + rv = APR_ENOSPC;
>>>>>> + }
>>>>>> + }
>>>>>> if (rv != APR_SUCCESS) {
>>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
>>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
>>>>>> + "balancer slotmem create or attach failed");
>>>>>> return !OK;
>>>>>> }
>>>>>> conf->bslot = new;
>>>>>> @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
>>>>>> (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>> (int)balancer->max_workers, i);
>>>>>>
>>>>>> - rv = storage->create(&new, balancer->s->sname,
>>>>>> - ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>> - balancer->max_workers, type, pconf);
>>>>>> + /* try to attach first (see rationale from balancers above) */
>>>>>> + rv = storage->attach(&new, balancer->s->sname,
>>>>>> + &attached_size, &attached_num,
>>>>>> + pconf);
>>>>>> + if (rv != APR_SUCCESS) {
>>>>>> + rv = storage->create(&new, balancer->s->sname,
>>>>>> + ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>> + balancer->max_workers, type, pconf);
>>>>>> + }
>>>>>> + else {
>>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
>>>>>> + "Workers attached: %s (%s), %d, %d (%d) [%u]",
>>>>>> + balancer->s->name, balancer->s->sname,
>>>>>> + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>> + (int)attached_num, balancer->max_workers, i);
>>>>>> + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
>>>>>> + && attached_num >= balancer->workers->nelts) {
>>>>>> + balancer->max_workers = attached_num;
>>>>>> + }
>>>>>> + else {
>>>>>> + rv = APR_ENOSPC;
>>>>>> + }
>>>>>> + }
>>>>>> if (rv != APR_SUCCESS) {
>>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
>>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
>>>>>> + "worker slotmem create or attach failed");
>>>>>> return !OK;
>>>>>> }
>>>>>> balancer->wslot = new;
>>>>>>
>>>>>>
>>>>>
>>>
>
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
You mean via changing the config file? If so, then all bets
are off. You change the config file and restart then Apache
should start off w/ a clean slate. Even so, you need to handle
freeing the old shared mem segment and creating a brand new one
with the "correct" size... Trying to "use" the old one is
just fraught with issues.
> On Aug 25, 2015, at 9:15 AM, Yann Ylavic <yl...@gmail.com> wrote:
>
> Well, if one adds a new balancer before restarting, at post_config
> time conf->balancers->nelts is +1 wrt the previous load...
>
> On Tue, Aug 25, 2015 at 2:46 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>> But the number of balancers does not change yet... And when it
>> is implemented the overall size of the storage slot should *never*
>> change. It must be constant, ala the scoreboard.
>>
>>> On Aug 25, 2015, at 8:39 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>
>>> At every startup, in balancer_post_config:
>>> conf->max_balancers = conf->balancers->nelts + conf->bgrowth;
>>>
>>> which means that if you add a balancer and restart, storage->create()
>>> (eg. slotmem_create) will try to find an existing slot with this ID
>>> and the exact ALIGNED_PROXY_BALANCER_SHARED_SIZE * conf->max_balancers
>>> size, and fail:
>>> if (apr_shm_size_get(shm) != size) {
>>> apr_shm_detach(shm);
>>> ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(02599)
>>> "existing shared memory for %s could not be used
>>> (failed size check)",
>>> fname);
>>> return APR_EINVAL;
>>> }
>>>
>>> By using storage->attach() first (which is based on the ID only and
>>> returns the existing item_size and item_num), we can do the check
>>> ourselves and fail only if conf->max_balancers > item_num, allowing
>>> for balancers to be added thanks to growth margin used at startup.
>>>
>>> On Tue, Aug 25, 2015 at 2:12 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>> How and when do ALIGNED_PROXY_BALANCER_SHARED_SIZE and conf->max_balancers
>>>> change???
>>>>
>>>>> On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
>>>>>
>>>>> Author: ylavic
>>>>> Date: Fri Aug 21 12:34:02 2015
>>>>> New Revision: 1696960
>>>>>
>>>>> URL: http://svn.apache.org/r1696960
>>>>> Log:
>>>>> mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>> restart when new ones are added. PR 58024.
>>>>>
>>>>> Since slotmem_create() issues a strict check on the size of an existing
>>>>> slot before reusing it, it won't reuse existing balancers and members when
>>>>> new ones are added during restart (whereas growth margins would allow it).
>>>>> Fix this by using slotmem_attach() first and if it succeeds do the checks
>>>>> based on the returned previous number of existing entries.
>>>>>
>>>>> Modified:
>>>>> httpd/httpd/trunk/CHANGES
>>>>> httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>
>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>> ==============================================================================
>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
>>>>> @@ -1,6 +1,9 @@
>>>>> -*- coding: utf-8 -*-
>>>>> Changes with Apache 2.5.0
>>>>>
>>>>> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>> + restart when new ones are added. PR 58024. [Yann Ylavic]
>>>>> +
>>>>> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
>>>>> long to keep idle connections with the memcache server(s).
>>>>> Change default value from 600 usec (!) to 15 sec. PR 58091
>>>>>
>>>>> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>> ==============================================================================
>>>>> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
>>>>> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
>>>>> @@ -1 +1 @@
>>>>> -2964
>>>>> +2966
>>>>>
>>>>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>> ==============================================================================
>>>>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
>>>>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
>>>>> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
>>>>> char *id;
>>>>> proxy_balancer *balancer;
>>>>> ap_slotmem_type_t type;
>>>>> + apr_size_t attached_size;
>>>>> + unsigned int attached_num;
>>>>> void *sconf = s->module_config;
>>>>> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
>>>>> +
>>>>> /*
>>>>> * During create_proxy_config() we created a dummy id. Now that
>>>>> * we have identifying info, we can create the real id
>>>>> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
>>>>> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>> (int)conf->balancers->nelts, conf->max_balancers);
>>>>>
>>>>> - rv = storage->create(&new, conf->id,
>>>>> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>> - conf->max_balancers, type, pconf);
>>>>> + /* First try to attach() since the number of configured balancers
>>>>> + * may have changed during restart, and we don't want create() to
>>>>> + * fail because the overall size * number of entries is not stricly
>>>>> + * identical to the previous run. There may still be enough room
>>>>> + * for this new run thanks to bgrowth margin, so if attach()
>>>>> + * succeeds we can only check for the number of available entries
>>>>> + * to be *greater or* equal to what we need now. If attach() fails
>>>>> + * we simply fall back to create().
>>>>> + */
>>>>> + rv = storage->attach(&new, conf->id,
>>>>> + &attached_size, &attached_num,
>>>>> + pconf);
>>>>> + if (rv != APR_SUCCESS) {
>>>>> + rv = storage->create(&new, conf->id,
>>>>> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>> + conf->max_balancers, type, pconf);
>>>>> + }
>>>>> + else {
>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
>>>>> + "Balancers attached: %d, %d (%d)",
>>>>> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>> + (int)attached_num, conf->max_balancers);
>>>>> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
>>>>> + && attached_num >= conf->balancers->nelts) {
>>>>> + conf->max_balancers = attached_num;
>>>>> + }
>>>>> + else {
>>>>> + rv = APR_ENOSPC;
>>>>> + }
>>>>> + }
>>>>> if (rv != APR_SUCCESS) {
>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
>>>>> + "balancer slotmem create or attach failed");
>>>>> return !OK;
>>>>> }
>>>>> conf->bslot = new;
>>>>> @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
>>>>> (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>> (int)balancer->max_workers, i);
>>>>>
>>>>> - rv = storage->create(&new, balancer->s->sname,
>>>>> - ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>> - balancer->max_workers, type, pconf);
>>>>> + /* try to attach first (see rationale from balancers above) */
>>>>> + rv = storage->attach(&new, balancer->s->sname,
>>>>> + &attached_size, &attached_num,
>>>>> + pconf);
>>>>> + if (rv != APR_SUCCESS) {
>>>>> + rv = storage->create(&new, balancer->s->sname,
>>>>> + ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>> + balancer->max_workers, type, pconf);
>>>>> + }
>>>>> + else {
>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
>>>>> + "Workers attached: %s (%s), %d, %d (%d) [%u]",
>>>>> + balancer->s->name, balancer->s->sname,
>>>>> + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>> + (int)attached_num, balancer->max_workers, i);
>>>>> + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
>>>>> + && attached_num >= balancer->workers->nelts) {
>>>>> + balancer->max_workers = attached_num;
>>>>> + }
>>>>> + else {
>>>>> + rv = APR_ENOSPC;
>>>>> + }
>>>>> + }
>>>>> if (rv != APR_SUCCESS) {
>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
>>>>> + "worker slotmem create or attach failed");
>>>>> return !OK;
>>>>> }
>>>>> balancer->wslot = new;
>>>>>
>>>>>
>>>>
>>
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
> On Aug 25, 2015, at 2:15 PM, Yann Ylavic <yl...@gmail.com> wrote:
>
> On Tue, Aug 25, 2015 at 6:16 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>> I see that, but the assumption is that the reason why the size
>> would have changed is that someone edited the httpd.conf file and
>> added another balancer.
>
> That's the only way to add a balancer AFAICT, the manager allows to
> add members to existing balancers only.
>
That's right. Adding balancers via the manager is to-be-done.
> If so, how is an admin supposed to add a new balancer?
> Stop than start (removing by hand the persistant file, in between, if
> the slot persisted)?
> Should we do this automatically on startup and restart when the size changes?
> How starting from a clean fresh slate (with new size?) would help
> remote configuration (via the manager)?
>
>> At that point, the more logical thing
>> would be to wipe the slate clean (as we do in other places
>> where certain values in the slotmem != what we expect)...
>> Also, as mentioned, we DO want to fail if the size isn't
>> the same, and the only way the size can be different is if
>> someone changed the file config. So the underlying assumption
>> behind the patch is invalid, imo.
>
> If the current code is already able to ignore other-than-size changes
> for the slots' management, this commit won't change that.
The current code tries to ensure that the read-in slotmem info
is "correct" and consistent. If there is any mismatch then the
slotmem is ignored, cleaned or removed.
> It solely allows the free space in slots (growth margin) to be used by
> both the restart and the manager for balancer members, and by the
> restart only for balancers (the limit on adding balancers is postponed
> to the growth margin exhaustion, currently it fails from the very
> first add in the configuration file whereas no one will ever use the
> reserved space).
I have a feeling that using the growth space for config-file
changes is ill-advised.
> Sharing the balancer members' slots' bgrowth space between the
> configuration file and the manager does not look bad to me either,
> this has to be taken into account (in bgrowth) by the admin from
> startup (and even from the very first startup with persisted slots),
> but the admin is supposed to know how much members are likely to be
> added in the configuration file and/or via the manager.
> If no one adds any balancer or member in the file (it fails today
> btw), httpd will still work as before.
> If httpd is started with persisted slots created by a previous
> version, with or without new balancers/members, it will also work (up
> to the growth margin with new additions).
> The only visible change from the manager will be less space available
> for new additions.
>
> But I don't want to insist on preserving this change, so I'll revert
> it if the above did not convince you ;)
I think that keeping it the way it was is likely easier and better
for when we close the loop in making balancer addition part of the
manager. This change will either be dropped or modded, in which case
it's likely best to revert to avoid unneeded deltas and potential weird
code paths.
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Tue, Aug 25, 2015 at 6:16 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> I see that, but the assumption is that the reason why the size
> would have changed is that someone edited the httpd.conf file and
> added another balancer.
That's the only way to add a balancer AFAICT, the manager allows to
add members to existing balancers only.
If so, how is an admin supposed to add a new balancer?
Stop than start (removing by hand the persistant file, in between, if
the slot persisted)?
Should we do this automatically on startup and restart when the size changes?
How starting from a clean fresh slate (with new size?) would help
remote configuration (via the manager)?
> At that point, the more logical thing
> would be to wipe the slate clean (as we do in other places
> where certain values in the slotmem != what we expect)...
> Also, as mentioned, we DO want to fail if the size isn't
> the same, and the only way the size can be different is if
> someone changed the file config. So the underlying assumption
> behind the patch is invalid, imo.
If the current code is already able to ignore other-than-size changes
for the slots' management, this commit won't change that.
It solely allows the free space in slots (growth margin) to be used by
both the restart and the manager for balancer members, and by the
restart only for balancers (the limit on adding balancers is postponed
to the growth margin exhaustion, currently it fails from the very
first add in the configuration file whereas no one will ever use the
reserved space).
Sharing the balancer members' slots' bgrowth space between the
configuration file and the manager does not look bad to me either,
this has to be taken into account (in bgrowth) by the admin from
startup (and even from the very first startup with persisted slots),
but the admin is supposed to know how much members are likely to be
added in the configuration file and/or via the manager.
If no one adds any balancer or member in the file (it fails today
btw), httpd will still work as before.
If httpd is started with persisted slots created by a previous
version, with or without new balancers/members, it will also work (up
to the growth margin with new additions).
The only visible change from the manager will be less space available
for new additions.
But I don't want to insist on preserving this change, so I'll revert
it if the above did not convince you ;)
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
I see that, but the assumption is that the reason why the size
would have changed is that someone edited the httpd.conf file and
added another balancer. At that point, the more logical thing
would be to wipe the slate clean (as we do in other places
where certain values in the slotmem != what we expect)...
Also, as mentioned, we DO want to fail if the size isn't
the same, and the only way the size can be different is if
someone changed the file config. So the underlying assumption
behind the patch is invalid, imo.
Also, slotmem->create already does an attach when it should; the
below "breaks" the protections provided when using create as the
primary method to "obtain" the slotmem. "Exposing" the attach here
leads to confusion ("why isn't this done elsewhere").
> On Aug 25, 2015, at 9:26 AM, Yann Ylavic <yl...@gmail.com> wrote:
>
> With this commit the overall size of the storage slot does not change,
> the slot is attached and if it has enough room (growth margin) for the
> new configuration, it is used (like with the balancer-manager),
> otherwise the restart fails.
>
> On Tue, Aug 25, 2015 at 3:15 PM, Yann Ylavic <yl...@gmail.com> wrote:
>> Well, if one adds a new balancer before restarting, at post_config
>> time conf->balancers->nelts is +1 wrt the previous load...
>>
>> On Tue, Aug 25, 2015 at 2:46 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>> But the number of balancers does not change yet... And when it
>>> is implemented the overall size of the storage slot should *never*
>>> change. It must be constant, ala the scoreboard.
>>>
>>>> On Aug 25, 2015, at 8:39 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>>
>>>> At every startup, in balancer_post_config:
>>>> conf->max_balancers = conf->balancers->nelts + conf->bgrowth;
>>>>
>>>> which means that if you add a balancer and restart, storage->create()
>>>> (eg. slotmem_create) will try to find an existing slot with this ID
>>>> and the exact ALIGNED_PROXY_BALANCER_SHARED_SIZE * conf->max_balancers
>>>> size, and fail:
>>>> if (apr_shm_size_get(shm) != size) {
>>>> apr_shm_detach(shm);
>>>> ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(02599)
>>>> "existing shared memory for %s could not be used
>>>> (failed size check)",
>>>> fname);
>>>> return APR_EINVAL;
>>>> }
>>>>
>>>> By using storage->attach() first (which is based on the ID only and
>>>> returns the existing item_size and item_num), we can do the check
>>>> ourselves and fail only if conf->max_balancers > item_num, allowing
>>>> for balancers to be added thanks to growth margin used at startup.
>>>>
>>>> On Tue, Aug 25, 2015 at 2:12 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>>> How and when do ALIGNED_PROXY_BALANCER_SHARED_SIZE and conf->max_balancers
>>>>> change???
>>>>>
>>>>>> On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
>>>>>>
>>>>>> Author: ylavic
>>>>>> Date: Fri Aug 21 12:34:02 2015
>>>>>> New Revision: 1696960
>>>>>>
>>>>>> URL: http://svn.apache.org/r1696960
>>>>>> Log:
>>>>>> mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>>> restart when new ones are added. PR 58024.
>>>>>>
>>>>>> Since slotmem_create() issues a strict check on the size of an existing
>>>>>> slot before reusing it, it won't reuse existing balancers and members when
>>>>>> new ones are added during restart (whereas growth margins would allow it).
>>>>>> Fix this by using slotmem_attach() first and if it succeeds do the checks
>>>>>> based on the returned previous number of existing entries.
>>>>>>
>>>>>> Modified:
>>>>>> httpd/httpd/trunk/CHANGES
>>>>>> httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>>
>>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>> ==============================================================================
>>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
>>>>>> @@ -1,6 +1,9 @@
>>>>>> -*- coding: utf-8 -*-
>>>>>> Changes with Apache 2.5.0
>>>>>>
>>>>>> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>>> + restart when new ones are added. PR 58024. [Yann Ylavic]
>>>>>> +
>>>>>> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
>>>>>> long to keep idle connections with the memcache server(s).
>>>>>> Change default value from 600 usec (!) to 15 sec. PR 58091
>>>>>>
>>>>>> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>> ==============================================================================
>>>>>> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
>>>>>> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
>>>>>> @@ -1 +1 @@
>>>>>> -2964
>>>>>> +2966
>>>>>>
>>>>>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>>> ==============================================================================
>>>>>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
>>>>>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
>>>>>> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
>>>>>> char *id;
>>>>>> proxy_balancer *balancer;
>>>>>> ap_slotmem_type_t type;
>>>>>> + apr_size_t attached_size;
>>>>>> + unsigned int attached_num;
>>>>>> void *sconf = s->module_config;
>>>>>> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
>>>>>> +
>>>>>> /*
>>>>>> * During create_proxy_config() we created a dummy id. Now that
>>>>>> * we have identifying info, we can create the real id
>>>>>> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
>>>>>> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>> (int)conf->balancers->nelts, conf->max_balancers);
>>>>>>
>>>>>> - rv = storage->create(&new, conf->id,
>>>>>> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>> - conf->max_balancers, type, pconf);
>>>>>> + /* First try to attach() since the number of configured balancers
>>>>>> + * may have changed during restart, and we don't want create() to
>>>>>> + * fail because the overall size * number of entries is not stricly
>>>>>> + * identical to the previous run. There may still be enough room
>>>>>> + * for this new run thanks to bgrowth margin, so if attach()
>>>>>> + * succeeds we can only check for the number of available entries
>>>>>> + * to be *greater or* equal to what we need now. If attach() fails
>>>>>> + * we simply fall back to create().
>>>>>> + */
>>>>>> + rv = storage->attach(&new, conf->id,
>>>>>> + &attached_size, &attached_num,
>>>>>> + pconf);
>>>>>> + if (rv != APR_SUCCESS) {
>>>>>> + rv = storage->create(&new, conf->id,
>>>>>> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>> + conf->max_balancers, type, pconf);
>>>>>> + }
>>>>>> + else {
>>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
>>>>>> + "Balancers attached: %d, %d (%d)",
>>>>>> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>>> + (int)attached_num, conf->max_balancers);
>>>>>> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
>>>>>> + && attached_num >= conf->balancers->nelts) {
>>>>>> + conf->max_balancers = attached_num;
>>>>>> + }
>>>>>> + else {
>>>>>> + rv = APR_ENOSPC;
>>>>>> + }
>>>>>> + }
>>>>>> if (rv != APR_SUCCESS) {
>>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
>>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
>>>>>> + "balancer slotmem create or attach failed");
>>>>>> return !OK;
>>>>>> }
>>>>>> conf->bslot = new;
>>>>>> @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
>>>>>> (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>> (int)balancer->max_workers, i);
>>>>>>
>>>>>> - rv = storage->create(&new, balancer->s->sname,
>>>>>> - ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>> - balancer->max_workers, type, pconf);
>>>>>> + /* try to attach first (see rationale from balancers above) */
>>>>>> + rv = storage->attach(&new, balancer->s->sname,
>>>>>> + &attached_size, &attached_num,
>>>>>> + pconf);
>>>>>> + if (rv != APR_SUCCESS) {
>>>>>> + rv = storage->create(&new, balancer->s->sname,
>>>>>> + ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>> + balancer->max_workers, type, pconf);
>>>>>> + }
>>>>>> + else {
>>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
>>>>>> + "Workers attached: %s (%s), %d, %d (%d) [%u]",
>>>>>> + balancer->s->name, balancer->s->sname,
>>>>>> + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>>> + (int)attached_num, balancer->max_workers, i);
>>>>>> + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
>>>>>> + && attached_num >= balancer->workers->nelts) {
>>>>>> + balancer->max_workers = attached_num;
>>>>>> + }
>>>>>> + else {
>>>>>> + rv = APR_ENOSPC;
>>>>>> + }
>>>>>> + }
>>>>>> if (rv != APR_SUCCESS) {
>>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
>>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
>>>>>> + "worker slotmem create or attach failed");
>>>>>> return !OK;
>>>>>> }
>>>>>> balancer->wslot = new;
>>>>>>
>>>>>>
>>>>>
>>>
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Yann Ylavic <yl...@gmail.com>.
With this commit the overall size of the storage slot does not change,
the slot is attached and if it has enough room (growth margin) for the
new configuration, it is used (like with the balancer-manager),
otherwise the restart fails.
On Tue, Aug 25, 2015 at 3:15 PM, Yann Ylavic <yl...@gmail.com> wrote:
> Well, if one adds a new balancer before restarting, at post_config
> time conf->balancers->nelts is +1 wrt the previous load...
>
> On Tue, Aug 25, 2015 at 2:46 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>> But the number of balancers does not change yet... And when it
>> is implemented the overall size of the storage slot should *never*
>> change. It must be constant, ala the scoreboard.
>>
>>> On Aug 25, 2015, at 8:39 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>>
>>> At every startup, in balancer_post_config:
>>> conf->max_balancers = conf->balancers->nelts + conf->bgrowth;
>>>
>>> which means that if you add a balancer and restart, storage->create()
>>> (eg. slotmem_create) will try to find an existing slot with this ID
>>> and the exact ALIGNED_PROXY_BALANCER_SHARED_SIZE * conf->max_balancers
>>> size, and fail:
>>> if (apr_shm_size_get(shm) != size) {
>>> apr_shm_detach(shm);
>>> ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(02599)
>>> "existing shared memory for %s could not be used
>>> (failed size check)",
>>> fname);
>>> return APR_EINVAL;
>>> }
>>>
>>> By using storage->attach() first (which is based on the ID only and
>>> returns the existing item_size and item_num), we can do the check
>>> ourselves and fail only if conf->max_balancers > item_num, allowing
>>> for balancers to be added thanks to growth margin used at startup.
>>>
>>> On Tue, Aug 25, 2015 at 2:12 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>>> How and when do ALIGNED_PROXY_BALANCER_SHARED_SIZE and conf->max_balancers
>>>> change???
>>>>
>>>>> On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
>>>>>
>>>>> Author: ylavic
>>>>> Date: Fri Aug 21 12:34:02 2015
>>>>> New Revision: 1696960
>>>>>
>>>>> URL: http://svn.apache.org/r1696960
>>>>> Log:
>>>>> mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>> restart when new ones are added. PR 58024.
>>>>>
>>>>> Since slotmem_create() issues a strict check on the size of an existing
>>>>> slot before reusing it, it won't reuse existing balancers and members when
>>>>> new ones are added during restart (whereas growth margins would allow it).
>>>>> Fix this by using slotmem_attach() first and if it succeeds do the checks
>>>>> based on the returned previous number of existing entries.
>>>>>
>>>>> Modified:
>>>>> httpd/httpd/trunk/CHANGES
>>>>> httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>>
>>>>> Modified: httpd/httpd/trunk/CHANGES
>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>> ==============================================================================
>>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
>>>>> @@ -1,6 +1,9 @@
>>>>> -*- coding: utf-8 -*-
>>>>> Changes with Apache 2.5.0
>>>>>
>>>>> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>>> + restart when new ones are added. PR 58024. [Yann Ylavic]
>>>>> +
>>>>> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
>>>>> long to keep idle connections with the memcache server(s).
>>>>> Change default value from 600 usec (!) to 15 sec. PR 58091
>>>>>
>>>>> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>> ==============================================================================
>>>>> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
>>>>> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
>>>>> @@ -1 +1 @@
>>>>> -2964
>>>>> +2966
>>>>>
>>>>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>>> ==============================================================================
>>>>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
>>>>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
>>>>> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
>>>>> char *id;
>>>>> proxy_balancer *balancer;
>>>>> ap_slotmem_type_t type;
>>>>> + apr_size_t attached_size;
>>>>> + unsigned int attached_num;
>>>>> void *sconf = s->module_config;
>>>>> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
>>>>> +
>>>>> /*
>>>>> * During create_proxy_config() we created a dummy id. Now that
>>>>> * we have identifying info, we can create the real id
>>>>> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
>>>>> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>> (int)conf->balancers->nelts, conf->max_balancers);
>>>>>
>>>>> - rv = storage->create(&new, conf->id,
>>>>> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>> - conf->max_balancers, type, pconf);
>>>>> + /* First try to attach() since the number of configured balancers
>>>>> + * may have changed during restart, and we don't want create() to
>>>>> + * fail because the overall size * number of entries is not stricly
>>>>> + * identical to the previous run. There may still be enough room
>>>>> + * for this new run thanks to bgrowth margin, so if attach()
>>>>> + * succeeds we can only check for the number of available entries
>>>>> + * to be *greater or* equal to what we need now. If attach() fails
>>>>> + * we simply fall back to create().
>>>>> + */
>>>>> + rv = storage->attach(&new, conf->id,
>>>>> + &attached_size, &attached_num,
>>>>> + pconf);
>>>>> + if (rv != APR_SUCCESS) {
>>>>> + rv = storage->create(&new, conf->id,
>>>>> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>> + conf->max_balancers, type, pconf);
>>>>> + }
>>>>> + else {
>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
>>>>> + "Balancers attached: %d, %d (%d)",
>>>>> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>>> + (int)attached_num, conf->max_balancers);
>>>>> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
>>>>> + && attached_num >= conf->balancers->nelts) {
>>>>> + conf->max_balancers = attached_num;
>>>>> + }
>>>>> + else {
>>>>> + rv = APR_ENOSPC;
>>>>> + }
>>>>> + }
>>>>> if (rv != APR_SUCCESS) {
>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
>>>>> + "balancer slotmem create or attach failed");
>>>>> return !OK;
>>>>> }
>>>>> conf->bslot = new;
>>>>> @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
>>>>> (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>> (int)balancer->max_workers, i);
>>>>>
>>>>> - rv = storage->create(&new, balancer->s->sname,
>>>>> - ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>> - balancer->max_workers, type, pconf);
>>>>> + /* try to attach first (see rationale from balancers above) */
>>>>> + rv = storage->attach(&new, balancer->s->sname,
>>>>> + &attached_size, &attached_num,
>>>>> + pconf);
>>>>> + if (rv != APR_SUCCESS) {
>>>>> + rv = storage->create(&new, balancer->s->sname,
>>>>> + ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>> + balancer->max_workers, type, pconf);
>>>>> + }
>>>>> + else {
>>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
>>>>> + "Workers attached: %s (%s), %d, %d (%d) [%u]",
>>>>> + balancer->s->name, balancer->s->sname,
>>>>> + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>>> + (int)attached_num, balancer->max_workers, i);
>>>>> + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
>>>>> + && attached_num >= balancer->workers->nelts) {
>>>>> + balancer->max_workers = attached_num;
>>>>> + }
>>>>> + else {
>>>>> + rv = APR_ENOSPC;
>>>>> + }
>>>>> + }
>>>>> if (rv != APR_SUCCESS) {
>>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
>>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
>>>>> + "worker slotmem create or attach failed");
>>>>> return !OK;
>>>>> }
>>>>> balancer->wslot = new;
>>>>>
>>>>>
>>>>
>>
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Yann Ylavic <yl...@gmail.com>.
Well, if one adds a new balancer before restarting, at post_config
time conf->balancers->nelts is +1 wrt the previous load...
On Tue, Aug 25, 2015 at 2:46 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> But the number of balancers does not change yet... And when it
> is implemented the overall size of the storage slot should *never*
> change. It must be constant, ala the scoreboard.
>
>> On Aug 25, 2015, at 8:39 AM, Yann Ylavic <yl...@gmail.com> wrote:
>>
>> At every startup, in balancer_post_config:
>> conf->max_balancers = conf->balancers->nelts + conf->bgrowth;
>>
>> which means that if you add a balancer and restart, storage->create()
>> (eg. slotmem_create) will try to find an existing slot with this ID
>> and the exact ALIGNED_PROXY_BALANCER_SHARED_SIZE * conf->max_balancers
>> size, and fail:
>> if (apr_shm_size_get(shm) != size) {
>> apr_shm_detach(shm);
>> ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(02599)
>> "existing shared memory for %s could not be used
>> (failed size check)",
>> fname);
>> return APR_EINVAL;
>> }
>>
>> By using storage->attach() first (which is based on the ID only and
>> returns the existing item_size and item_num), we can do the check
>> ourselves and fail only if conf->max_balancers > item_num, allowing
>> for balancers to be added thanks to growth margin used at startup.
>>
>> On Tue, Aug 25, 2015 at 2:12 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>>> How and when do ALIGNED_PROXY_BALANCER_SHARED_SIZE and conf->max_balancers
>>> change???
>>>
>>>> On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
>>>>
>>>> Author: ylavic
>>>> Date: Fri Aug 21 12:34:02 2015
>>>> New Revision: 1696960
>>>>
>>>> URL: http://svn.apache.org/r1696960
>>>> Log:
>>>> mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>> restart when new ones are added. PR 58024.
>>>>
>>>> Since slotmem_create() issues a strict check on the size of an existing
>>>> slot before reusing it, it won't reuse existing balancers and members when
>>>> new ones are added during restart (whereas growth margins would allow it).
>>>> Fix this by using slotmem_attach() first and if it succeeds do the checks
>>>> based on the returned previous number of existing entries.
>>>>
>>>> Modified:
>>>> httpd/httpd/trunk/CHANGES
>>>> httpd/httpd/trunk/docs/log-message-tags/next-number
>>>> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>>
>>>> Modified: httpd/httpd/trunk/CHANGES
>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>> ==============================================================================
>>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
>>>> @@ -1,6 +1,9 @@
>>>> -*- coding: utf-8 -*-
>>>> Changes with Apache 2.5.0
>>>>
>>>> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
>>>> + restart when new ones are added. PR 58024. [Yann Ylavic]
>>>> +
>>>> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
>>>> long to keep idle connections with the memcache server(s).
>>>> Change default value from 600 usec (!) to 15 sec. PR 58091
>>>>
>>>> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>> ==============================================================================
>>>> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
>>>> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
>>>> @@ -1 +1 @@
>>>> -2964
>>>> +2966
>>>>
>>>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
>>>> ==============================================================================
>>>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
>>>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
>>>> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
>>>> char *id;
>>>> proxy_balancer *balancer;
>>>> ap_slotmem_type_t type;
>>>> + apr_size_t attached_size;
>>>> + unsigned int attached_num;
>>>> void *sconf = s->module_config;
>>>> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
>>>> +
>>>> /*
>>>> * During create_proxy_config() we created a dummy id. Now that
>>>> * we have identifying info, we can create the real id
>>>> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
>>>> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>> (int)conf->balancers->nelts, conf->max_balancers);
>>>>
>>>> - rv = storage->create(&new, conf->id,
>>>> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>> - conf->max_balancers, type, pconf);
>>>> + /* First try to attach() since the number of configured balancers
>>>> + * may have changed during restart, and we don't want create() to
>>>> + * fail because the overall size * number of entries is not stricly
>>>> + * identical to the previous run. There may still be enough room
>>>> + * for this new run thanks to bgrowth margin, so if attach()
>>>> + * succeeds we can only check for the number of available entries
>>>> + * to be *greater or* equal to what we need now. If attach() fails
>>>> + * we simply fall back to create().
>>>> + */
>>>> + rv = storage->attach(&new, conf->id,
>>>> + &attached_size, &attached_num,
>>>> + pconf);
>>>> + if (rv != APR_SUCCESS) {
>>>> + rv = storage->create(&new, conf->id,
>>>> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>> + conf->max_balancers, type, pconf);
>>>> + }
>>>> + else {
>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
>>>> + "Balancers attached: %d, %d (%d)",
>>>> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>>> + (int)attached_num, conf->max_balancers);
>>>> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
>>>> + && attached_num >= conf->balancers->nelts) {
>>>> + conf->max_balancers = attached_num;
>>>> + }
>>>> + else {
>>>> + rv = APR_ENOSPC;
>>>> + }
>>>> + }
>>>> if (rv != APR_SUCCESS) {
>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
>>>> + "balancer slotmem create or attach failed");
>>>> return !OK;
>>>> }
>>>> conf->bslot = new;
>>>> @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
>>>> (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>> (int)balancer->max_workers, i);
>>>>
>>>> - rv = storage->create(&new, balancer->s->sname,
>>>> - ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>> - balancer->max_workers, type, pconf);
>>>> + /* try to attach first (see rationale from balancers above) */
>>>> + rv = storage->attach(&new, balancer->s->sname,
>>>> + &attached_size, &attached_num,
>>>> + pconf);
>>>> + if (rv != APR_SUCCESS) {
>>>> + rv = storage->create(&new, balancer->s->sname,
>>>> + ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>> + balancer->max_workers, type, pconf);
>>>> + }
>>>> + else {
>>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
>>>> + "Workers attached: %s (%s), %d, %d (%d) [%u]",
>>>> + balancer->s->name, balancer->s->sname,
>>>> + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>>> + (int)attached_num, balancer->max_workers, i);
>>>> + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
>>>> + && attached_num >= balancer->workers->nelts) {
>>>> + balancer->max_workers = attached_num;
>>>> + }
>>>> + else {
>>>> + rv = APR_ENOSPC;
>>>> + }
>>>> + }
>>>> if (rv != APR_SUCCESS) {
>>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
>>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
>>>> + "worker slotmem create or attach failed");
>>>> return !OK;
>>>> }
>>>> balancer->wslot = new;
>>>>
>>>>
>>>
>
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
But the number of balancers does not change yet... And when it
is implemented the overall size of the storage slot should *never*
change. It must be constant, ala the scoreboard.
> On Aug 25, 2015, at 8:39 AM, Yann Ylavic <yl...@gmail.com> wrote:
>
> At every startup, in balancer_post_config:
> conf->max_balancers = conf->balancers->nelts + conf->bgrowth;
>
> which means that if you add a balancer and restart, storage->create()
> (eg. slotmem_create) will try to find an existing slot with this ID
> and the exact ALIGNED_PROXY_BALANCER_SHARED_SIZE * conf->max_balancers
> size, and fail:
> if (apr_shm_size_get(shm) != size) {
> apr_shm_detach(shm);
> ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(02599)
> "existing shared memory for %s could not be used
> (failed size check)",
> fname);
> return APR_EINVAL;
> }
>
> By using storage->attach() first (which is based on the ID only and
> returns the existing item_size and item_num), we can do the check
> ourselves and fail only if conf->max_balancers > item_num, allowing
> for balancers to be added thanks to growth margin used at startup.
>
> On Tue, Aug 25, 2015 at 2:12 PM, Jim Jagielski <ji...@jagunet.com> wrote:
>> How and when do ALIGNED_PROXY_BALANCER_SHARED_SIZE and conf->max_balancers
>> change???
>>
>>> On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
>>>
>>> Author: ylavic
>>> Date: Fri Aug 21 12:34:02 2015
>>> New Revision: 1696960
>>>
>>> URL: http://svn.apache.org/r1696960
>>> Log:
>>> mod_proxy_balancer: Fix balancers and balancer members reuse on
>>> restart when new ones are added. PR 58024.
>>>
>>> Since slotmem_create() issues a strict check on the size of an existing
>>> slot before reusing it, it won't reuse existing balancers and members when
>>> new ones are added during restart (whereas growth margins would allow it).
>>> Fix this by using slotmem_attach() first and if it succeeds do the checks
>>> based on the returned previous number of existing entries.
>>>
>>> Modified:
>>> httpd/httpd/trunk/CHANGES
>>> httpd/httpd/trunk/docs/log-message-tags/next-number
>>> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>>
>>> Modified: httpd/httpd/trunk/CHANGES
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>>> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
>>> @@ -1,6 +1,9 @@
>>> -*- coding: utf-8 -*-
>>> Changes with Apache 2.5.0
>>>
>>> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
>>> + restart when new ones are added. PR 58024. [Yann Ylavic]
>>> +
>>> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
>>> long to keep idle connections with the memcache server(s).
>>> Change default value from 600 usec (!) to 15 sec. PR 58091
>>>
>>> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
>>> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
>>> @@ -1 +1 @@
>>> -2964
>>> +2966
>>>
>>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
>>> ==============================================================================
>>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
>>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
>>> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
>>> char *id;
>>> proxy_balancer *balancer;
>>> ap_slotmem_type_t type;
>>> + apr_size_t attached_size;
>>> + unsigned int attached_num;
>>> void *sconf = s->module_config;
>>> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
>>> +
>>> /*
>>> * During create_proxy_config() we created a dummy id. Now that
>>> * we have identifying info, we can create the real id
>>> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
>>> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>> (int)conf->balancers->nelts, conf->max_balancers);
>>>
>>> - rv = storage->create(&new, conf->id,
>>> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>> - conf->max_balancers, type, pconf);
>>> + /* First try to attach() since the number of configured balancers
>>> + * may have changed during restart, and we don't want create() to
>>> + * fail because the overall size * number of entries is not stricly
>>> + * identical to the previous run. There may still be enough room
>>> + * for this new run thanks to bgrowth margin, so if attach()
>>> + * succeeds we can only check for the number of available entries
>>> + * to be *greater or* equal to what we need now. If attach() fails
>>> + * we simply fall back to create().
>>> + */
>>> + rv = storage->attach(&new, conf->id,
>>> + &attached_size, &attached_num,
>>> + pconf);
>>> + if (rv != APR_SUCCESS) {
>>> + rv = storage->create(&new, conf->id,
>>> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>> + conf->max_balancers, type, pconf);
>>> + }
>>> + else {
>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
>>> + "Balancers attached: %d, %d (%d)",
>>> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>>> + (int)attached_num, conf->max_balancers);
>>> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
>>> + && attached_num >= conf->balancers->nelts) {
>>> + conf->max_balancers = attached_num;
>>> + }
>>> + else {
>>> + rv = APR_ENOSPC;
>>> + }
>>> + }
>>> if (rv != APR_SUCCESS) {
>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
>>> + "balancer slotmem create or attach failed");
>>> return !OK;
>>> }
>>> conf->bslot = new;
>>> @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
>>> (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>> (int)balancer->max_workers, i);
>>>
>>> - rv = storage->create(&new, balancer->s->sname,
>>> - ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>> - balancer->max_workers, type, pconf);
>>> + /* try to attach first (see rationale from balancers above) */
>>> + rv = storage->attach(&new, balancer->s->sname,
>>> + &attached_size, &attached_num,
>>> + pconf);
>>> + if (rv != APR_SUCCESS) {
>>> + rv = storage->create(&new, balancer->s->sname,
>>> + ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>> + balancer->max_workers, type, pconf);
>>> + }
>>> + else {
>>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
>>> + "Workers attached: %s (%s), %d, %d (%d) [%u]",
>>> + balancer->s->name, balancer->s->sname,
>>> + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>>> + (int)attached_num, balancer->max_workers, i);
>>> + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
>>> + && attached_num >= balancer->workers->nelts) {
>>> + balancer->max_workers = attached_num;
>>> + }
>>> + else {
>>> + rv = APR_ENOSPC;
>>> + }
>>> + }
>>> if (rv != APR_SUCCESS) {
>>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
>>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
>>> + "worker slotmem create or attach failed");
>>> return !OK;
>>> }
>>> balancer->wslot = new;
>>>
>>>
>>
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Yann Ylavic <yl...@gmail.com>.
At every startup, in balancer_post_config:
conf->max_balancers = conf->balancers->nelts + conf->bgrowth;
which means that if you add a balancer and restart, storage->create()
(eg. slotmem_create) will try to find an existing slot with this ID
and the exact ALIGNED_PROXY_BALANCER_SHARED_SIZE * conf->max_balancers
size, and fail:
if (apr_shm_size_get(shm) != size) {
apr_shm_detach(shm);
ap_log_error(APLOG_MARK, APLOG_ERR, 0, ap_server_conf, APLOGNO(02599)
"existing shared memory for %s could not be used
(failed size check)",
fname);
return APR_EINVAL;
}
By using storage->attach() first (which is based on the ID only and
returns the existing item_size and item_num), we can do the check
ourselves and fail only if conf->max_balancers > item_num, allowing
for balancers to be added thanks to growth margin used at startup.
On Tue, Aug 25, 2015 at 2:12 PM, Jim Jagielski <ji...@jagunet.com> wrote:
> How and when do ALIGNED_PROXY_BALANCER_SHARED_SIZE and conf->max_balancers
> change???
>
>> On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
>>
>> Author: ylavic
>> Date: Fri Aug 21 12:34:02 2015
>> New Revision: 1696960
>>
>> URL: http://svn.apache.org/r1696960
>> Log:
>> mod_proxy_balancer: Fix balancers and balancer members reuse on
>> restart when new ones are added. PR 58024.
>>
>> Since slotmem_create() issues a strict check on the size of an existing
>> slot before reusing it, it won't reuse existing balancers and members when
>> new ones are added during restart (whereas growth margins would allow it).
>> Fix this by using slotmem_attach() first and if it succeeds do the checks
>> based on the returned previous number of existing entries.
>>
>> Modified:
>> httpd/httpd/trunk/CHANGES
>> httpd/httpd/trunk/docs/log-message-tags/next-number
>> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>>
>> Modified: httpd/httpd/trunk/CHANGES
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
>> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
>> @@ -1,6 +1,9 @@
>> -*- coding: utf-8 -*-
>> Changes with Apache 2.5.0
>>
>> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
>> + restart when new ones are added. PR 58024. [Yann Ylavic]
>> +
>> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
>> long to keep idle connections with the memcache server(s).
>> Change default value from 600 usec (!) to 15 sec. PR 58091
>>
>> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
>> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
>> @@ -1 +1 @@
>> -2964
>> +2966
>>
>> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
>> ==============================================================================
>> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
>> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
>> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
>> char *id;
>> proxy_balancer *balancer;
>> ap_slotmem_type_t type;
>> + apr_size_t attached_size;
>> + unsigned int attached_num;
>> void *sconf = s->module_config;
>> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
>> +
>> /*
>> * During create_proxy_config() we created a dummy id. Now that
>> * we have identifying info, we can create the real id
>> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
>> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>> (int)conf->balancers->nelts, conf->max_balancers);
>>
>> - rv = storage->create(&new, conf->id,
>> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>> - conf->max_balancers, type, pconf);
>> + /* First try to attach() since the number of configured balancers
>> + * may have changed during restart, and we don't want create() to
>> + * fail because the overall size * number of entries is not stricly
>> + * identical to the previous run. There may still be enough room
>> + * for this new run thanks to bgrowth margin, so if attach()
>> + * succeeds we can only check for the number of available entries
>> + * to be *greater or* equal to what we need now. If attach() fails
>> + * we simply fall back to create().
>> + */
>> + rv = storage->attach(&new, conf->id,
>> + &attached_size, &attached_num,
>> + pconf);
>> + if (rv != APR_SUCCESS) {
>> + rv = storage->create(&new, conf->id,
>> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>> + conf->max_balancers, type, pconf);
>> + }
>> + else {
>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
>> + "Balancers attached: %d, %d (%d)",
>> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
>> + (int)attached_num, conf->max_balancers);
>> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
>> + && attached_num >= conf->balancers->nelts) {
>> + conf->max_balancers = attached_num;
>> + }
>> + else {
>> + rv = APR_ENOSPC;
>> + }
>> + }
>> if (rv != APR_SUCCESS) {
>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
>> + "balancer slotmem create or attach failed");
>> return !OK;
>> }
>> conf->bslot = new;
>> @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
>> (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>> (int)balancer->max_workers, i);
>>
>> - rv = storage->create(&new, balancer->s->sname,
>> - ALIGNED_PROXY_WORKER_SHARED_SIZE,
>> - balancer->max_workers, type, pconf);
>> + /* try to attach first (see rationale from balancers above) */
>> + rv = storage->attach(&new, balancer->s->sname,
>> + &attached_size, &attached_num,
>> + pconf);
>> + if (rv != APR_SUCCESS) {
>> + rv = storage->create(&new, balancer->s->sname,
>> + ALIGNED_PROXY_WORKER_SHARED_SIZE,
>> + balancer->max_workers, type, pconf);
>> + }
>> + else {
>> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
>> + "Workers attached: %s (%s), %d, %d (%d) [%u]",
>> + balancer->s->name, balancer->s->sname,
>> + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
>> + (int)attached_num, balancer->max_workers, i);
>> + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
>> + && attached_num >= balancer->workers->nelts) {
>> + balancer->max_workers = attached_num;
>> + }
>> + else {
>> + rv = APR_ENOSPC;
>> + }
>> + }
>> if (rv != APR_SUCCESS) {
>> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
>> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
>> + "worker slotmem create or attach failed");
>> return !OK;
>> }
>> balancer->wslot = new;
>>
>>
>
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Jim Jagielski <ji...@jaguNET.com>.
How and when do ALIGNED_PROXY_BALANCER_SHARED_SIZE and conf->max_balancers
change???
> On Aug 21, 2015, at 8:34 AM, ylavic@apache.org wrote:
>
> Author: ylavic
> Date: Fri Aug 21 12:34:02 2015
> New Revision: 1696960
>
> URL: http://svn.apache.org/r1696960
> Log:
> mod_proxy_balancer: Fix balancers and balancer members reuse on
> restart when new ones are added. PR 58024.
>
> Since slotmem_create() issues a strict check on the size of an existing
> slot before reusing it, it won't reuse existing balancers and members when
> new ones are added during restart (whereas growth margins would allow it).
> Fix this by using slotmem_attach() first and if it succeeds do the checks
> based on the returned previous number of existing entries.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/docs/log-message-tags/next-number
> httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
>
> Modified: httpd/httpd/trunk/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1696960&r1=1696959&r2=1696960&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/CHANGES [utf-8] (original)
> +++ httpd/httpd/trunk/CHANGES [utf-8] Fri Aug 21 12:34:02 2015
> @@ -1,6 +1,9 @@
> -*- coding: utf-8 -*-
> Changes with Apache 2.5.0
>
> + *) mod_proxy_balancer: Fix balancers and balancer members reuse on
> + restart when new ones are added. PR 58024. [Yann Ylavic]
> +
> *) mod_socache_memcache: Add the 'MemcacheConnTTL' directive to control how
> long to keep idle connections with the memcache server(s).
> Change default value from 600 usec (!) to 15 sec. PR 58091
>
> Modified: httpd/httpd/trunk/docs/log-message-tags/next-number
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/log-message-tags/next-number?rev=1696960&r1=1696959&r2=1696960&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/docs/log-message-tags/next-number (original)
> +++ httpd/httpd/trunk/docs/log-message-tags/next-number Fri Aug 21 12:34:02 2015
> @@ -1 +1 @@
> -2964
> +2966
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c?rev=1696960&r1=1696959&r2=1696960&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_balancer.c Fri Aug 21 12:34:02 2015
> @@ -761,8 +761,11 @@ static int balancer_post_config(apr_pool
> char *id;
> proxy_balancer *balancer;
> ap_slotmem_type_t type;
> + apr_size_t attached_size;
> + unsigned int attached_num;
> void *sconf = s->module_config;
> conf = (proxy_server_conf *)ap_get_module_config(sconf, &proxy_module);
> +
> /*
> * During create_proxy_config() we created a dummy id. Now that
> * we have identifying info, we can create the real id
> @@ -794,11 +797,39 @@ static int balancer_post_config(apr_pool
> (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> (int)conf->balancers->nelts, conf->max_balancers);
>
> - rv = storage->create(&new, conf->id,
> - ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> - conf->max_balancers, type, pconf);
> + /* First try to attach() since the number of configured balancers
> + * may have changed during restart, and we don't want create() to
> + * fail because the overall size * number of entries is not stricly
> + * identical to the previous run. There may still be enough room
> + * for this new run thanks to bgrowth margin, so if attach()
> + * succeeds we can only check for the number of available entries
> + * to be *greater or* equal to what we need now. If attach() fails
> + * we simply fall back to create().
> + */
> + rv = storage->attach(&new, conf->id,
> + &attached_size, &attached_num,
> + pconf);
> + if (rv != APR_SUCCESS) {
> + rv = storage->create(&new, conf->id,
> + ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> + conf->max_balancers, type, pconf);
> + }
> + else {
> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02964)
> + "Balancers attached: %d, %d (%d)",
> + (int)ALIGNED_PROXY_BALANCER_SHARED_SIZE,
> + (int)attached_num, conf->max_balancers);
> + if (attached_size == ALIGNED_PROXY_BALANCER_SHARED_SIZE
> + && attached_num >= conf->balancers->nelts) {
> + conf->max_balancers = attached_num;
> + }
> + else {
> + rv = APR_ENOSPC;
> + }
> + }
> if (rv != APR_SUCCESS) {
> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179) "balancer slotmem_create failed");
> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01179)
> + "balancer slotmem create or attach failed");
> return !OK;
> }
> conf->bslot = new;
> @@ -864,11 +895,32 @@ static int balancer_post_config(apr_pool
> (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
> (int)balancer->max_workers, i);
>
> - rv = storage->create(&new, balancer->s->sname,
> - ALIGNED_PROXY_WORKER_SHARED_SIZE,
> - balancer->max_workers, type, pconf);
> + /* try to attach first (see rationale from balancers above) */
> + rv = storage->attach(&new, balancer->s->sname,
> + &attached_size, &attached_num,
> + pconf);
> + if (rv != APR_SUCCESS) {
> + rv = storage->create(&new, balancer->s->sname,
> + ALIGNED_PROXY_WORKER_SHARED_SIZE,
> + balancer->max_workers, type, pconf);
> + }
> + else {
> + ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(02965)
> + "Workers attached: %s (%s), %d, %d (%d) [%u]",
> + balancer->s->name, balancer->s->sname,
> + (int)ALIGNED_PROXY_WORKER_SHARED_SIZE,
> + (int)attached_num, balancer->max_workers, i);
> + if (attached_size == ALIGNED_PROXY_WORKER_SHARED_SIZE
> + && attached_num >= balancer->workers->nelts) {
> + balancer->max_workers = attached_num;
> + }
> + else {
> + rv = APR_ENOSPC;
> + }
> + }
> if (rv != APR_SUCCESS) {
> - ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185) "worker slotmem_create failed");
> + ap_log_error(APLOG_MARK, APLOG_EMERG, rv, s, APLOGNO(01185)
> + "worker slotmem create or attach failed");
> return !OK;
> }
> balancer->wslot = new;
>
>