test_io_shared: Replace sync/skip workarounds with barriers - #982
Merged
Merged
Conversation
…lly needed The flakiness (#555 on Windows, #879 on macOS, and the Ubuntu failures seen in #926) was a data race in the test itself, not a library bug. MPI_File_get_position_shared is not collective, but every other shared file pointer call in this test moves the pointer: write_ordered and read_ordered increment it, seek_shared and set_view reset it. ROMIO and OMPIO both perform the reset on rank 0 (ROMIO's set_view: on every rank that has the hidden .shfp file open) *before* their internal barrier, and write_ordered increments the counter on rank 0 as soon as rank 0 enters the call. So a fast rank entering the next operation clobbers the pointer while a slow rank is still inside `@test get_position_shared(fh) == ...`. That is exactly the `0 == 10` / `0 == 3` reported in the issues. The fix is a plain MPI.Barrier after each block of position checks and before the next shared-pointer operation. MPI_File_sync only flushes file data (ROMIO keeps the pointer in a side file, OMPIO in shared memory) and plays no role; the old sync/Barrier/sync wrapper only helped through its embedded Barrier. With atomicity enabled the header data needs no sync either, only ordering. The vendor detection and the Apple/MPICH/Windows `skip=` clauses were masking the same race and are removed. Reproduced locally on macOS with MPICH 4.3.1, MPICH_jll 5.x, OpenMPI_jll 5.0.10 and MPItrampoline_jll: 30-60% failure rate without the barriers, 0 failures in 80 runs with them. Fixes #555, fixes #879.
vchuravy
force-pushed
the
eschnett/test-io-shared-flakiness-c88603
branch
from
September 9, 2026 10:35
d6cbfbc to
07fbedb
Compare
Member
|
Still seems to have some issues on Windows? |
Contributor
Author
|
I hope #984 will help on Windows. |
`test_io.jl` wrote collectively, called `MPI_File_sync`, and then had rank 0
read the whole file at once, with nothing ordering that read after the other
ranks' writes. In the default non-atomic mode the standard requires writer
sync, a barrier, and reader sync before another process's data is visible.
Without the barrier, a rank 0 that runs ahead reads zeros:
Test Failed at test/test_io.jl:24
Evaluated: Int64[1 0 0 0; 1 0 0 0] == [1 2 3 4; 1 2 3 4]
as seen on the Windows nightly x86 job of #982. Because the failing rank's
`atexit` hook then called `MPI_Finalize` while the other ranks waited in the
next barrier, the job deadlocked until the 20-minute timeout instead of
failing fast. The same missing barrier preceded the collective read after
rank sz-1's overwrite; both places now use sync, barrier, sync.
`test_io_subarray.jl` needs no change: every rank reads back only the bytes
it wrote itself through the same file handle, which the standard guarantees
to be consistent without synchronization. A comment records this so nobody
adds barriers there by analogy.
Verified with 4 ranks: MPICH 4.3.1, MPICH_jll 5.0.1, OpenMPI_jll 5.0.10 and
MPItrampoline_jll, 10 repetitions each, no failures.
vchuravy
reviewed
Sep 9, 2026
This was referenced Sep 10, 2026
vchuravy
approved these changes
Sep 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
test_io_shared: Replace sync/skip workarounds with the barriers actually needed
The flakiness (#555 on Windows, #879 on macOS, and the Ubuntu failures seen in #926) was a data race in the test itself, not a library bug.
MPI_File_get_position_shared is not collective, but every other shared file pointer call in this test moves the pointer: write_ordered and read_ordered increment it, seek_shared and set_view reset it. ROMIO and OMPIO both perform the reset on rank 0 (ROMIO's set_view: on every rank that has the hidden .shfp file open) before their internal barrier, and write_ordered increments the counter on rank 0 as soon as rank 0 enters the call. So a fast rank entering the next operation clobbers the pointer while a slow rank is still inside
@test get_position_shared(fh) == .... That is exactly the0 == 10/0 == 3reported in the issues.The fix is a plain MPI.Barrier after each block of position checks and before the next shared-pointer operation. MPI_File_sync only flushes file data (ROMIO keeps the pointer in a side file, OMPIO in shared memory) and plays no role; the old sync/Barrier/sync wrapper only helped through its embedded Barrier. With atomicity enabled the header data needs no sync either, only ordering.
The vendor detection and the Apple/MPICH/Windows
skip=clauses were masking the same race and are removed. Reproduced locally on macOS with MPICH 4.3.1, MPICH_jll 5.x, OpenMPI_jll 5.0.10 and MPItrampoline_jll: 30-60% failure rate without the barriers, 0 failures in 80 runs with them.Fixes #555, fixes #879.
(Thumbs pressed that this works!)