Repository navigation
Update the test history under a lock, in batches of finished tests - #188
Conversation
ac48ad8 to
ae5d541
Compare
christiangnrd
left a comment
christiangnrd
left a comment
There was a problem hiding this comment.
Conceptually LGTM, I glanced at the implementation but I can take a closer look if you would like me to. Can probably be merged as a stack with #189?
The history of durations and failures was read once at the start of `runtests` and the whole in-memory copy was serialized back at the end, so of several runs sharing a machine the last to finish overwrote what the others had recorded, and a reader could see a half-written file. Finished tests are now merged into the file under a `FileWatching.Pidfile` lock, re-reading the stored history first, and the file is replaced through a temporary file and rename. Entries of tests a run did not execute are left untouched, and a lock left behind by a dead process is removed right away. Outcomes are written in batches of `history_flush_every` tests (and once more at the end), so the lock and rename traffic stays low on slow filesystems while an interrupted run still keeps all but its last few measurements. Fixes #187 (first proposal). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TNK7qXSraxSfzK6T4UxUtk
How many finished tests to accumulate before merging them into the on-disk history trades lock and rename traffic against how much an interrupted run can lose, and the right balance depends on where the scratch space lives. Expose it as a documented keyword with the same default of 20 instead of a module-level variable. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TNK7qXSraxSfzK6T4UxUtk
81b06f9 to
b682d53
Compare
|
BTW, if you still have a chance to give this a closer look, I'd appreciate it! It should be decent though, and it has all the locks (on the filesystem and in process) where I was expecting them. Still, happy to know whether I missed something! |
|
Claude found this while I was asking about something else:
I'll add a fix for it with my PR moving the code over to a file |
Motivation for this is that I'm moving Oceananigans to ParallelTestRunner and we use self-hosted runners which share the depot. The idea is to make the accesses to the history file safe.
History files are updated not just at the end of the tests but every 20 finished tests to save intermediate progress for large test suites, and also not to entirely lose the data in case the test driver process is abruptly terminated without a chance of finishing things up cleanly.
Ref: #187