-
Notifications
You must be signed in to change notification settings - Fork 22
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
NC_SHARE: call sync after fill operations #107
base: master
Are you sure you want to change the base?
Conversation
I can confirm that this PR allows the Without that, the test produces errors like the following:
|
I assume adding one ncmpi_sync call after all ncmpi_fill_var_rec calls will pass |
Thanks @wkliao . Yes, that's right. A single There are a number of test cases that I've come across so far, e.g., e47759a |
Thanks. That is a very useful information. PnetCDF is built on top of MPI-IO, so follows the MPI-IO consistency semantics. Your patch will affect all applications that are using POSIX compliant file |
How common is another option would be to consult the ROMIO is not the only MPI-IO implementation, so it does feel wrong to have PnetCDF rely on a non-standard hint like this. @adammoody where did we leave https://github.com/roblatham00/mpich/tree/dev-unify ? To wei-keng's point, trying to use a POSIX-assuming ROMIO driver on a non-posix storage system is going to get you into trouble, but the goal with the Unify ROMIO driver was Unify can (cheaply) sync and flush if needed, when needed. |
I figured the change in this PR aligns well with the PnetCDF data consistency doc:
It seems like most calls that lead to an |
By the way, I think it would help to add an empty line on that page to fix the github markdown display:
As it is, the "Users are warned" statement gets attached to the second bullet rather than standing on its own. |
Thanks @roblatham00 . Right, the MPICH dev-unify branch would also resolve this, though I haven't tried it yet. I have been testing against our default system MPI libraries to get a baseline. For that, I'm taking extra steps like disabling ROMIO data sieving and other aspects that are known to be problematic for UnifyFS. |
NC_SHARE is a flag inherited from NetCDF. NetCDF before version 4 supported It is in my plan to retire this flag in the future and leave the users to enforce Requiring PnetCDF and UnifyFS users to always set NC_SHARE (and hence |
Hi, @adammoody |
Sure. I'm still working through the different test cases. I'll let you know which tests I had to modify when I'm done. |
Here is the latest list of changes to the test cases: master...adammoody:PnetCDF:unifyfs In some tests, I may have added more @wangvsa, is using Recorder and VerifyIO to develop a more comprehensive list of test cases. |
PR #120 has been created to add the following code fragment into the test programs
Please give it a try and let me know if you found more test programs need the similar fix. FYI. The git command to fetch PR #120 is:
|
051cdc1
to
f1db1d6
Compare
While running PnetCDF tests on a file system that requires stricter adherence to the MPI-I/O consistency semantics (UnifyFS), I found that several test cases involving fill operations were not calling
MPI_File_sync
after the writes associated with the fill calls. This led to data inconsistency when other ranks later wrote different data to the same region of the file. This was true even after modifying the test to create the file withNC_SHARE
set.One example shows up when running
test/testcases/flexible.c
with 4 ranks on 1 node due to the fill calls here:PnetCDF/test/testcases/flexible.c
Lines 91 to 97 in e475964
Adding
NC_SHARE
to thencmpi_create
call does not help:PnetCDF/test/testcases/flexible.c
Line 79 in e475964
To get this test to pass, I had to add a call to
ncmpi_sync
after the fill calls.The intent with this PR is to call
MPI_File_sync/MPI_Barrier
after writing data during a fill call if theNC_SHARE
mode bit was set when creating the file.I wanted to open the PR for discussion.
Are you open to a change like this?
If so, are there other locations where
MPI_File_sync
should be called during fills?