Re: svn commit: r1696105 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_socache_memcache.xml modules/cache/mod_socache_memcache.c
On Mon, Aug 24, 2015 at 11:39 PM, Christophe JAILLET christophe.jail...@wanadoo.fr wrote: Le 24/08/2015 11:18, Yann Ylavic a écrit : I don't know if memcached can be configured to always close the connections when done (I guess so), but since the minimal TTL is 1 second, we could end up reusing connections closed remotely (without recovering on error) in this case. AFAIK, memcached can not be configured to close connections by itself. No keepalive/idle timeout? Apparently all mpm are configured to ignore SIGPIPE, so apr_socket_send() should just return the error and the logic in apr_memcache should handle it. I.e.: ms_bad_conn(ms, conn); apr_memcache_disable_server(mc, ms); So why wouldn't we recover? We would, but AFAICS not for the current connection (and that's suitable). If the connection is closed for whatever reason, wouldn't the server be marked as DEAD? Yes, but so will the current request which has been sent to a sink, should the remote have closed the idle connection (under us), with an error returned to the socache. The TTL aims to be synchronized with the remote idle timeout, so that on the client side (with TTL remote-idle-timeout) we never send anything to a connection already (half-)closed remotely, and are sure that any send() error comes from an interrupted connection (for whatever reason, e.g. memcached restart). I mean either memcached (or a fork) handles an idle timeout, or the TTL won't help here. For forcible closures detection, we'd need something like ap_proxy_is_socket_connected() before reusing it (but that's also racy and BTW should be done in apr_memcache). So maybe our best option is to use ap_timeout_parameter_parse() to set sconf-ttl, which would allow a TTL of 1 microsecond (min), and almost no max (native 64bit apr_time_t)... TTL value passed to apr_memcache_server_create is a apr_uint32_t. It is then passed as-is to apr_reslist_create as a apr_interval_time_t (i.e. apr_int64_t) So, there *IS* a max value of 2^32 usec = 4294 s ~ 3600 s = 1h Indeed, ap_timeout_parameter_parse() would be nice. This would allow TTL in millisecond (ms). min is for minutes. However having an upper limit of 4294 s (or 3600 in order to have a round value) still makes sense to me. I'm fine with the upper limit too (though INT64_MAX hours would be kind of an infinity :p ). I was just worried about the lower one with agressive ( 1s) idle timeouts (if that ever exists), and the compatibility with the ineffective but legacy/working hard TTL 1ms (600us)... Anyway, thanks for the commit Christophe, you are probably right we shouldn't overengineer this if not necessary.
Re: svn commit: r1696105 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_socache_memcache.xml modules/cache/mod_socache_memcache.c
Hi, Yann and thanks for the review.
Le 21/08/2015 15:47, Yann Ylavic a écrit :
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...
Obviously 0 should be allowed, but I don't see any reason for -1. What
difference would you make between the 2?
INT_MAX on a 32 bits system is 2147483647. I considered that giving the
TTL in sec was enough and more standard. This has to be converted to sec.
So, the maximum allowed value would be 2147. 1800 (half an hour) looked
a more logical value for an end user. That's why I have chosen this limit.
Maybe up to 3600 (an hour) should also be allowed as it is accepted by
apr_memcache_server_create.
+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) ?
Didn't do that because of the the different type (apr_time_t (i.e.
apr_int64_t) vs apr_uint32_t)
But using apr_time_from_sec looks good and self-document the code (i.e.
ttl in usec)
+
+return NULL;
+}
Regards,
Yann.
Re: svn commit: r1696105 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_socache_memcache.xml modules/cache/mod_socache_memcache.c
On Mon, Aug 24, 2015 at 8:17 AM, Christophe JAILLET
christophe.jail...@wanadoo.fr wrote:
Le 21/08/2015 15:47, Yann Ylavic a écrit :
On Sun, Aug 16, 2015 at 12:05 AM, jaillet...@apache.org wrote:
+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...
Obviously 0 should be allowed, but I don't see any reason for -1. What
difference would you make between the 2?
Hm, I thought that for apr_memcache_server_create (hence
apr_reslist_create), ttl=0 was always close (immediately) and -1 was
never close (infinite), but that's not the case (ttl=0 means never
close, and -1 is invalid).
I don't know if memcached can be configured to always close the
connections when done (I guess so), but since the minimal TTL is 1
second, we could end up reusing connections closed remotely (without
recovering on error) in this case.
So maybe our best option is to use ap_timeout_parameter_parse() to set
sconf-ttl, which would allow a TTL of 1 microsecond (min), and almost
no max (native 64bit apr_time_t)...
Regards,
Yann.
Re: svn commit: r1696105 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_socache_memcache.xml modules/cache/mod_socache_memcache.c
Le 24/08/2015 11:18, Yann Ylavic a écrit :
On Mon, Aug 24, 2015 at 8:17 AM, Christophe JAILLET
christophe.jail...@wanadoo.fr wrote:
Le 21/08/2015 15:47, Yann Ylavic a écrit :
On Sun, Aug 16, 2015 at 12:05 AM, jaillet...@apache.org wrote:
+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...
Obviously 0 should be allowed, but I don't see any reason for -1. What
difference would you make between the 2?
Hm, I thought that for apr_memcache_server_create (hence
apr_reslist_create), ttl=0 was always close (immediately) and -1 was
never close (infinite), but that's not the case (ttl=0 means never
close, and -1 is invalid).
Correct.
I don't know if memcached can be configured to always close the
connections when done (I guess so), but since the minimal TTL is 1
second, we could end up reusing connections closed remotely (without
recovering on error) in this case.
AFAIK, memcached can not be configured to close connections by itself.
Apparently all mpm are configured to ignore SIGPIPE, so
apr_socket_send() should just return the error and the logic in
apr_memcache should handle it.
I.e.:
ms_bad_conn(ms, conn);
apr_memcache_disable_server(mc, ms);
So why wouldn't we recover? If the connection is closed for whatever
reason, wouldn't the server be marked as DEAD?
Following calls to apr_memcache_find_server_hash_default would try it
again after 5 s.
So maybe our best option is to use ap_timeout_parameter_parse() to set
sconf-ttl, which would allow a TTL of 1 microsecond (min), and almost
no max (native 64bit apr_time_t)...
Regards,
Yann.
TTL value passed to apr_memcache_server_create is a apr_uint32_t.
It is then passed as-is to apr_reslist_create as a apr_interval_time_t
(i.e. apr_int64_t)
So, there *IS* a max value of 2^32 usec = 4294 s ~ 3600 s = 1h
Indeed, ap_timeout_parameter_parse() would be nice. This would allow TTL in
millisecond (ms). min is for minutes.
However having an upper limit of 4294 s (or 3600 in order to have a round
value) still makes sense to me.
CJ
Re: svn commit: r1696105 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_socache_memcache.xml modules/cache/mod_socache_memcache.c
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.
