Commit b7f7b509 authored by Jim Jagielski's avatar Jim Jagielski
Browse files

Merge r1696105, r1700418 from trunk: 

With the current implementation, it is likely to connect/close a socket with the memcache server for each command sent.
The root cause is a too small idle timeout (600 microseconds).

Add a new directive, 'MemcacheConnTTL',  to control this idle connection timeout with the memcache server(s).
Change the default value from 600 usec (!) to 15 sec as per Yann suggestion.

I've limited accepted values from 1 to 1800 seconds (half an hour) because internaly, the value passed to 'apr_memcache_server_create' is still in mirco-seconds.

PR 58091
~~~~~~~~~~~~~~~~~~~_
Homemade measurement (on a slighly modified version of httpd) shows a +30% in number of processed requests using memcache to cache /index.html.
Comparison made between the 600 usec and 15 sec TTL.

Memcache config:
    default
httpd Config:
    CacheEnable socache /
    CacheSocache memcache:127.0.0.1
    LoadModule mpm_event_module modules/mod_mpm_event.so
httpd compiled with:
    ./configure --enable-mpms-shared=all --with-included-apr --with-mysql --with-libxml2 --enable-modules=reallyall --enable-ssl-ct=no --enable-maintainer-mode --prefix=$HOME/httpd-2.5
httpd and memcache running on the same VM running under Ubuntu 15.04
Load tested using:
    ab -n 20000 http://127.0.0.1/index.html

Creation/closing of connections beetween httpd and memcache confirmed using the telnet connection to memcache and the stats command



Allow 0 as a valid value (never close idle connections)
Increased maximum allowed value to 3600 s (1 hour)
Use 'ap_timeout_parameter_parse' to allow more flexible configuration (i.e. h, min, s, ms suffixes)
Use 'apr_time_from_sec' when applicable.
Submitted by: jailletc36
Reviewed/backported by: jim


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1701771 13f79535-47bb-0310-9956-ffa450edef68
parent c1ea031e

CHANGES

+5 −0
Original line number Diff line number Diff line
@@ -15,6 +15,11 @@ Changes with Apache 2.4.17 
  *) mod_authz_dbd: Avoid a crash when lacking correct DB access permissions.

     PR 57868. [Jose Kahan <jose w3.org>, 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

     [Christophe Jaillet]



  *) mod_dir: Prevent the internal identifier "httpd/unix-directory" from

     appearing as a Content-Type response header when requests for a directory

     are rewritten by mod_rewrite. [Eric Covener]

STATUS

+0 −12
Original line number Diff line number Diff line
@@ -109,18 +109,6 @@ RELEASE SHOWSTOPPERS: 
PATCHES ACCEPTED TO BACKPORT FROM TRUNK:

  [ start all new proposals below, under PATCHES PROPOSED. ]



  *) 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

     Measurements available in the svn commit message.

     trunk: http://svn.apache.org/r1696105

            http://svn.apache.org/r1700418

     2.4.x: http://people.apache.org/~jailletc36/socache_memcache.diff

     +1: jailletc36, ylavic, covener

     covener: please add compatibility tag to doc during backport

     jailletc36: done in r1700418





  *) Fix some spurious and/or inconsistant indent spotted by sparse

     Fix some minor style issues.

     trunk: http://svn.apache.org/r1700317

docs/manual/mod/mod_socache_memcache.xml

+39 −0
Original line number Diff line number Diff line
@@ -52,4 +52,43 @@ 


</summary>



<directivesynopsis>

<name>MemcacheConnTTL</name>

<description>Keepalive time for idle connections</description>

<syntax>MemcacheConnTTL <em>num[units]</em></syntax>

<default>MemcacheConnTTL 15s</default>

<contextlist>

<context>server config</context>

<context>virtual host</context>

</contextlist>

<compatibility>Available in Apache 2.4.17 and later</compatibility>



<usage>



    <p>Set the time to keep idle connections with the memcache server(s)

    alive (threaded platforms only).</p>

    

    <p>Valid values for <directive>MemcacheConnTTL</directive> are times

    up to one hour. 0 means no timeout.</p>



    <note><p>This timeout defaults to units of seconds, but accepts

    suffixes for milliseconds (ms), seconds (s), minutes (min), and hours (h).

    </p></note>



    <p>Before Apache 2.4.17, this timeout was hardcoded and its value was 600 usec.

    So, the closest configuration to match the legacy behaviour is to set

    <directive>MemcacheConnTTL</directive> to 1ms.</p>



    <example>

    <highlight language="config">

