Re: svn commit: r1696105 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_socache_memcache.xml modules/cache/mod_socache_memcache.c

2015-08-21 Thread Yann Ylavic
On Sun, Aug 16, 2015 at 12:05 AM,  jaillet...@apache.org wrote:
 Author: jailletc36
 Date: Sat Aug 15 22:05:08 2015
 New Revision: 1696105

 URL: http://svn.apache.org/r1696105
[]
 Modified: httpd/httpd/trunk/modules/cache/mod_socache_memcache.c
 URL: 
 http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_socache_memcache.c?rev=1696105r1=1696104r2=1696105view=diff
 ==
 --- httpd/httpd/trunk/modules/cache/mod_socache_memcache.c (original)
 +++ httpd/httpd/trunk/modules/cache/mod_socache_memcache.c Sat Aug 15 
 22:05:08 2015
[]
 @@ -310,6 +319,35 @@ static const ap_socache_provider_t socac
[]
 +static const char *socache_mc_set_ttl(cmd_parms *cmd, void *dummy,
 +  const char *arg)
 +{
 +socache_mc_svr_cfg *sconf = 
 ap_get_module_config(cmd-server-module_config,
 + 
 socache_memcache_module);
 +int i;
 +
 +i = atoi(arg);
 +
 +if ((i  1) || (i  1800)) {

Why a limit of 1800? Maybe the implicit INT_MAX is good enough.
Also the memcached may never close its connections by itself, or
always do, so -1 and 0 could also be interesting...

 +return apr_pstrcat(cmd-pool, cmd-cmd-name,
 +must be a number between 1 and 1800., NULL);
 +}
 +
 +/* apr_memcache_server_create needs a ttl in usec. */
 +sconf-ttl = i * 1000 * 1000;

sconf-ttl = apr_time_from_sec(i) ?

 +
 +return NULL;
 +}

Regards,
Yann.


Re: svn commit: r1696997 - /httpd/httpd/branches/2.2.x/STATUS

2015-08-21 Thread Yann Ylavic
On Fri, Aug 21, 2015 at 3:59 PM,  rpl...@apache.org wrote:

[]
 Modified: httpd/httpd/branches/2.2.x/STATUS
 URL: 
 http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=1696997r1=1696996r2=1696997view=diff
 ==
 --- httpd/httpd/branches/2.2.x/STATUS (original)
 +++ httpd/httpd/branches/2.2.x/STATUS Fri Aug 21 13:59:33 2015
 @@ -126,6 +126,7 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
http://svn.apache.org/r1688343
   2.2.x patch: 
 http://people.apache.org/~ylavic/httpd-2.2.x-SubstituteInheritBefore-v4.patch
   +1: ylavic
 + rpluem: Doesn't that change the previous behaviour if 
 SubstituteInheritBefore is not set?

Thanks! Fixed in r1697002, I will update the backport proposals (2.4
and 2.2) too.


Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c

2015-08-21 Thread Ruediger Pluem
Wasn't it intended for trunk to change the default behavior?

Regards

Rüdiger

