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