The whole intent of the dynamic capability of balancer manage, plus the persistence of it, is to allow changes to be done to the reverse proxy setup w/o requiring any changes to the config at all. It allows it to be reconfigured dynamically, either via the manager or some other mechanism. It's to *avoid* changing the file config. It's been a long-requested change and something which *completely* distinguishes httpd from others. I am very -1 on any changes that reduce that capability or any that somehow "encourages" going back to a preference for changes to the config file.
> On Aug 25, 2015, at 10:43 AM, Yann Ylavic <ylavic....@gmail.com> wrote: > > The advantage of using the balancer-manager to add a member is that > failures won't kill httpd, whereas failing to add member or balancer > via the configuration file (no space left) stops httpd (now > documented). > > To avoid that, maybe could we use non-shared (per child) memory in > this case, like in 2.2.x, so that httpd can continue to ~work~? > > > On Tue, Aug 25, 2015 at 4:41 PM, Yann Ylavic <ylavic....@gmail.com> wrote: >> I don't see the difference between the configuration via >> balancer-manager or file+graceful-restart, both can change the number >> of balancers/members AND the settings while children are running. >> I'd kind of agree if we were talking about non-graceful restarts or >> first startup, we could add a new storage->destroy() method to do the >> job depending on the slotmem provider (though what about remote >> storage shared between multiple httpds?). >> Currently any restart (graceful or not) reuses the existing slots, >> even if the settings changed (but the size of the slot prior to this >> commit), so your proposed changes may break compatibility too... >> >> >> On Tue, Aug 25, 2015 at 3:59 PM, Jim Jagielski <j...@jagunet.com> wrote: >>> When 1st configured, Apache sees how many balancers there are >>> set in the config file. It then pads that amount by growth to >>> allow for the to-be-added dynamic addition of new balancers via >>> the balancer manager (ala adding members). >>> >>> The assumption is that if the actual config file is changed, >>> then the runtime configuration of balancers and members >>> is no longer valid; after all, if someone is changing the >>> config file, they could have done the same to add/change >>> bal/mem settings. So a config file change means something >>> more "serious" at which point the dynamic config should be >>> considered void and Apache starts with a clean (dynamic) slate >>> and just uses what's in the config only. >>> >>>> On Aug 25, 2015, at 9:48 AM, Yann Ylavic <ylavic....@gmail.com> wrote: >>>> >>>> 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; >>>>>>>>>> >>>>>>>>>> >>>>>>>>> >>>>>>> >>>>> >>>