Closed GoogleCodeExporter closed 9 years ago
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
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:
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
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
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
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:
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
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
This issue was closed by revision r316.
Original comment by dmoore4...@gmail.com
on 12 Feb 2011 at 4:48
Original issue reported on code.google.com by
dmoore4...@gmail.com
on 11 Feb 2011 at 4:09