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