s3fs does not cache directory attributes

[GoogleCodeExporter]

I've been attempting to improve the performance of s3fs, running some tests, it 
appears s3fs isn't caching directory attributes:

$ mkdir -p <$MOUNT_PATH>/test/one/two/three/four
$ time cd /mnt/cloudfront/test/one/two/three/four
  real    0m1.061s

# run a second time
$ time cd /mnt/cloudfront/test/one/two/three/four
  real    0m0.739s

I've created a patch, here's the initial results:
$  time cd /mnt/cloudfront/test/one/two/three/four
   real    0m0.669s

# run a second time
$ time cd /mnt/cloudfront/test/one/two/three/four
  real    0m0.000s

I'm not sure if there's a reason for this behavior though, it looks like this 
could be a useful performance improvement :) Any thoughts?

revision r308

Original issue reported on code.google.com by ben.lema...@gmail.com on 7 Feb 2011 at 6:52

Attachments:

• s3fs_getattr_stat_cache.diff

[GoogleCodeExporter]

Another possible improvement here, in s3fs_getattr(), for some reason if 
there's a cache hit, the cached item is then erased. 

Following the above steps above, if you were to add a third step:

$ time cd /mnt/cloudfront/test/one/two/three
  real    0m0.769s

The cache has been removed. Attached is an updated patch to address this issue.

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

Attachments:

• s3fs_getattr_stat_cache_v2.diff

[GoogleCodeExporter]

Ben, let us know when you think that these patches have been thoroughly tested, 
we'll be happy to include them.

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

[GoogleCodeExporter]

After running a few tests I ran into some cache invalidation issues. I've 
addressed them and I think we're ready with the latest version of the patch 
(attached).

"There are only two hard problems in Computer Science:
   cache invalidation and naming things."

   -- Phil Karlton

Original comment by ben.lema...@gmail.com on 8 Feb 2011 at 5:56

Attachments:

• s3fs_getattr_stat_cache_v3.diff
• getattr_cache_testing.txt

[GoogleCodeExporter]

Hi Ben,

Thanks for the patch and the tests :) I went ahead and committed it with r309.

Original comment by apetresc on 8 Feb 2011 at 6:24

• Changed state: Fixed

[GoogleCodeExporter]

Great to hear! There is one strange thing, I've found an issue while attempting 
to rename directories/objects. Attempting something like:

$ touch foo
$ mv foo bar
s3fs_getattr[path=/testing/bar]
s3fs_getattr[path=/testing/foo]
        stat cache hit
s3fs_getattr[path=/testing/bar]
s3fs_getattr[path=/testing/bar]
rename[from=/testing/foo][to=/testing/bar]
[hangs here...]
[syslog output:]
Feb  8 15:53:40 tuberculosis s3fs: rename [from=/testing/foo] [to=/testing/bar]
Feb  8 15:53:40 tuberculosis s3fs:    rename: calling stat on fullpath: 

I figured this must have something to do with the latest patch, however when I 
tested on 1.35 I noticed the same problem. Modifying s3fs_rename() to call 
fstat() rather that stat() seems to fix the problem. 

With that fixed, I realized s3fs_unlink is called from 
rename_[object,directory], so I was able to remove a potential bug in the 
previous patch by removing a call to delete_stat_cache_entry() in 
s3fs_rename(). Hence the follow-up on this bug.

Hope that all makes sense :\ Attached is an *additional* patch.

Thanks again guys, s3fs rocks!

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

Attachments:

• s3fs_stat_hang.diff

[GoogleCodeExporter]

Well how about that? you rock! the change from stat() to fstat() in 
s3fs_rename() solves the hang on "git init" issue #129

I can't explain it, but maybe someone can?

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

[GoogleCodeExporter]

r309 breaks "make check" -- see issue #152

Original comment by dmoore4...@gmail.com on 9 Feb 2011 at 5:27