Skip to content

Replace fsync with global sync call #31

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

Closed
wants to merge 1 commit into from
Closed

Conversation

rht
Copy link
Contributor

@rht rht commented Oct 3, 2015

No description provided.

@rht
Copy link
Contributor Author

rht commented Oct 3, 2015

On ext4

fsync:
outdata

sync:
outdata_sync

@rht
Copy link
Contributor Author

rht commented Oct 3, 2015

@rht
Copy link
Contributor Author

rht commented Oct 3, 2015

nosync:
outdata

@jbenet
Copy link
Member

jbenet commented Oct 4, 2015

@rht thanks for these. However, i'm not sure how these scale with lots of other disk activity and multiple concurrent sync calls. I suspect hammering at ipfs with thousands of requests would show that fsync is better, but i haven't run the test. cc @whyrusleeping @tv42

i still think no-sync should be an option, (sync by default). ideally we could do it on a per-command basis on ipfs. so one could ipfs add --no-sync or something.

@rht rht mentioned this pull request Nov 6, 2015
@rht
Copy link
Contributor Author

rht commented Nov 13, 2015

In some filesystems, per-file fsync is already short-circuited to global sync. This PR simply makes it explicit that it is the default for any host fs.

Why would fsync be faster for the case of "lots of other disk activity and multiple concurrent sync calls"?

@rht
Copy link
Contributor Author

rht commented Nov 13, 2015

Benchmark already shows otherwise (sync vs fsync).

But now that there are two options (default sync for safe operation and nosync for fast one), this middle ground is no longer necessary.

@rht rht closed this Nov 13, 2015
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