Does stat_cache grow unbounded?

[GoogleCodeExporter]

I never really noticed the stat_cache but after understanding it a little bit 
more, it looks like it may grow unbounded.

If this is the case, then this might be a contributor to what some users have 
reported as unusual memory usage. That is, why does such a small program 
consume so much memory?

If stat_cache does grow unbounded then something that helps performance may 
have a fatal side effect, in which case this would need to regulated.

Original issue reported on code.google.com by dmoore4...@gmail.com on 11 Feb 2011 at 4:09

[GoogleCodeExporter]

It most certainly does grow unbound! That's actually what I was looking into 
with the last readdir() patch. I have yet to run some tests, but I think the 
biggest contributor to memory usage isn't in the stat cache, but the way 
s3fs_readdir() gets file attributes. For each object in a bucket location, 
s3fs_readdir creates a curl_multi handle. Then, all at once, it initiates every 
http request queued in curl_multi_handle. For a large directory, that's equal 
to the value of 'max-keys' (currently 500).

As for the stat_cache, I'll run some more tests today to see what kind of 
memory usage we're dealing with. Hopefully that'll give us an idea on how to 
approach regulating its growth.

Either way, I think it goes without saying, s3fs_readdir could use some serious 
attention : )

Original comment by ben.lema...@gmail.com on 11 Feb 2011 at 4:46

[GoogleCodeExporter]

So I've ran a few basic tests, here's the (simulated) memory usage from a stat 
cache with 3,000 entries, and a cache with 100,000 entries:

$ mkdir 3000_files && cd $_
$ for x in `seq 1 3000`; do echo "foo" >> $x.txt; done
$ valgrind --leak-check=full /home/ben/code/stat_cache.cpp

==31754== Memcheck, a memory error detector
==31754== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==31754== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for 
copyright info
==31754== Command: ./stat_cache
==31754== 
==31754== 
==31754== HEAP SUMMARY:
==31754==     in use at exit: 0 bytes in 0 blocks
==31754==   total heap usage: 18,014 allocs, 18,014 frees, 1,260,318 bytes 
allocated
==31754== 
==31754== All heap blocks were freed -- no leaks are possible
==31754== 
==31754== For counts of detected and suppressed errors, rerun with: -v
==31754== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 4)

1,260,318 bytes (1.2Mb)

$ mkdir 3000_files && cd $_
$ for x in `seq 1 100000`; do echo "foo" >> $x.txt; done
$ valgrind --leak-check=full /home/ben/code/stat_cache.cpp

==31784== Memcheck, a memory error detector
==31784== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==31784== Using Valgrind-3.6.0.SVN-Debian and LibVEX; rerun with -h for 
copyright info
==31784== Command: ./stat_cache
==31784== 
==31784== 
==31784== HEAP SUMMARY:
==31784==     in use at exit: 0 bytes in 0 blocks
==31784==   total heap usage: 600,014 allocs, 600,014 frees, 41,900,338 bytes 
allocated
==31784== 
==31784== All heap blocks were freed -- no leaks are possible
==31784== 
==31784== For counts of detected and suppressed errors, rerun with: -v
==31784== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 4)
41,900,338 bytes (39.9Mb)

While it definitely grows unbounded, for most people I'd have to assume this 
isn't a _HUGE_ issue.

Any thoughts on regulation?

Original comment by ben.lema...@gmail.com on 11 Feb 2011 at 6:13

Attachments:

• stat_cache.cpp

[GoogleCodeExporter]

Yes s3fs_readdir() is relatively complex as mentioned in issue #148, comment 14:
http://code.google.com/p/s3fs/issues/detail?id=148#c14

The keys = 500 was originally 50, someone recommended changing this to 1000, I 
took the middle road.

It's nice to see 2nd party confirmation that the leak issues have been resolved 
-- there were several sources for the leaks.

regulation? Hmmm, map::count can return the number of entries easily, so a hard 
limit on the number of entries can be made.  Another member to the structure 
indicating a hit count, when the map::count grows too big, throw away the 
entries with a relatively low hit count.  

But, then there may be other more serious culprits for unbounded memory growth. 
 I recall an email from a user and then the stats_cache made me think that this 
may be a contributor.

Original comment by dmoore4...@gmail.com on 11 Feb 2011 at 7:17

[GoogleCodeExporter]

I really like the idea of a hit count, possibly in combination with 
memcache-esque FIFO invalidation. I'll see if I can whip something up!

p.s., ended up @ suncup.net and noticed you're in CO, me too :) (boulder)

Original comment by ben.lema...@gmail.com on 11 Feb 2011 at 8:20

[GoogleCodeExporter]

OT: suncup.net yeah, it needs some work, especially after I recently dumped a 
hosting company and went to hosting/email off of an EC2 instance

Original comment by dmoore4...@gmail.com on 11 Feb 2011 at 9:43

[GoogleCodeExporter]

Attached is a patch which limits the number of stat cache entries to 10,000 
(~4Mb). At this point it's nothing fancy. If the cache is full, a call to 
add_stat_cache_entry() will pop entries off the stack, lowest hit count first. 

Thoughts/Suggestions? What about this being a command line option? For my use 
case I'd probably set this around 200K entries.

Original comment by ben.lema...@gmail.com on 11 Feb 2011 at 11:59

Attachments:

• s3fs_limit_stat_cache_r314.diff

[GoogleCodeExporter]

Compile time warning:

s3fs.cpp: In function ‘void add_stat_cache_entry(const char*, stat*)’:
s3fs.cpp:3486: warning: comparison between signed and unsigned integer 
expressions

Original comment by dmoore4...@gmail.com on 12 Feb 2011 at 4:39

[GoogleCodeExporter]

Implemented in r315, made the minor fix to the data type:

static unsigned long max_stat_cache_size = 10000;

Maybe this size is sufficient for most users, but I'll make it an option as 
well.

Original comment by dmoore4...@gmail.com on 12 Feb 2011 at 3:05

[GoogleCodeExporter]

This issue was closed by revision r316.

Original comment by dmoore4...@gmail.com on 12 Feb 2011 at 4:48

• Changed state: Fixed