Add recycle_on_failure and retries options to runtests - #148
Conversation
|
If you have a job using too many resources, maybe you want to mark that one as serial? That's what #125 enables (and I'm not looking forward to rebase it for the umpteenth time) |
|
"Too many resources" is very system-dependent. I don't want to run large tests sequentially on a system where they can run in parallel. Or am I missing something? |
You could detect vram and use that to set the number of jobs to a lower value than would be otherwise selected. That said, I do like the idea of getting rid of a worker on failure and I would even go as far as to say that it should become the default in the next breaking version. I do agree with Mose that #125 should be reviewed and merged first. |
|
This is based on the experience I have when developing oneAPI.jl locally on my box. Manually setting a '--jobs' value does not guarantee that the tests pass, because the order of the tests is non-deterministic and timing-dependent. Therefore, I have to be overly defensive and set for example @christiangnrd I've added a VRAM budget for each worker into the oneAPI.jl tests. But that's not enough for various reasons. |
|
You seem to have a version of the problem that led me to open #77 which is to be implemented by #125. In which case I would designate the bigger tests to be run serially. I think you could also potentially only add them to the serial tests list if you detect lower resources or something. This probably wouldn't completely fix your issue but it could help mitigate it significantly. |
|
There are merge conflicts to resolve |
A test that corrupts process-wide state — the motivating case is a GPU whose driver ends up in a state where every subsequent allocation in the process fails — poisons every later test scheduled onto the same worker, turning one bad test into a cascade of failed files. The Distributed- based harness this package was extracted from recycled a worker after any failed test; restore that behavior behind `recycle_on_failure = true`, alongside the existing max-rss and crash recycling. With `retries = N`, tests that did not pass are re-run up to N times after the main run completes: sequentially, on a single fresh worker, with all other workers stopped. Parallel test runs create resource contention (several workers sharing one GPU or a limited amount of RAM), so a failure can mean "lost the resource race" rather than "broken": re-running on an otherwise-idle system distinguishes the two. Tests that failed due to contention reliably pass on the idle retry, while deterministic failures fail again and are reported exactly once — only the final attempt of each test enters the results, and retried tests are visibly marked in the output. Both options default to off.
e447a8b to
5c41172
Compare
Add a "Failure Handling" section to the advanced usage guide covering both options: why worker recycling after a failure is useful (process-wide state corruption cascading onto later tests on the same worker) and what the retry environment guarantees (all other workers stopped, sequential re-run on a fresh worker, only the final attempt reported, retry worker recycled after a repeat failure). Also mention them in the feature list on the front page, and add a best practice warning against using retries to paper over genuinely broken tests.
For `recycle_on_failure`, run a fixed sequence of failing and passing tests with a single job and count the workers created: the default reuses one worker for all of them, while `recycle_on_failure=true` needs a fresh worker after each failure. For `retries`, use a test that fails on its first attempt and passes on any subsequent one (recording attempts in a file, since each attempt runs in a different process) to check that a test rescued by a retry is reported as passing, and that it is the only worker alive while it runs. A persistently failing test is checked to exhaust its retries and still be reported exactly once. Also cover that retries are off by default and skipped under `--quickfail`.
|
Rebased, added docs, and some tests inspired by #125. |
| retry_wrkr = addworker(; init_worker_code, io_ctx.color, exename, | ||
| exeflags, env) |
There was a problem hiding this comment.
If the tests has a custom worker, this would be wrong
| isa(ex, InterruptException) && rethrow() | ||
| ex |
There was a problem hiding this comment.
Should the worker be stopped here?
| end | ||
| test_t0 = time() | ||
| result = try | ||
| Malt.remote_eval_wait(Main, retry_wrkr.w, :(import ParallelTestRunner)) |
There was a problem hiding this comment.
Why is import needed?
| ## Failure Handling | ||
|
|
||
| With `recycle_on_failure = true`, a worker is recycled after any test that did not pass, so | ||
| a test that corrupts process-wide state (e.g. wedges a GPU driver) cannot poison subsequent | ||
| tests on the same worker. | ||
|
|
||
| With `retries = N` (default 0), tests that did not pass are re-run up to `N` times after | ||
| the main run completes — sequentially, on a single fresh worker, with all other workers | ||
| stopped — so tests that failed due to resource pressure from concurrent workers get an | ||
| otherwise-idle system. Only the final attempt of each test is reported. |
There was a problem hiding this comment.
These arguments aren't documented in the signature and list of arguments above
3 participants
Two opt-in failure-handling options for
runtests, motivated by running oneAPI.jl's test suite on a memory-constrained GPU (8GB Arc A750,--jobs=2), where a baseline run failed 520 tests — 366 of themOutOfGPUMemoryError— cascading across ~20 test files that all pass individually.recycle_on_failure = trueA test that corrupts process-wide state poisons every later test scheduled onto the same worker. The motivating case is a GPU whose driver ends up in a state where every subsequent allocation in the process fails: in the run above, a single such event turned into ten consecutive failed files on one worker. The Distributed-based harness this package was extracted from recycled a worker after any failed test (
# the worker encountered some failure, recycle it so future tests get a fresh environment); this restores that behavior alongside the existing max-rss and crash recycling.retries = NTests that did not pass are re-run up to
Ntimes after the main run completes — sequentially, on a single fresh worker, with all other workers stopped. Parallel test runs create resource contention (several workers sharing one GPU, or limited RAM), so a failure can mean "lost the resource race" rather than "broken": re-running on an otherwise-idle system distinguishes the two. Analogous to pytest-rerunfailures / Bazel'sflaky_test_attempts, but the retry environment is deliberately quiesced. Only the final attempt of each test enters the results, and retried tests are visibly marked in the output, so flakiness is surfaced rather than hidden.With both options enabled (plus a per-worker GPU memory budget on the oneAPI.jl side), the run above goes from 10167 pass / 520 errors to 11751 pass / 4 errors with zero OOM errors; one file that failed under concurrency was rescued by its idle retry, and deterministic failures failed again and were reported exactly once.
Both options default to off, preserving current behavior. The package's own test suite passes.