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

call fsync between part file write and rename #13652

Merged
merged 1 commit into from
Feb 23, 2024
Merged

Conversation

kasey
Copy link
Contributor

@kasey kasey commented Feb 22, 2024

What type of PR is this?

Feature

What does this PR do? Why is it needed?

This PR adds a feature flag --blob-save-fsync. When this flag is set, the blob storage code will call fsync after writing blob data to the .part file, before renaming the part file to the .ssz file.

This can be used by node operators who want extra reassurance that zero-length blob files won't be written, particularly in the event of a node restart.

@kasey kasey requested a review from a team as a code owner February 22, 2024 14:21
@kasey kasey requested review from rauljordan, potuz and nisdas February 22, 2024 14:21
@@ -155,6 +155,9 @@ func (bs *BlobStorage) Save(sidecar blocks.VerifiedROBlob) error {
if err != nil {
return err
}
if err := partialFile.Sync(); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

This file is closed already?

Copy link
Member

Choose a reason for hiding this comment

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

nvm -- that makes sense

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no you're right, fixed.

@kasey kasey marked this pull request as draft February 22, 2024 15:27
@kasey kasey force-pushed the fsync-blob-writes branch from 43bbe7b to 0f907da Compare February 23, 2024 17:02
@kasey kasey marked this pull request as ready for review February 23, 2024 17:05
prestonvanloon
prestonvanloon previously approved these changes Feb 23, 2024
@kasey kasey force-pushed the fsync-blob-writes branch from 6029ac0 to c0b5302 Compare February 23, 2024 17:54
@kasey kasey enabled auto-merge February 23, 2024 19:49
@kasey kasey added this pull request to the merge queue Feb 23, 2024
Merged via the queue into develop with commit 70e1b11 Feb 23, 2024
17 checks passed
@kasey kasey deleted the fsync-blob-writes branch February 23, 2024 23:21
prestonvanloon pushed a commit that referenced this pull request Mar 1, 2024
Co-authored-by: Kasey Kirkham <kasey@users.noreply.github.com>
(cherry picked from commit 70e1b11)
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.

3 participants