You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Ruediger Pluem <rp...@apache.org> on 2015/08/21 16:15:22 UTC
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
On 08/21/2015 02:34 PM, ylavic@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;
Hm. Doesn't that mean that if I keep on adding stuff and do graceful restarts that at one point in time I will have used
up all my growth and need to do a hard restart?
If yes, it should be probably documented to avoid some unexpected surprises.
Regards
Rüdiger
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Aug 21, 2015 at 8:07 PM, Ruediger Pluem <rp...@apache.org> wrote:
>
>
> On 08/21/2015 07:02 PM, Yann Ylavic wrote:
>> On Fri, Aug 21, 2015 at 4:15 PM, Ruediger Pluem <rp...@apache.org> wrote:
>>>
>>> Hm. Doesn't that mean that if I keep on adding stuff and do graceful restarts that at one point in time I will have used
>>> up all my growth and need to do a hard restart?
>>
>> Yes, the last restart would fail.
>>
>>> If yes, it should be probably documented to avoid some unexpected surprises.
>>
>> I agree it should be documented (will do), currently we already have
>> this issue from the first restart though.
>
> So it is an improvement over what we have :-)
Yes, including the doc :)
Added in r1697352, and to the backport proposal.
Thanks,
Yann.
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 08/21/2015 07:02 PM, Yann Ylavic wrote:
> On Fri, Aug 21, 2015 at 4:15 PM, Ruediger Pluem <rp...@apache.org> wrote:
>>
>> Hm. Doesn't that mean that if I keep on adding stuff and do graceful restarts that at one point in time I will have used
>> up all my growth and need to do a hard restart?
>
> Yes, the last restart would fail.
>
>> If yes, it should be probably documented to avoid some unexpected surprises.
>
> I agree it should be documented (will do), currently we already have
> this issue from the first restart though.
So it is an improvement over what we have :-)
Regards
Rüdiger
Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES
docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c
Posted by Yann Ylavic <yl...@gmail.com>.
On Fri, Aug 21, 2015 at 4:15 PM, Ruediger Pluem <rp...@apache.org> wrote:
>
> Hm. Doesn't that mean that if I keep on adding stuff and do graceful restarts that at one point in time I will have used
> up all my growth and need to do a hard restart?
Yes, the last restart would fail.
> If yes, it should be probably documented to avoid some unexpected surprises.
I agree it should be documented (will do), currently we already have
this issue from the first restart though.
Regards,
Yann.