Skip to content

fix(services/fs): make writes crash-safe on supported platforms - #8315

Open
Huliiiiii wants to merge 1 commit into
apache:mainfrom
Huliiiiii:crash-safe-unix
Open

Huliiiiii wants to merge 1 commit into
apache:mainfrom
Huliiiiii:crash-safe-unix

Conversation

@Huliiiiii

Copy link
Copy Markdown

Which issue does this PR close?

Closes #.

Rationale for this change

Write is not crash-safe on unix.

What changes are included in this PR?

  • Write metas to the tempfile before renaming it, rather than writing it after fsync.
  • Fsync parent directories after renaming the file.

Are there any user-facing changes?

N/A

Breaking changes

N/A

AI Usage Statement

GPT: Investigation, research, and exploration
Guided code implementation.

@Huliiiiii
Huliiiiii requested a review from Xuanwo as a code owner September 16, 2026 06:17
@github-actions github-actions Bot added core releases-note/fix The PR fixes a bug or has a title that begins with "fix" size:L This PR changes 100-499 lines, ignoring generated files. labels Sep 16, 2026
@Huliiiiii

Huliiiiii commented Sep 16, 2026

Copy link
Copy Markdown
Author

On Apple's platforms, std file sync will use full fsync instead of regular fsync, with higher overheads.

Matching the behavior on other platforms would require adding a dependency such as rustix.

Some Unix platforms don't support fsync on directories, but they are uncommon, so I think it's fine to leave them unsupported for now.

I also ran into another issue. fsync errors should generally not be retried, but new_std_io_error marks other io errors as temporary. I have to mark them as permanent everywhere.

@Xuanwo

Xuanwo commented Sep 16, 2026

Copy link
Copy Markdown
Member

Could you run a benchmark to measure the differences?

@Huliiiiii

Copy link
Copy Markdown
Author

Performance impact

Size Prev Current Difference
4 KiB 1.614 ms 4.393 ms +2.779 ms (+172.2%)
64 KiB 1.742 ms 4.492 ms +2.750 ms (+157.9%)
1 MiB 2.329 ms 5.134 ms +2.805 ms (+120.4%)
16 MiB 12.585 ms 15.540 ms +2.955 ms (+23.5%)

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

Labels

core releases-note/fix The PR fixes a bug or has a title that begins with "fix" size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants