Prevent unsafe concurrent coordinate writes #17
Merged
Conversation
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
spencerkclark
commented
May 8, 2023
spencerkclark
force-pushed
the
coordinate-bug
branch
from
96be43e to
d2955e9
Compare
spencerkclark
commented
May 8, 2023
spencerkclark
added a commit
that referenced
this pull request
May 10, 2023
This bumps the version of xpartition for a new release to include the fix in #17.
spencerkclark
added a commit
to ai2cm/fv3net
that referenced
this pull request
May 10, 2023
This PR bumps the version of xpartition to 0.2.1, which was just released, and includes a safety-oriented bug fix. This fix should eliminate the risk of corrupted coordinate data, which we only observed once on a recently created dataset, but is important to address. xref: spencerkclark/xpartition#17 Coverage reports (updated automatically):
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When concurrently writing partitions of DataArrays in a Dataset, any coordinates carried along by those DataArrays are also written concurrently. These attached coordinates do not necessarily adhere to the same chunk structure as the DataArray itself. This is an issue, since frequently they are completely unchunked, meaning that concurrent jobs attempt to write coordinates to the same blob files on disk, opening the possibility for data corruption. In practice data corruption of coordinates has been rare, but we recently encountered a situation where it occurred.
This PR fixes this issue by dropping all coordinates when doing low-level partitioned writes. As expected, now any unchunked coordinates or data variables are written once (and only once) during the store initialization step. This PR also addresses the subtle issue of writing chunked coordinates by treating them as though they were independent data variables (previously we did not have test coverage for this case, though it is admittedly rare to encounter in practice).