Enable auto cache and disable compression on sshfs mounts #1605

jasonmccallister · 2020-06-17T18:42:01Z

This is related to #1502

sshfs has a default cache duration of 20 seconds. Since multipass is a local virtual machine, it might make sense to disable compression (as we are less worried about the network traffic) and auto caching enables caching based on modification times.

Note: this came out of research and trial and error based on this SE post: https://superuser.com/a/1359783/215387

Happy to discuss the options in further detail, but the default 20 second cache is rather noticeable in our project.

Signed-off-by: Jason McCallister <jason@craftcms.com>

Saviq · 2020-06-23T15:54:54Z

Hi @jasonmccallister, thanks for this!

You will need to update the tests:

[  FAILED  ] 3 tests, listed below:
[  FAILED  ] SshfsMountSuccess/SshfsMountExecute.test_succesful_invocation/0, where GetParam() = ("target", { ("sudo env LD_LIBRARY_PATH=/foo/bar /baz/bin/sshfs -V", "FUSE library version: 2.9.0\n"), ("sudo env LD_LIBRARY_PATH=/foo/bar /baz/bin/sshfs -o slave -o transform_symlinks -o allow_other -o nonempty :\"source\" \"/home/ubuntu/target\"", "don't care\n") })
[  FAILED  ] SshfsMountSuccess/SshfsMountExecute.test_succesful_invocation/1, where GetParam() = ("target", { ("sudo env LD_LIBRARY_PATH=/foo/bar /baz/bin/sshfs -V", "FUSE library version: 3.0.0\n"), ("sudo env LD_LIBRARY_PATH=/foo/bar /baz/bin/sshfs -o slave -o transform_symlinks -o allow_other :\"source\" \"/home/ubuntu/target\"", "don't care\n") })
[  FAILED  ] SshfsMountSuccess/SshfsMountExecute.test_succesful_invocation/2, where GetParam() = ("target", { ("sudo env LD_LIBRARY_PATH=/foo/bar /baz/bin/sshfs -V", "weird fuse version\n"), ("sudo env LD_LIBRARY_PATH=/foo/bar /baz/bin/sshfs -o slave -o transform_symlinks -o allow_other :\"source\" \"/home/ubuntu/target\"", "don't care\n") })
 3 FAILED TESTS

townsend2010

I'm +1 with this. I don't see any regressions and looks like it should work on older versions of sshfs & fuse if users' instances has that. All we need is the 3 tests to be fixed and it's good to go.

jasonmccallister · 2020-07-07T01:51:24Z

Thank you! I’ll update these tests tomorrow, apologies for the delay!

townsend2010 · 2020-07-07T15:23:04Z

Still have a failing test:

[  FAILED  ] 1 test, listed below:
[  FAILED  ] SshfsMountSuccess/SshfsMountExecute.test_succesful_invocation/0, where GetParam() = ("target", { ("sudo env LD_LIBRARY_PATH=/foo/bar /baz/bin/sshfs -V", "FUSE library version: 2.9.0\n"), ("sudo env LD_LIBRARY_PATH=/foo/bar /baz/bin/sshfs -o slave -o transform_symlinks -o allow_other -o auto_cache -o Compression=no :\"source\" \"/home/ubuntu/target\"", "don't care\n") })
 1 FAILED TEST

jasonmccallister · 2020-07-07T15:56:23Z

@townsend2010 I just pushed another commit. Unfortunately I can't compile this locally on macOS, so I need to use Travis :)

townsend2010 · 2020-07-07T16:08:57Z

Ugh, now clang-format is complaining and won't start the build- https://travis-ci.org/github/canonical/multipass/jobs/705843563

As an aside, it's possible to build Multipass inside a Multipass instance, just allocate enough CPU's and memory and it builds just fine.

Saviq · 2020-07-07T16:14:41Z

Ugh, now clang-format is complaining and won't start the build- https://travis-ci.org/github/canonical/multipass/jobs/705843563

Fixed that :)

codecov · 2020-07-08T14:57:53Z

Codecov Report

Merging #1605 into master will increase coverage by 1.66%.
The diff coverage is 78.64%.

@@            Coverage Diff             @@
##           master    #1605      +/-   ##
==========================================
+ Coverage   73.79%   75.45%   +1.66%     
==========================================
  Files         223      223              
  Lines        8163     8263     +100     
