Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set O_NOATIME flag on Linux #2816

Merged
merged 3 commits into from
Feb 7, 2022
Merged

Conversation

greatroar
Copy link
Contributor

@greatroar greatroar commented Jul 1, 2020

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

The O_NOATIME flag is set when opening a file on Linux, which prevents the access time from being updated on every read.

Citing Kerrisk, The Linux Programming Interface:

The O_NOATIME flag is intended for use by indexing and backup programs. Its use can significantly reduce the amount of disk activity, because repeated disk seeks back and forth across the disk are not required to read the contents of a file and to update the last access time in the file’s i-node[.]

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

restic used to do this, but the functionality was removed along with the fadvise call in #670. That solved #666, which is unrelated to O_NOATIME.

The implementation here is different: it uses fcntl, so we don't need to open the file twice when an error occurs (leading to duplicate path traversals). Errors are expected to be common, as O_NOATIME is only available to the owner of a file or root. Also, fcntl can set O_DIRECT on FreeBSD and is needed to set F_NOCACHE on MacOS, in case we ever want to do that.

Checklist

  • I have read the Contribution Guidelines
  • I have enabled maintainer edits for this PR
  • I have added tests for all changes in this PR
  • I have 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

Copy link
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 rebased the PR, moved the setFlags call to fs_local.go (see commit message) and updated to test.

@greatroar Can you have a look at my modifications to the setFlags usages?

@MichaelEischer
Copy link
Member

@greatroar Can you have a look at my modifications to your PR?

greatroar and others added 3 commits February 6, 2022 15:00
Citing Kerrisk, The Linux Programming Interface:

    The O_NOATIME flag is intended for use by indexing and backup
    programs. Its use can significantly reduce the amount of disk
    activity, because repeated disk seeks back and forth across the
    disk are not required to read the contents of a file and to update
    the last access time in the file’s i-node[.]

restic used to do this, but the functionality was removed along with the
fadvise call in restic#670.
The archiver uses FS.OpenFile, where FS is an instance of the FS
interface. This is different from fs.OpenFile, which uses the OpenFile
method provided by the fs package.
@greatroar
Copy link
Contributor Author

Merged the t.Log message into t.Skip setflags_linux_test.go. Your changes LGTM.

@MichaelEischer MichaelEischer merged commit cc90f2b into restic:master Feb 7, 2022
@MichaelEischer
Copy link
Member

Thanks for checking.

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.

None yet

2 participants