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

headerfs: sync file after writing raw header in appendRaw function #296

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mohamedawnallah
Copy link

Change Description

Closes #276

@guggero
Copy link
Member

guggero commented Mar 25, 2024

While this implementation is likely correct and probably the thing we should do, I worry about the performance hit this will take. Do we have a benchmark test to see how much of an impact this change has? Perhaps we need to also refactor things a bit to make sure we only call the appendRaw method for big chunks of data where it's worth syncing and not for every single entry for example.

@mohamedawnallah
Copy link
Author

Do we have a benchmark test to see how much of an impact this change has?

Yes, you're right! It may cause overhead of syncing unless a given benchmark proves otherwise 👍

Perhaps we need to also refactor things a bit to make sure we only call the appendRaw method for big chunks of data where it's worth syncing and not for every single entry for example.

I will look into this. Thanks for sharing!

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.

fsync instead of calling Write
2 participants