Fix size of symlinks and enable kernel symlink cache #2285
Conversation
Symlink inodes are expected to have size equal to the length of its target. This is defined in POSIX spec. FUSE's symlink cache also expects this, so fixing this prepares the enabling of the symlink cache. Signed-off-by: Gary Guo <[email protected]>
This would provide significant savings when symlink'ed path is frequently accessed. Since symlink target is not mutable, this is always safe regardless TTL settings. (When the symlink is removed and re-created, its generatiton would change so a different inode is created). Signed-off-by: Gary Guo <[email protected]>
|
This improves perf by 10% in our workload on top of #2269. |
|
Thanks @nbdd0121 for the contribution!! Changes look good to me, but requires a couple of changes to make it mergeable. (a) We need to guard the symlink caching under a flag like, --experimental-enable-symlink-cache (by default disabled). Keeping this experimental as we haven't performed all the testing. Note: Adding an experimental flag would be trivial if you do this after 1 week. We are making the changes to simplify those path. |
|
@nbdd0121 Please find the steps to add a new flag, here - https://github.com/GoogleCloudPlatform/gcsfuse/blob/master/docs/dev_guide.md#adding-a-new-param-in-gcsfuse |
|
Tried to write a test but adding the test to symlink_test.go doesn't seem trivial to me. The test that exists there don't use (My experience is mostly with C and Rust and I have no experience with Go at all.) |
|
Hi @nbdd0121, Sorry for the delay here! We were busy putting together a formal development guide, and it's finally ready: https://github.com/GoogleCloudPlatform/gcsfuse/blob/master/docs/dev_guide.md#dev-guide Please give it a look and follow the instructions – looking forward to get these changes merged. Thanks! |
Description
Symlink inodes are expected to have size equal to the length of its target. This is defined in POSIX spec.
FUSE's symlink cache also expects this, so fixing this allows the enabling of the symlink cache. Note that since symlink targets are not mutable, the cache is always valid while the inode is valid, so there is no need for any TTL or invalidation logic.
This is expected to bring significant time saving for workloads that make use of symlinks (especially for a directory symlink, since all ops inside the symlink directory would require reading the symlink at least once).
Link to the issue in case of a bug fix.
Fix #2273
Testing details