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 <ylavic....@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 <j...@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, 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;
>>> +                }
>>> +            }
>>>            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;
>>> 
>>> 
>> 

Reply via email to