prefix structure memory leak

[GoogleCodeExporter]

What steps will reproduce the problem?

1. memcached -D: -m 10
2. keep insert unique keys with no prefix :
3. watch resident set size of memcached process, it keeps growing without 
stopping

What is the expected output? What do you see instead?

RSS should stabilize at some point (maybe 10MB + 10%)

What version of the product are you using? On what operating system?

memcached 1.4.1, Linux CentOS 5.3

Please provide any additional information below.

settings.detail_enabled = 1;

causes calls to stats_prefix_record_*() 

these calls stats_prefix_find() which calloc() and malloc() PREFIX_STATS 
structures if not found.
Since we are keep inserting unique keys, memcached keeps calloc/malloc these 
structures. 
The only thing that frees all the memory is to call "STATS RESET"

But I think it is better for key structure to have a pointer to this 
prefix_stats structures with refcount.
When a refcount goes zero, you can free these structures. 

Original issue reported on code.google.com by tru64ufs@gmail.com on 12 Oct 2009 at 6:24

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

I think it is better not to include prefix statistics for keys without a prefix 
delimiter.

--- stats.c.orig    2009-10-12 14:14:21.000000000 +0900
+++ stats.c 2009-10-12 21:39:21.000000000 +0900
@@ -68,12 +68,18 @@
     PREFIX_STATS *pfs;
     uint32_t hashval;
     size_t length;
+    int bailout=1;

     assert(key != NULL);

     for (length = 0; key[length] != '\0' && length < nkey; length++)
-        if (key[length] == settings.prefix_delimiter)
+        if (key[length] == settings.prefix_delimiter) {
+            bailout=0;
             break;
+        }
+
+    if (bailout)
+      return NULL;

     hashval = hash(key, length, 0) % PREFIX_HASH_SIZE;

Original comment by tru64ufs@gmail.com on 12 Oct 2009 at 12:41

[GoogleCodeExporter]

I would not call it a memory leak before we actually loose track of memory... 
As you
say in your description: stats reset will release all of the allocated 
memory... Bad
design? maybe, but not a memory leak.

Original comment by trond.no...@gmail.com on 12 Oct 2009 at 1:56

[GoogleCodeExporter]

Yes, Trond, I agree that it is not a memory leak. I just wanted to get your 
attention ;-)

But the thing is that on a real production server, constant RSS in process 
memory with no clue can be a real 
pain and can lead to real performance problem in the end since it will start 
swapping due to physical memory 
requirement. And also it is not easy for us to simply issue "stats reset" 
command since we are keep track of 
these statistics in RRD file and if you reset them, the RRD file and chart will 
be all messed up.

Furthermore, we ran into problems when people forgot to put their prefix in the 
app; our memcached stats 
collection server hung with using up 100% CPU cycle collecting all that unique 
keys as prefix statistics.

Trond. I think ignoring keys with no prefix in the prefix stat is perfectly 
fine for us.
Thank you for your reply.

Original comment by tru64ufs@gmail.com on 12 Oct 2009 at 2:14

[GoogleCodeExporter]

"s/constant RSS/constant increase in RSS" above

Original comment by tru64ufs@gmail.com on 12 Oct 2009 at 2:17

[GoogleCodeExporter]

Seems reasonable

Original comment by trond.no...@gmail.com on 30 Oct 2009 at 7:40

• Changed state: Started

[GoogleCodeExporter]

Fix pushed

Original comment by trond.no...@gmail.com on 30 Oct 2009 at 4:13

• Changed state: Fixed