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

RFC - [content-store] Commit should check first fsync() error #10158

Closed
fuweid opened this issue May 2, 2024 · 1 comment · Fixed by #10171
Closed

RFC - [content-store] Commit should check first fsync() error #10158

fuweid opened this issue May 2, 2024 · 1 comment · Fixed by #10171

Comments

@fuweid
Copy link
Member

fuweid commented May 2, 2024

Description

In the #10128, the content-store's Commit API fsync dirty-pages before enter bbolt transaction to avoid long mutex contention. However, the commit doesn't check the error from fsync.

var innerErr error
// We pre-sync the in-flight writes to the disk. This avoids the [subsequent fp.Sync() call]
// (https://github.com/containerd/containerd/blob/c4c3c6ea568ce0cfbcf754863abadeea37d77c8f/plugins/content/local/writer.go#L95)
// from taking too long (10s+) while holding the metadata database lock as in the following
// `update` transaction. We intentionally ignore any error on Sync() because it will be
// handled by the subsequent `fp.Sync` anyway.
nw.Sync()

Checked https://wiki.postgresql.org/wiki/Fsync_Errors and I think we should check it before we enter bbolt transaction just in case that we lost the error, even if we call fsync again in the transaction.

Steps to reproduce the issue

No Idea about how to reproduce fsync error.

We might introduce dmflaky or something like - https://github.com/kdave/xfstests/blob/0e5c12dfd008efc2848c98108c9237487e91ef35/tests/generic/108#L5-L12

Describe the results you received and expected

Should fast-fail

What version of containerd are you using?

4167416

Any other relevant information

No response

Show configuration if it is related to CRI plugin.

No response

@fuweid
Copy link
Member Author

fuweid commented May 2, 2024

ping @xinyangge-db @dmcgowan @AkihiroSuda

fuweid added a commit to fuweid/containerd that referenced this issue May 4, 2024
Close: containerd#10158

Signed-off-by: Wei Fu <fuweid89@gmail.com>
abel-von pushed a commit to abel-von/containerd that referenced this issue May 7, 2024
Close: containerd#10158

Signed-off-by: Wei Fu <fuweid89@gmail.com>
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 a pull request may close this issue.

1 participant