# Set a timeout of 10 minutes

MemcacheConnTTL 10min

# Set a timeout of 60 seconds

MemcacheConnTTL 60

    </highlight>

    </example>



</usage>

</directivesynopsis>



</modulesynopsis>

modules/cache/mod_socache_memcache.c

+55 −4
Original line number Diff line number Diff line
@@ -51,9 +51,15 @@ 
#endif



#ifndef MC_DEFAULT_SERVER_TTL

#define MC_DEFAULT_SERVER_TTL 600

#define MC_DEFAULT_SERVER_TTL    apr_time_from_sec(15)

#endif



module AP_MODULE_DECLARE_DATA socache_memcache_module;



typedef struct {

    apr_uint32_t ttl;

} socache_mc_svr_cfg;



struct ap_socache_instance_t {

    const char *servers;

    apr_memcache_t *mc;
@@ -90,6 +96,9 @@ static apr_status_t socache_mc_init(ap_socache_instance_t *ctx, 
    char *split;

    char *tok;



    socache_mc_svr_cfg *sconf = ap_get_module_config(s->module_config,

                                                     &socache_memcache_module);



    ap_mpm_query(AP_MPMQ_HARD_LIMIT_THREADS, &thread_limit);



    /* Find all the servers in the first run to get a total count */
@@ -140,7 +149,7 @@ static apr_status_t socache_mc_init(ap_socache_instance_t *ctx, 
                                        MC_DEFAULT_SERVER_MIN,

                                        MC_DEFAULT_SERVER_SMAX,

                                        thread_limit,

                                        MC_DEFAULT_SERVER_TTL,

                                        sconf->ttl,

                                        &st);

        if (rv != APR_SUCCESS) {

            ap_log_error(APLOG_MARK, APLOG_CRIT, rv, s, APLOGNO(00788)
@@ -304,6 +313,38 @@ static const ap_socache_provider_t socache_mc = { 


#endif /* HAVE_APU_MEMCACHE */



static void *create_server_config(apr_pool_t *p, server_rec *s)

{

    socache_mc_svr_cfg *sconf = apr_pcalloc(p, sizeof(socache_mc_svr_cfg));

    

    sconf->ttl = MC_DEFAULT_SERVER_TTL;



    return sconf;

}



static const char *socache_mc_set_ttl(cmd_parms *cmd, void *dummy,

                                      const char *arg)

{

    apr_interval_time_t ttl;

    socache_mc_svr_cfg *sconf = ap_get_module_config(cmd->server->module_config,

                                                     &socache_memcache_module);



    if (ap_timeout_parameter_parse(arg, &ttl, "s") != APR_SUCCESS) {

        return apr_pstrcat(cmd->pool, cmd->cmd->name,

                           " has wrong format", NULL);

    }



    if ((ttl < apr_time_from_sec(0)) || (ttl > apr_time_from_sec(3600))) {

        return apr_pstrcat(cmd->pool, cmd->cmd->name,

                           " can only be 0 or up to one hour.", NULL);

    }



    /* apr_memcache_server_create needs a ttl in usec. */

    sconf->ttl = ttl;



    return NULL;

}



static void register_hooks(apr_pool_t *p)

{

#ifdef HAVE_APU_MEMCACHE
@@ -313,8 +354,18 @@ static void register_hooks(apr_pool_t *p) 
#endif

}



static const command_rec socache_memcache_cmds[] = {

    AP_INIT_TAKE1("MemcacheConnTTL", socache_mc_set_ttl, NULL, RSRC_CONF,

                  "TTL used for the connection with the memcache server(s)"),

    { NULL }

};



AP_DECLARE_MODULE(socache_memcache) = {

    STANDARD20_MODULE_STUFF,

    NULL, NULL, NULL, NULL, NULL,

    register_hooks

    NULL,                     /* create per-dir    config structures */

    NULL,                     /* merge  per-dir    config structures */

    create_server_config,     /* create per-server config structures */

    NULL,                     /* merge  per-server config structures */

    socache_memcache_cmds,    /* table of config file commands       */

    register_hooks            /* register hooks                      */

};