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.
