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 <j...@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 <ylavic....@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 <j...@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 <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