Hello all,
We ran into a problem with apr_memcache after upgrading from httpd 2.0
to 2.2. Depending on the TTL passed to apr_memcache_server_create in our
Apache module, we were either leaking lots of memory or grinding to a
halt a few seconds after httpd was started due to an overwhelming number
of connections to memcached servers.
This turned out to be a known problem with apr_memcache, which we didn't
trigger with the apr_reslist implementation used in httpd 2.0 / APR-util
0.9.x. It seems to have been first reported ~two years ago:
[1] http://issues.outoforder.cc/view.php?id=68
[2] http://tinyurl.com/apr-dev-200701-memcache
[3] http://tinyurl.com/apr-dev-200810-memcache
To sum it up, the problem is that memcache functions keep allocating
memory from a connection-specific pool during the entire lifetime of the
connection. Therefore, for a process that constantly performs requests,
any reasonable TTL parameter means there are server connections that are
never closed and just keep consuming more memory.
The patch in [3] seems to mostly mitigate the memory leak issue,
but addresses only apr_memcache_getp and doesn't work for multiline
data. I ended up writing the attached patch (against APR-util 1.3.4),
which allocates bucket brigades from a temporary pool stored in
apr_memcache_conn_t and clears it after each request. We have used this
in production for a couple of days now and the memory leak problem with
persistent connections is gone. There doesn't seem to be any noticeable
performance impact either, at least for our work load.
I would appreciate it if someone more familiar with the memcache code
could review the patch and let me know if there are any gotchas with
this approach. Unless there's a better way for fixing the memory leak
without modifying the API, I propose applying this patch to APR-util as
more than one apr_memcache user seems to have been affected over the
years.
Sami
Index: memcache/apr_memcache.c
===================================================================
--- memcache/apr_memcache.c (revision 38)
+++ memcache/apr_memcache.c (working copy)
@@ -25,8 +25,8 @@
char *buffer;
apr_size_t blen;
apr_pool_t *p;
+ apr_pool_t *tp;
apr_socket_t *sock;
- apr_bucket_alloc_t *balloc;
apr_bucket_brigade *bb;
apr_bucket_brigade *tb;
apr_memcache_server_t *ms;
@@ -224,12 +224,29 @@
static apr_status_t ms_find_conn(apr_memcache_server_t *ms, apr_memcache_conn_t **conn)
{
+ apr_status_t rv;
+ apr_bucket_alloc_t *balloc;
+ apr_bucket *e;
+
#if APR_HAS_THREADS
- return apr_reslist_acquire(ms->conns, (void **)conn);
+ rv = apr_reslist_acquire(ms->conns, (void **)conn);
#else
*conn = ms->conn;
- return APR_SUCCESS;
+ rv = APR_SUCCESS;
#endif
+
+ if (rv != APR_SUCCESS) {
+ return rv;
+ }
+
+ balloc = apr_bucket_alloc_create((*conn)->tp);
+ (*conn)->bb = apr_brigade_create((*conn)->tp, balloc);
+ (*conn)->tb = apr_brigade_create((*conn)->tp, balloc);
+
+ e = apr_bucket_socket_create((*conn)->sock, balloc);
+ APR_BRIGADE_INSERT_TAIL((*conn)->bb, e);
+
+ return rv;
}
static apr_status_t ms_bad_conn(apr_memcache_server_t *ms, apr_memcache_conn_t *conn)
@@ -243,6 +260,7 @@
static apr_status_t ms_release_conn(apr_memcache_server_t *ms, apr_memcache_conn_t *conn)
{
+ apr_pool_clear(conn->tp);
#if APR_HAS_THREADS
return apr_reslist_release(ms->conns, conn);
#else
@@ -322,8 +340,8 @@
{
apr_status_t rv = APR_SUCCESS;
apr_memcache_conn_t *conn;
- apr_bucket *e;
apr_pool_t *np;
+ apr_pool_t *tp;
apr_memcache_server_t *ms = params;
rv = apr_pool_create(&np, pool);
@@ -331,6 +349,12 @@
return rv;
}
+ rv = apr_pool_create(&tp, np);
+ if (rv != APR_SUCCESS) {
+ apr_pool_destroy(np);
+ return rv;
+ }
+
#if APR_HAS_THREADS
conn = malloc(sizeof( apr_memcache_conn_t )); /* non-pool space! */
#else
@@ -338,6 +362,7 @@
#endif
conn->p = np;
+ conn->tp = tp;
rv = apr_socket_create(&conn->sock, APR_INET, SOCK_STREAM, 0, np);
@@ -349,16 +374,10 @@
return rv;
}
- conn->balloc = apr_bucket_alloc_create(conn->p);
- conn->bb = apr_brigade_create(conn->p, conn->balloc);
- conn->tb = apr_brigade_create(conn->p, conn->balloc);
conn->buffer = apr_palloc(conn->p, BUFFER_SIZE);
conn->blen = 0;
conn->ms = ms;
- e = apr_bucket_socket_create(conn->sock, conn->balloc);
- APR_BRIGADE_INSERT_TAIL(conn->bb, e);
-
rv = conn_connect(conn);
if (rv != APR_SUCCESS) {
apr_pool_destroy(np);
