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

pull: Implement --enable-syncfs #49

Closed

Conversation

giuseppe
Copy link
Member

Differently than fsync that is called after each file is modified,
syncfs is used only once at the end of the pull to sync the file
system which contains the repository.

Results of pull on an empty repository:

$ LANG=C sudo time ./ostree --repo=repo --disable-fsync pull master master
0 metadata, 0 content objects fetched; 83524 KiB; 6 delta parts fetched, transferred in 5 seconds
17.09user 2.41system 0:05.77elapsed 337%CPU (0avgtext+0avgdata 296132maxresident)k
0inputs+868584outputs (0major+102828minor)pagefaults 0swaps

$ LANG=C sudo time ./ostree --repo=repo pull master master
0 metadata, 0 content objects fetched; 83524 KiB; 6 delta parts fetched, transferred in 401 seconds
17.31user 5.73system 6:41.33elapsed 5%CPU (0avgtext+0avgdata 296864maxresident)k
0inputs+868584outputs (0major+132664minor)pagefaults 0swaps

$ LANG=C sudo time ./ostree --repo=repo --enable-syncfs pull master master
0 metadata, 0 content objects fetched; 83524 KiB; 6 delta parts fetched, transferred in 6 seconds
17.14user 2.57system 0:10.60elapsed 185%CPU (0avgtext+0avgdata 296212maxresident)k
0inputs+868584outputs (0major+86783minor)pagefaults 0swaps

Signed-off-by: Giuseppe Scrivano [email protected]

@cgwalters
Copy link
Member

See https://bugzilla.gnome.org/show_bug.cgi?id=728065

The problem with this is that if the system crashes in the middle, there's no guarantee that the objects written before the syncfs() are valid. I actually hit that, see https://git.gnome.org/browse/ostree/commit/?id=2396608754060e9e7a68843b65e4d3b8f4a4b5f2

@giuseppe giuseppe force-pushed the giuseppe/add-pull-enable-syncfs branch from f86de76 to b13e13b Compare January 22, 2015 11:30
@giuseppe
Copy link
Member Author

I've pushed another version with the logic described here: https://bugzilla.gnome.org/show_bug.cgi?id=728065

All tests pass

@giuseppe giuseppe force-pushed the giuseppe/add-pull-enable-syncfs branch 2 times, most recently from 4631eb7 to d062aaa Compare January 26, 2015 18:08
@cgwalters
Copy link
Member

Hmm, didn't I have more comments on an older version of this commit? I can't find it offhand. But for example I thought I wrote up a concern about power cuts; this patch isn't addressing that yet is it?
I had the idea to use the boot uuid or something like that.

I wish github made it easier to find old comments...

@giuseppe
Copy link
Member Author

yes, I didn't implement that. I believed it was a comment for a later improvement. I am going to send another version that uses the boot uuid.

Do not write directly to objects/ but maintain pulled files under tmp/
with a "tmpobject-$CHECKSUM.$OBJTYPE" name until they are syncfs'ed to
disk.

Move them under objects/ at ostree_repo_commit_transaction cleanup
time.

Before (test done on a local network):

$ LANG=C sudo time ./ostree --repo=repo pull origin master

0 metadata, 3 content objects fetched; 83820 KiB; 4 delta parts
fetched, transferred in 417 seconds
16.42user 6.73system 6:57.19elapsed 5%CPU (0avgtext+0avgdata
248428maxresident)k
24inputs+794472outputs (0major+233968minor)pagefaults 0swaps

After:

$ LANG=C sudo time ./ostree --repo=repo pull origin master

0 metadata, 3 content objects fetched; 83820 KiB; 4 delta parts
fetched, transferred in 9 seconds
14.70user 2.87system 0:09.99elapsed 175%CPU (0avgtext+0avgdata
256168maxresident)k
0inputs+794472outputs (0major+164333minor)pagefaults 0swaps

https://bugzilla.gnome.org/show_bug.cgi?id=728065

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe giuseppe force-pushed the giuseppe/add-pull-enable-syncfs branch 2 times, most recently from bc29e71 to c64793d Compare January 29, 2015 10:00
This prevents to use files after a kernel crash or power failure and
that can be not completely synced to disk.

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe giuseppe force-pushed the giuseppe/add-pull-enable-syncfs branch from c64793d to c88ea80 Compare January 29, 2015 19:00
@cgwalters
Copy link
Member

Squashed these fixups on top and pushed. 👍

@cgwalters cgwalters closed this Jan 30, 2015
cgwalters pushed a commit to cgwalters/ostree that referenced this pull request Mar 31, 2022
Add manual bindings for MutableTree reading
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