==========================================
+ Hits         6024     6235     +211     
+ Misses       2139     2028     -111

Impacted Files	Coverage Δ
include/multipass/process/basic_process.h	`100.00% <ø> (ø)`
include/multipass/process/process.h	`100.00% <ø> (ø)`
include/multipass/process/process_spec.h	`100.00% <ø> (ø)`
include/multipass/process/qemuimg_process_spec.h	`100.00% <ø> (ø)`
include/multipass/sshfs_mount/sshfs_mounts.h	`100.00% <ø> (ø)`
include/multipass/utils.h	`61.53% <ø> (ø)`
include/multipass/virtual_machine_factory.h	`100.00% <ø> (ø)`
src/cert/ssl_cert_provider.cpp	`87.23% <ø> (ø)`
src/client/cli/client.cpp	`100.00% <ø> (ø)`
src/client/cli/cmd/common_cli.cpp	`72.13% <ø> (ø)`
... and 60 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a0688f9...c8466d8. Read the comment docs.

jasonmccallister · 2020-07-08T16:59:19Z

Thank you for stepping in and resolving those issues @Saviq!

Is there a way to test a macOS build with this change?

townsend2010

Ok, I'm good with this.

Saviq · 2020-07-09T12:50:00Z

bors try

Saviq · 2020-07-09T13:22:14Z

@jasonmccallister the macOS package got built here: e84fa46#comments

bors · 2020-07-09T13:26:00Z

try

Build succeeded:

continuous-integration/travis-ci/push

jasonmccallister · 2020-07-09T19:13:00Z

Thanks @Saviq giving it a try now. If there are existing mounts, do they need to be readded or will updating multipass and restarting the machine suffice?

townsend2010 · 2020-07-09T19:14:23Z

Hi @jasonmccallister,

Updating Multipass and restarting the instance is fine.

townsend2010 · 2020-07-10T16:06:36Z

Hi @jasonmccallister,

Have you had a chance to try it out and if so, does this work better for your project?

Thanks!

jasonmccallister · 2020-07-10T18:25:50Z

@townsend2010 its better - still not perfect but results seem favorable in my testing - will talk to the team and get some additional feedback.

jasonmccallister · 2020-07-14T00:32:34Z

@townsend2010 there is a minor improvement on the speeds - but we are still seeing issues. What would be the performance impact in disabling the cache altogether? I think it may cause more harm than good, but it might resolve our issues.

Another idea we are considering is embedded a "watcher" in the virtual machine that will scan the directories for our needs. That seems to "force" sshfs to update its cache instantly. We do have some large files (PHP and Node dependency directories) that the watcher can ignore but wanted to get some thoughts for your team?

townsend2010 · 2020-07-21T19:59:24Z

Hey @jasonmccallister,

I did some basic testing of disabling the cache via -o dir_cache=no (which replaces -o cache=no in older sshsf versions) and I see a ~15% decrease in read speeds. I think the better short term answer is to introduce a setting that globally sets the caching policy for all Multipass mounts. I will look into doing that soon.

jasonmccallister · 2020-07-21T21:44:12Z

That makes sense, no caching I'm sure would have some impacts. What about setting the cache duration to 3 seconds?

townsend2010 · 2020-07-22T12:44:10Z

Hi @jasonmccallister,

I set the cache duration to 3 seconds and saw an ~4% decrease in read speeds which to me is acceptable. I will put up a PR with this change and after the packages build, I'd like to ask you test to see if this improves things for you.

Thanks!

townsend2010 · 2020-07-22T14:55:07Z

Hi @jasonmccallister,

Both the Linux and Mac versions of my PR are available at #1654 (comment). Please let me know in that PR how it goes for you. Thanks!

jasonmccallister · 2020-07-22T15:19:12Z

@townsend2010 I will give it a try right now. Thank you!

jasonmccallister · 2020-07-22T22:44:54Z

@townsend2010 that is a huge improvement on my side, I will ask a few other team members to test and verify but that looks like it was the right combo! Thank you!

townsend2010 · 2020-07-23T12:27:10Z

@jasonmccallister,

That is awesome! To make sure I'm not missing something on performance under "normal" (whatever "normal" is 😄) conditions, I'll get some of my teammates to try out my PR as well. Thanks again!

