Skip to content

Change all fsync to fdatasync#31229

Merged
alexey-milovidov merged 12 commits intoClickHouse:masterfrom
zhanglistar:fsync
Nov 14, 2021
Merged

Change all fsync to fdatasync#31229
alexey-milovidov merged 12 commits intoClickHouse:masterfrom
zhanglistar:fsync

Conversation

@zhanglistar
Copy link
Copy Markdown
Contributor

@zhanglistar zhanglistar commented Nov 10, 2021

Changelog category (leave one):

  • Performance Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Improve performance of syncing data to block device. This closes #31181.

@robot-clickhouse robot-clickhouse added doc-alert pr-feature Pull request with new product feature labels Nov 10, 2021
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Nov 10, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ alexey-milovidov
✅ zhanglistar
❌ listar


listar seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@robot-clickhouse robot-clickhouse added pr-performance Pull request with some performance improvements and removed pr-feature Pull request with new product feature labels Nov 11, 2021
@alexey-milovidov alexey-milovidov self-assigned this Nov 11, 2021
@alexey-milovidov
Copy link
Copy Markdown
Member

macOS does not have fdatasync.

@azat
Copy link
Copy Markdown
Member

azat commented Nov 12, 2021

Improve performance of syncing data to block device

@zhanglistar do you have any performance numbers for this change? I think that the difference for MergeTree code for example (where we write the whole file and only after this calling sync) should be negligible.

@zhanglistar
Copy link
Copy Markdown
Contributor Author

zhanglistar commented Nov 12, 2021

Improve performance of syncing data to block device

@zhanglistar do you have any performance numbers for this change? I think that the difference for MergeTree code for example (where we write the whole file and only after this calling sync) should be negligible.

@azat In the clickhouse-keeper code, I try to use fdatasync instead of fsync, the writing performance increase about 30%.
Here is the detail:
keeper + fsync:
image
keeper + fdatasync:
image

And there is a conversation about this in the end of pr:
#23038

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-performance Pull request with some performance improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace fsync to fdatasync in all places.

6 participants