stop the two builds of a case deleting each other's files - #5
Merged
Merged
Conversation
Every file in this suite is built twice, at C++23 and at the C++20 floor, so under ctest -j two processes run the same cases at the same moment. The temp directory helper named its directory after the case and a counter that restarts in each process, and then cleared whatever was already at that name, so both processes asked for zu-cpp-append-6 and the second one deleted the first one's database halfway through a load. What came back was "no such file or directory" on a path the case had written a line earlier, which reads like an engine bug and is not one. It was intermittent and it moved around, and a flaky suite is a suite people stop reading. The name is now found by making it rather than by picking one and clearing it. create_directory is a single mkdir, so of two processes asking for the same name exactly one is told it made it and the other tries the next number. Nothing removes a directory it did not create.
22 tasks
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
1 participant
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.
The suite is flaky under
ctest -jand has been. This is why.Every file here is built twice, once at C++23 and once at the C++20 floor, so
test_bulkandtest_bulk_cxx20are the same cases in two processes and under-jthey run at the same moment.TempDirnamed its directory after the case and a counter that restarts in each process, then calledremove_allon that name before creating it. Both processes asked forzu-cpp-append-6, and whichever got there second deleted the first one's database in the middle of a bulk load.The failures that came back said
io: /tmp/zu-cpp-append-6/people.zu: No such file or directoryon a path the case had written a line earlier. That reads like an engine bug, and it is not one. It moved between files from run to run, which is the worst shape a failure can have: a suite that fails somewhere different each time is a suite people stop reading.The fix is to find a free name by making it rather than by picking one and clearing whatever is there.
create_directoryis a singlemkdir, so of two processes asking for the same name at the same instant exactly one is told it created it and the other moves to the next number. Nothing in here removes a directory it did not create, which is the property that was missing.Verified on Linux with gcc 13 against
libzufrom engine HEAD: three consecutivectest -j 16runs, 35 of 35 green each time. Before this, the same command failed between two and eleven cases per run.