jasonmccallister · 2020-07-24T15:46:59Z

@townsend2010 feedback I've received from the community has been really positive with this change. Thank you!

Saviq · 2020-08-07T11:05:19Z

We're merging #1654 instead :)

enable auto cache and disable compression

0d2ac79

Signed-off-by: Jason McCallister <jason@craftcms.com>

Saviq requested a review from townsend2010 July 6, 2020 14:12

townsend2010 reviewed Jul 6, 2020

View reviewed changes

update tests

07fc2ca

fix failing test

7376d10

Fix formatting in test_sshfsmount.cpp

80d154c

angrybrad mentioned this pull request Jul 7, 2020

Template and file changes are delayed craftcms/nitro#168

Closed

Saviq added 3 commits July 7, 2020 20:06

Fix old FUSE options

0635dbd

Fix more…

fd492b7

And fix formatting again…

c8466d8

townsend2010 approved these changes Jul 9, 2020

View reviewed changes

bors bot added a commit that referenced this pull request Jul 9, 2020

Try #1605:

e84fa46

Saviq added this to the v1.4.0 milestone Jul 27, 2020

Saviq closed this Aug 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable auto cache and disable compression on sshfs mounts #1605

Enable auto cache and disable compression on sshfs mounts #1605

jasonmccallister commented Jun 17, 2020 •

edited

Saviq commented Jun 23, 2020

townsend2010 left a comment

jasonmccallister commented Jul 7, 2020

townsend2010 commented Jul 7, 2020

jasonmccallister commented Jul 7, 2020

townsend2010 commented Jul 7, 2020

Saviq commented Jul 7, 2020

codecov bot commented Jul 8, 2020 •

edited

jasonmccallister commented Jul 8, 2020 •

edited

townsend2010 left a comment

Saviq commented Jul 9, 2020

Saviq commented Jul 9, 2020

bors bot commented Jul 9, 2020

jasonmccallister commented Jul 9, 2020

townsend2010 commented Jul 9, 2020

townsend2010 commented Jul 10, 2020

jasonmccallister commented Jul 10, 2020

jasonmccallister commented Jul 14, 2020

townsend2010 commented Jul 21, 2020

jasonmccallister commented Jul 21, 2020

townsend2010 commented Jul 22, 2020

townsend2010 commented Jul 22, 2020

jasonmccallister commented Jul 22, 2020

jasonmccallister commented Jul 22, 2020

townsend2010 commented Jul 23, 2020

jasonmccallister commented Jul 24, 2020

Saviq commented Aug 7, 2020

Enable auto cache and disable compression on sshfs mounts #1605

Enable auto cache and disable compression on sshfs mounts #1605

Conversation

jasonmccallister commented Jun 17, 2020 • edited

Saviq commented Jun 23, 2020

townsend2010 left a comment

Choose a reason for hiding this comment

jasonmccallister commented Jul 7, 2020

townsend2010 commented Jul 7, 2020

jasonmccallister commented Jul 7, 2020

townsend2010 commented Jul 7, 2020

Saviq commented Jul 7, 2020

codecov bot commented Jul 8, 2020 • edited

Codecov Report

jasonmccallister commented Jul 8, 2020 • edited

townsend2010 left a comment

Choose a reason for hiding this comment

Saviq commented Jul 9, 2020

Saviq commented Jul 9, 2020

bors bot commented Jul 9, 2020

try

jasonmccallister commented Jul 9, 2020

townsend2010 commented Jul 9, 2020

townsend2010 commented Jul 10, 2020

jasonmccallister commented Jul 10, 2020

jasonmccallister commented Jul 14, 2020

townsend2010 commented Jul 21, 2020

jasonmccallister commented Jul 21, 2020

townsend2010 commented Jul 22, 2020

townsend2010 commented Jul 22, 2020

jasonmccallister commented Jul 22, 2020

jasonmccallister commented Jul 22, 2020

townsend2010 commented Jul 23, 2020

jasonmccallister commented Jul 24, 2020

Saviq commented Aug 7, 2020

jasonmccallister commented Jun 17, 2020 •

edited

codecov bot commented Jul 8, 2020 •

edited

jasonmccallister commented Jul 8, 2020 •

edited