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; >>>> >>>> >>> >