On 08/21/2015 04:19 PM, yla...@apache.org wrote:
 Author: ylavic
 Date: Fri Aug 21 14:19:17 2015
 New Revision: 1697002
 
 URL: http://svn.apache.org/r1697002
 Log:
 mod_substitute: follow up r1684900.
 Really inherit before when asked to (inverse if/else contents).
 
 Modified:
 httpd/httpd/trunk/modules/filters/mod_substitute.c
 
 Modified: httpd/httpd/trunk/modules/filters/mod_substitute.c
 URL: 
 http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_substitute.c?rev=1697002r1=1697001r2=1697002view=diff
 ==
 --- httpd/httpd/trunk/modules/filters/mod_substitute.c (original)
 +++ httpd/httpd/trunk/modules/filters/mod_substitute.c Fri Aug 21 14:19:17 
 2015
 @@ -97,13 +97,13 @@ static void *merge_substitute_dcfg(apr_p
   * 'off' to follow the corrected/expected behavior, without violating 
 POLS.
   */
  if (a-inherit_before == 1) {
 -a-patterns = apr_array_append(p, base-patterns,
 -  over-patterns);
 -}
 -else {
  a-patterns = apr_array_append(p, over-patterns,
base-patterns);
  }
 +else {
 +a-patterns = apr_array_append(p, base-patterns,
 +  over-patterns);
 +}
  a-max_line_length = over-max_line_length_set ?
   over-max_line_length : base-max_line_length;
  a-max_line_length_set = over-max_line_length_set
 
 
 


Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c

2015-08-21 Thread Yann Ylavic
On Fri, Aug 21, 2015 at 4:29 PM, Yann Ylavic ylavic@gmail.com wrote:
 On Fri, Aug 21, 2015 at 4:25 PM, Ruediger Pluem rpl...@apache.org wrote:
 Wasn't it intended for trunk to change the default behavior?

 Yes it was, still when inherit_before is set we need to do that.

I meant, we need to do apr_array_append(p, over-patterns,
base-patterns) and not the inverse.


Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c

2015-08-21 Thread Ruediger Pluem


On 08/21/2015 04:31 PM, Yann Ylavic wrote:
 On Fri, Aug 21, 2015 at 4:29 PM, Yann Ylavic ylavic@gmail.com wrote:
 On Fri, Aug 21, 2015 at 4:25 PM, Ruediger Pluem rpl...@apache.org wrote:
 Wasn't it intended for trunk to change the default behavior?

 Yes it was, still when inherit_before is set we need to do that.
 
 I meant, we need to do apr_array_append(p, over-patterns,
 base-patterns) and not the inverse.
 

But apr_array_append(p, over-patterns, base-patterns) means that we do NOT 
inherit the stuff from the base
configuration before the configuration of our section. So the code before your 
recent patch was correct.

Regards

Rüdiger



Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c

2015-08-21 Thread Ruediger Pluem


On 08/21/2015 02:34 PM, 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=1696960r1=1696959r2=1696960view=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=1696960r1=1696959r2=1696960view=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=1696960r1=1696959r2=1696960view=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 {
 

Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c

2015-08-21 Thread Yann Ylavic
On Fri, Aug 21, 2015 at 4:25 PM, Ruediger Pluem rpl...@apache.org wrote:
 Wasn't it intended for trunk to change the default behavior?

Yes it was, still when inherit_before is set we need to do that.

The code in trunk is:
if (a-inherit_before == 1) {
whereas 2.4 and 2.2 (would) have:
if (a-inherit_before) { = can be 1 but also -1

Regards,
Yann.


Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c

2015-08-21 Thread William A Rowe Jr
On Fri, Aug 21, 2015 at 10:05 AM, Yann Ylavic ylavic@gmail.com wrote:

 On Fri, Aug 21, 2015 at 4:36 PM, Ruediger Pluem rpl...@apache.org wrote:
 
 
  On 08/21/2015 04:31 PM, Yann Ylavic wrote:
  On Fri, Aug 21, 2015 at 4:29 PM, Yann Ylavic ylavic@gmail.com
 wrote:
  On Fri, Aug 21, 2015 at 4:25 PM, Ruediger Pluem rpl...@apache.org
 wrote:
  Wasn't it intended for trunk to change the default behavior?
 
  Yes it was, still when inherit_before is set we need to do that.
 
  I meant, we need to do apr_array_append(p, over-patterns,
  base-patterns) and not the inverse.
 
 
  But apr_array_append(p, over-patterns, base-patterns) means that we do
 NOT inherit the stuff from the base
  configuration before the configuration of our section. So the code
 before your recent patch was correct.

 Hm, you are right, I was misleaded by the comment in trunk:
 /* SubstituteInheritBefore was the default behavior until 2.5.x, ... */
 since SubstituteInheritBefore was *not* the default behavior until 2.5.x.

 To recap, apr_array_append(p, over-patterns, base-patterns) is the
 legacy but it is obviously not the inherited base which is merged
 before...

 So to keep the existing/prior-to-r1697002 logic in the code (which was
 good), I propose to revert r1697002 and rename SubstituteInheritBefore
 to SubstituteInheritAfter, WDYT?


It's a toggle, please leave it alone.  The default state will be one case in
2.4.x, and a different case in 2.5.x and later, but a user who puts that
directive into their config will observe the same behavior when they run
their config under 2.4 or later under 2.6.


Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c

2015-08-21 Thread Yann Ylavic
On Fri, Aug 21, 2015 at 4:36 PM, Ruediger Pluem rpl...@apache.org wrote:


 On 08/21/2015 04:31 PM, Yann Ylavic wrote:
 On Fri, Aug 21, 2015 at 4:29 PM, Yann Ylavic ylavic@gmail.com wrote:
 On Fri, Aug 21, 2015 at 4:25 PM, Ruediger Pluem rpl...@apache.org wrote:
 Wasn't it intended for trunk to change the default behavior?

 Yes it was, still when inherit_before is set we need to do that.

 I meant, we need to do apr_array_append(p, over-patterns,
 base-patterns) and not the inverse.


 But apr_array_append(p, over-patterns, base-patterns) means that we do NOT 
 inherit the stuff from the base
 configuration before the configuration of our section. So the code before 
 your recent patch was correct.

Hm, you are right, I was misleaded by the comment in trunk:
/* SubstituteInheritBefore was the default behavior until 2.5.x, ... */
since SubstituteInheritBefore was *not* the default behavior until 2.5.x.

To recap, apr_array_append(p, over-patterns, base-patterns) is the
legacy but it is obviously not the inherited base which is merged
before...

So to keep the existing/prior-to-r1697002 logic in the code (which was
good), I propose to revert r1697002 and rename SubstituteInheritBefore
to SubstituteInheritAfter, WDYT?


Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c

2015-08-21 Thread Yann Ylavic
On Fri, Aug 21, 2015 at 5:46 PM, William A Rowe Jr wr...@rowe-clan.net wrote:
 On Fri, Aug 21, 2015 at 10:32 AM, William A Rowe Jr wr...@rowe-clan.net
 wrote:


 It's a toggle, please leave it alone.  The default state will be one case
 in
 2.4.x, and a different case in 2.5.x and later, but a user who puts that
 directive into their config will observe the same behavior when they run
 their config under 2.4 or later under 2.6.


 (But correcting the comments and the docs in the respective code branches as
 applicable would be terrific, so users and the rest of us aren't confused
 again when this is reviewed again in the future.)

Done in r1697013 and r1697015.
Backports proposals to 2.4.x and 2.2.x updated to v5 too.

Thanks Ruediger and Bill for the review.


Re: svn commit: r1697002 - /httpd/httpd/trunk/modules/filters/mod_substitute.c

2015-08-21 Thread William A Rowe Jr
On Fri, Aug 21, 2015 at 10:32 AM, William A Rowe Jr wr...@rowe-clan.net
wrote:


 It's a toggle, please leave it alone.  The default state will be one case
 in
 2.4.x, and a different case in 2.5.x and later, but a user who puts that
 directive into their config will observe the same behavior when they run
 their config under 2.4 or later under 2.6.


(But correcting the comments and the docs in the respective code branches
as applicable would be terrific, so users and the rest of us aren't
confused again when this is reviewed again in the future.)


Re: svn commit: r1696960 - in /httpd/httpd/trunk: CHANGES docs/log-message-tags/next-number modules/proxy/mod_proxy_balancer.c

2015-08-21 Thread Ruediger Pluem


On 08/21/2015 07:02 PM, Yann Ylavic wrote:
 On Fri, Aug 21, 2015 at 4:15 PM, Ruediger Pluem rpl...@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

2015-08-21 Thread Yann Ylavic
On Fri, Aug 21, 2015 at 4:15 PM, Ruediger Pluem rpl...@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.