You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Sami Tolvanen <sa...@mywot.com> on 2009/01/05 12:07:42 UTC
[PATCH] apr_memcache memory leak with persistent connections
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
Re: Does apr_atomic_inc64 exist ?
Posted by Paul Querna <ch...@force-elite.com>.
Daniel May wrote:
> Yes, I agree, the coverage is sparse.
>
> Under Windows you have:
>
> InterlockedIncrement64
>
> But it is only supported under Windows Vista and Windows Server 2003 or later.
>
>
>
> In looking at /unix/apr_atomic.c, the 32-bit operations appear to be implemented as either inline asm, or
> brute force using a mutex.
>
> Does it make sense to stub them out, implementing the ones that can be supported with asm or intrinsics ?
>
+1, even a stubbed out mutex version would be nice, we can always
optimize to asm or platform stuff in a later patch release.
-Paul
RE: Does apr_atomic_inc64 exist ?
Posted by Daniel May <Da...@spryware.com>.
Yes, I agree, the coverage is sparse.
Under Windows you have:
InterlockedIncrement64
But it is only supported under Windows Vista and Windows Server 2003 or later.
In looking at /unix/apr_atomic.c, the 32-bit operations appear to be implemented as either inline asm, or
brute force using a mutex.
Does it make sense to stub them out, implementing the ones that can be supported with asm or intrinsics ?
/Daniel
-----Original Message-----
From: justin.erenkrantz@gmail.com [mailto:justin.erenkrantz@gmail.com] On Behalf Of Justin Erenkrantz
Sent: Tuesday, January 06, 2009 11:51 AM
To: Daniel May
Cc: dev@apr.apache.org
Subject: Re: Does apr_atomic_inc64 exist ?
On Tue, Jan 6, 2009 at 7:17 AM, Daniel May <Da...@spryware.com> wrote:
> I am looking for a 64-bit counterpart to apr_atomic_inc32(). Has this been implemented ?
I don't believe that we've guaranteed 64-bit atomics anywhere. I'm
not sure how much underlying OS/architecture support there would
generically be for 64-bit atomics... -- justin
Re: Does apr_atomic_inc64 exist ?
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Daniel May wrote:
> What is the APR standard for utilizing functions that may or may not be present in a specific version of an OS ?
> For example, there are two ways, at run time, to detect the presence of InterlockedIncrement64() under Windows.
>
> A. You could call a LoadLibrary/GetProcAddress combination on the function, and use it if you succeed in getting a valid function pointer.
> B. You can make Kernel32.dll delayload, and only attempt to call InterlockedIncrement64() if the proper OS version is detected at run time that is known to support InterlockedIncrement64().
We've implemented A. above, along with the opportunity to test the osver.
Beginning to think we need an apr_os_ call to identify if we are on some
minimal version of a given OS or OS family (say either generic BSD, or
rather test FreeBSD, OpenBSD, NetBSD). Complimentary to this might be
an interface to ask if a particular patch/patch level is installed.
This idea is still pretty fuzzy, but I've noted plenty of XXX Doesn't work
prior to rev XXX of FooOS comments throughout the ifdef tests. These are
really not useful since you don't gain the functionality by upgrading until
you have rebuilt for the new OS level or patch level.
RE: Does apr_atomic_inc64 exist ?
Posted by Daniel May <Da...@spryware.com>.
What is the APR standard for utilizing functions that may or may not be present in a specific version of an OS ?
For example, there are two ways, at run time, to detect the presence of InterlockedIncrement64() under Windows.
A. You could call a LoadLibrary/GetProcAddress combination on the function, and use it if you succeed in getting a valid function pointer.
B. You can make Kernel32.dll delayload, and only attempt to call InterlockedIncrement64() if the proper OS version is detected at run time that is known to support InterlockedIncrement64().
I guess the same can be done for processor specific features at run time.
/Daniel
-----Original Message-----
From: Jim Jagielski [mailto:jim@jaguNET.com]
Sent: Wednesday, January 07, 2009 7:46 AM
To: Justin Erenkrantz
Cc: Daniel May; dev@apr.apache.org
Subject: Re: Does apr_atomic_inc64 exist ?
On Jan 6, 2009, at 12:50 PM, Justin Erenkrantz wrote:
> On Tue, Jan 6, 2009 at 7:17 AM, Daniel May <Da...@spryware.com>
> wrote:
>> I am looking for a 64-bit counterpart to apr_atomic_inc32(). Has
>> this been implemented ?
>
> I don't believe that we've guaranteed 64-bit atomics anywhere. I'm
> not sure how much underlying OS/architecture support there would
> generically be for 64-bit atomics... -- justin
>
Unless we do it with mutexes... ugly. But we do have places
in the code where the lack of 64bit "atomics" are a concern...
PS: Am I the only one who, when I see "atomics" think of Patrick
Stewart in the old Dune movie screaming: "Atomics!" ??
Re: Does apr_atomic_inc64 exist ?
Posted by Jim Jagielski <ji...@jaguNET.com>.
On Jan 6, 2009, at 12:50 PM, Justin Erenkrantz wrote:
> On Tue, Jan 6, 2009 at 7:17 AM, Daniel May <Da...@spryware.com>
> wrote:
>> I am looking for a 64-bit counterpart to apr_atomic_inc32(). Has
>> this been implemented ?
>
> I don't believe that we've guaranteed 64-bit atomics anywhere. I'm
> not sure how much underlying OS/architecture support there would
> generically be for 64-bit atomics... -- justin
>
Unless we do it with mutexes... ugly. But we do have places
in the code where the lack of 64bit "atomics" are a concern...
PS: Am I the only one who, when I see "atomics" think of Patrick
Stewart in the old Dune movie screaming: "Atomics!" ??
Re: Does apr_atomic_inc64 exist ?
Posted by Dale Ghent <da...@elemental.org>.
On Jan 6, 2009, at 12:50 PM, Justin Erenkrantz wrote:
> On Tue, Jan 6, 2009 at 7:17 AM, Daniel May <Da...@spryware.com>
> wrote:
>> I am looking for a 64-bit counterpart to apr_atomic_inc32(). Has
>> this been implemented ?
>
> I don't believe that we've guaranteed 64-bit atomics anywhere. I'm
> not sure how much underlying OS/architecture support there would
> generically be for 64-bit atomics... -- justin
FWIW, Solaris's status is defined in the atomic_inc(3C) man page:
http://docs.sun.com/app/docs/doc/816-5168/atomic-inc-3c?a=view
Refer to the page's "See Also" section for the other variants and
inverse operators.
/dale
Re: Does apr_atomic_inc64 exist ?
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Tue, Jan 6, 2009 at 7:17 AM, Daniel May <Da...@spryware.com> wrote:
> I am looking for a 64-bit counterpart to apr_atomic_inc32(). Has this been implemented ?
I don't believe that we've guaranteed 64-bit atomics anywhere. I'm
not sure how much underlying OS/architecture support there would
generically be for 64-bit atomics... -- justin
Does apr_atomic_inc64 exist ?
Posted by Daniel May <Da...@spryware.com>.
I am looking for a 64-bit counterpart to apr_atomic_inc32(). Has this been implemented ?
Daniel May
Re: [PATCH] apr_memcache memory leak with persistent connections
Posted by Sami Tolvanen <sa...@mywot.com>.
On Tue, Jan 06, 2009 at 12:20:51PM +1100, Bojan Smojver wrote:
> Would you mind opening a bug and attaching the patch, so it doesn't fall
> through the cracks?
Sure, here you go:
https://issues.apache.org/bugzilla/show_bug.cgi?id=46482
Sami
Re: [PATCH] apr_memcache memory leak with persistent connections
Posted by Bojan Smojver <bo...@rexursive.com>.
On Mon, 2009-01-05 at 13:07 +0200, Sami Tolvanen wrote:
> 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.
Looks pretty good to me (without actually testing it). Pools/sub-pools
and resource list are always tricky...
Would you mind opening a bug and attaching the patch, so it doesn't fall
through the cracks?
https://issues.apache.org/bugzilla/
--
Bojan