Skip to content

mount: detect mountpoint does not exist before opening the repository#4590

Merged
MichaelEischer merged 1 commit intorestic:masterfrom
renard:optimize-mount-failure
Dec 24, 2023
Merged

mount: detect mountpoint does not exist before opening the repository#4590
MichaelEischer merged 1 commit intorestic:masterfrom
renard:optimize-mount-failure

Conversation

@renard
Copy link
Copy Markdown

@renard renard commented Dec 12, 2023

What does this PR change? What problem does it solve?

Bug #1681 suggests that restic should not be nice to user and should refrain from creating a mountpoint if it does not exist. Nevertheless, it currently opens the repository before checking for the mountpoint's existence. In the case of large or remote repositories, this process can be time-consuming, delaying the inevitable outcome.

/restic mount --repo=REMOTE --verbose /tmp/backup
repository 33f14e42 opened (version 2, compression level max)
[0:38] 100.00%  162 / 162 index files loaded
Mountpoint /tmp/backup doesn't exist
stat /tmp/backup: no such file or directory

real	0m39.534s
user	1m53.961s
sys	0m3.044s

In this scenario, 40 seconds could have been saved if the nonexistence of the path had been verified beforehand.

Was the change previously discussed in an issue or on the forum?

This patch relocates the mountpoint check to the beginning of the runMount function, preceding the opening of the repository.

/restic mount --repo=REMOTE --verbose /tmp/backup
Mountpoint /tmp/backup doesn't exist
stat /tmp/backup: no such file or directory

real	0m0.136s
user	0m0.018s
sys	0m0.027s

Checklist

  • I have read the contribution guidelines.
  • I have enabled maintainer edits.
  • I have added tests for all code changes.
  • I have added documentation for relevant changes (in the manual).
  • There's a new file in changelog/unreleased/ that describes the changes for our users (see template).
  • 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.

@renard renard force-pushed the optimize-mount-failure branch 2 times, most recently from 5948ccf to f3489e8 Compare December 12, 2023 21:38
Bug restic#1681 suggests that restic should not be nice to user and should
refrain from creating a mountpoint if it does not exist. Nevertheless,
it currently opens the repository before checking for the mountpoint's
existence. In the case of large or remote repositories, this process
can be time-consuming, delaying the inevitable outcome.

    /restic mount --repo=REMOTE --verbose /tmp/backup
    repository 33f14e42 opened (version 2, compression level max)
    [0:38] 100.00%  162 / 162 index files loaded
    Mountpoint /tmp/backup doesn't exist
    stat /tmp/backup: no such file or directory

    real	0m39.534s
    user	1m53.961s
    sys	0m3.044s

In this scenario, 40 seconds could have been saved if the nonexistence
of the path had been verified beforehand.

This patch relocates the mountpoint check to the beginning of the
runMount function, preceding the opening of the repository.

    /restic mount --repo=REMOTE --verbose /tmp/backup
    Mountpoint /tmp/backup doesn't exist
    stat /tmp/backup: no such file or directory

    real	0m0.136s
    user	0m0.018s
    sys	0m0.027s

Signed-off-by: Sébastien Gross <seb•ɑƬ•chezwam•ɖɵʈ•org>
@MichaelEischer MichaelEischer added this pull request to the merge queue Dec 24, 2023
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 slightly tweaked the changelog entry.

@renard
Copy link
Copy Markdown
Author

renard commented Dec 24, 2023

LGTM. I've slightly tweaked the changelog entry.

Thanks. Let me know if there are better best practices / phrasings for the changelog.

Merged via the queue into restic:master with commit 433dd92 Dec 24, 2023
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.

2 participants