Skip to content

Make restic mount faster and consume less memory#2587

Merged
MichaelEischer merged 1 commit intorestic:masterfrom
aawsome:optimize-fuse
Jul 28, 2020
Merged

Make restic mount faster and consume less memory#2587
MichaelEischer merged 1 commit intorestic:masterfrom
aawsome:optimize-fuse

Conversation

@aawsome
Copy link
Copy Markdown
Contributor

@aawsome aawsome commented Feb 18, 2020

What is the purpose of this change? What does it change?

In the FUSE implementation restic data structure have been read (meaning loading blobs from the repository) and internal data structures have been created when a FUSE "node" was created. This was the case, for instance, for all item within a dir when this dir was accessed. This lead to quite some memory usage and made the FUSE filesystem "unresponsive".

This PR changes this behavior and only reads restic data structure when the information is really needed. Moreover, the internal data structure have been optimized. Also concurrent operation within FUSE should now work correctly. (FUSE documentation says that access should be synced which wasn't)

Was the change discussed in an issue or in the forum before?

closes #1680

Thanks @greatroar for issuing #2585 which makes me think about whether the bottleneck really is only reading tree blobs.

Checklist

  • I have read the Contribution Guidelines
  • I have not added tests for all changes in this PR
  • I have not added documentation for the changes (in the manual)
  • There's a new file in changelog/unreleased/ that describes the changes for our users (template here)
  • I have run gofmt on the code in all commits
  • All commit messages are formatted in the same style as the other commits in the repo
  • I'm done, this Pull Request is ready for review

@MichaelEischer
Copy link
Copy Markdown
Member

It might be an option to implement the NodeOpener which is called when a file is opened. That way the chunk size collection would happen immediately when opening a file and is not delayed until reading the first bytes.

The main difference is the point in time when the user would be notified in case some blobs are missing. The current implementation in restic probably fails with an error before providing any details on the file node, with the change this is delayed until the first bytes are read. I think that the new semantic is actually what a user would expect: Reading a damaged file fails, but usually you are able to see the metadata.

@aawsome
Copy link
Copy Markdown
Contributor Author

aawsome commented Feb 23, 2020

@MichaelEischer Thanks for searching the manual and offering the NodeOpener option which I'm using now.
I tried to do performance tests of this new version but did not really succeed to see differences when comparing this to. In fact it seems that there a lot of variation w.r.t. performance when using FUSE (maybe some kernel caching?)...
Is there anyone who can help out with testing this change?

Memory footprint is however clearly reduced.

@ProactiveServices
Copy link
Copy Markdown
Contributor

ProactiveServices commented Feb 23, 2020

@aawsome Happy to help, do you have any specific tests you'd like me to perform? I have several hundred-gig repos I can test with.

@aawsome
Copy link
Copy Markdown
Contributor Author

aawsome commented Feb 24, 2020

@ProactiveServices Thank you very much! I'm especially interested in response times and CPU usage and comparison between "plain" restic and a version including this PR.
The code changes should apply for listing and reading files, e.g. ls file (also ls dir) or cat file/cp file somewhere.

I used /usr/bin/time restic mount to get CPU timings and a simple time before the commands, e.g. time ls file to get response times.

@aawsome
Copy link
Copy Markdown
Contributor Author

aawsome commented Feb 24, 2020

With the last commit, also dir operations should be faster (and a bit less memory consuming)...

@aawsome aawsome mentioned this pull request Jun 14, 2020
6 tasks
@aawsome
Copy link
Copy Markdown
Contributor Author

aawsome commented Jun 14, 2020

rebased to master and added the changes suggested by @greatroar

@aawsome
Copy link
Copy Markdown
Contributor Author

aawsome commented Jul 12, 2020

Rebased this PR after #2790 has been merged.

@aawsome
Copy link
Copy Markdown
Contributor Author

aawsome commented Jul 12, 2020

Noticed that the new blob cache by @greatroar already implements locking; hence I removed the locks from file.go and just left a comment there.

@aawsome
Copy link
Copy Markdown
Contributor Author

aawsome commented Jul 25, 2020

rebased after #2787 has been merged.

…emory

- Add Open() functionality to dir
- only access index for blobs when file is read
- Implement NodeOpener and put one-time file stuff there
- Add comment about locking as suggested by bazil.org/fuse

=> Thanks at Michael Eischer for suggesting the last two improvements
Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I've force-pushed the branch to retrigger the CI. Seems like the flaky rclone test error has changed into a flaky rclone test crash, after #2855.

@MichaelEischer MichaelEischer merged commit 248c7c3 into restic:master Jul 28, 2020
@aawsome aawsome deleted the optimize-fuse branch December 7, 2020 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Horrible fuse mount performance on large repo with many small files

4 participants