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

deploy: Remove global sync by default #2968

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

cgwalters
Copy link
Member

Our previous change here was not actually sufficient for the ceph case, because what (I think) is happening is that our other syncfs() invocation is getting blocked on some kernel mutexes that are used in sync, and that's causing the process to fully block.

We should not be dependent on a full filesystem sync, only on the sync of the sysroot and boot filesystems.

Anyone who does want this behavior could inject an override for ostree-finalize-staged.service that overrides ExecStop to add a run of sync.

@cgwalters
Copy link
Member Author

OK I think we should do this PR at some point, but some more investigation here narrowed in on #2969

Our previous change here was not actually sufficient for
the ceph case, because what (I think) is happening is that
our other `syncfs()` invocation is getting blocked on some
kernel mutexes that are used in `sync`, and that's causing the
process to fully block.

We should not be dependent on a full filesystem `sync`, only
on the sync of the sysroot and boot filesystems.

Anyone who *does* want this behavior could inject an override
for `ostree-finalize-staged.service` that overrides `ExecStop`
to add a run of `sync`.
@cgwalters cgwalters force-pushed the drop-global-syncfs-by-default branch from aea3c2e to fa69eaa Compare August 14, 2023 18:17
@cgwalters cgwalters marked this pull request as ready for review August 30, 2023 17:27
@cgwalters
Copy link
Member Author

Lifting draft on this one, I think we should merge it.

Copy link
Member

@jmarrero jmarrero left a comment

Choose a reason for hiding this comment

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

lgtm

@cgwalters cgwalters merged commit c0014e0 into ostreedev:main Aug 30, 2023
23 checks passed
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

3 participants