On 08/21/2015 02:34 PM, yla...@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

Reply via email to