-
Notifications
You must be signed in to change notification settings - Fork 8
Add recycle_on_failure and retries options to runtests #148
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
base: main
Are you sure you want to change the base?
Changes from all commits
bef7798
90b1896
d977a73
5f3dfa1
b20a5cd
fa6f27c
50d29aa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -833,7 +833,9 @@ end | |
| stderr = Base.stderr, | ||
| max_worker_rss = get_max_worker_rss(), | ||
| serial = String[], | ||
| serial_position::Symbol = :before) | ||
| serial_position::Symbol = :before, | ||
| recycle_on_failure::Bool = false, | ||
| retries::Integer = 0) | ||
| runtests(mod::Module, ARGS; ...) | ||
|
|
||
| Run Julia tests in parallel across multiple worker processes. | ||
|
|
@@ -880,6 +882,10 @@ Several keyword arguments are also supported: | |
| instead of in parallel. An `ArgumentError` is thrown if any name is not found in the testsuite. | ||
| - `serial_position`: When to run serial tests relative to the parallel batch. | ||
| Must be `:before` (default) or `:after`. | ||
| - `recycle_on_failure`: Whether to recycle a worker after any test that did not pass | ||
| (default: `false`). See the Failure Handling section below. | ||
| - `retries`: How many times to re-run tests that did not pass after the main run completes | ||
| (default: `0`). See the Failure Handling section below. | ||
|
|
||
| ## Command Line Options | ||
|
|
||
|
|
@@ -959,6 +965,17 @@ runtests(MyPackage, ARGS; serial=["big_alloc_test", "huge_matrix"]) | |
|
|
||
| Workers are automatically recycled when they exceed memory limits to prevent out-of-memory | ||
| issues during long test runs. The memory limit is set based on system architecture. | ||
|
|
||
| ## 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. | ||
| """ | ||
| function runtests(mod::Module, args::ParsedArgs; | ||
| testsuite::Dict{String,Expr} = find_tests(pwd()), | ||
|
|
@@ -973,6 +990,8 @@ function runtests(mod::Module, args::ParsedArgs; | |
| stdout = Base.stdout, | ||
| stderr = Base.stderr, | ||
| max_worker_rss = get_max_worker_rss(), | ||
| recycle_on_failure::Bool = false, | ||
| retries::Integer = 0, | ||
| ) | ||
| # | ||
| # set-up | ||
|
|
@@ -1023,6 +1042,8 @@ function runtests(mod::Module, args::ParsedArgs; | |
| stdout, | ||
| stderr, | ||
| max_worker_rss, | ||
| recycle_on_failure, | ||
| retries, | ||
| ) | ||
| end | ||
|
|
||
|
|
@@ -1045,6 +1066,8 @@ function _runtests(mod::Module, args::ParsedArgs; | |
| stdout = Base.stdout, | ||
| stderr = Base.stderr, | ||
| max_worker_rss = get_max_worker_rss(), | ||
| recycle_on_failure::Bool = false, | ||
| retries::Integer = 0, | ||
| ) | ||
|
|
||
| # partition into serial and parallel groups | ||
|
|
@@ -1274,6 +1297,7 @@ function _runtests(mod::Module, args::ParsedArgs; | |
| # | ||
|
|
||
| tests_to_start = Threads.Atomic{Int}(length(tests)) | ||
| interrupted = false | ||
| # After parallel-before-serial: stop extra workers so only one process is alive for | ||
| # serial tests, but keep one parallel worker so we do not add a third addworker (ID_COUNTER). | ||
| function drain_pool_leaving_one_worker!(pool, njobs) | ||
|
|
@@ -1374,6 +1398,11 @@ function _runtests(mod::Module, args::ParsedArgs; | |
| # the worker has reached the max-rss limit, recycle it | ||
| # so future tests start with a smaller working set | ||
| Malt.stop(wrkr) | ||
| elseif recycle_on_failure && anynonpass(result[]) | ||
| # a failing test may have left the worker in a bad state | ||
| # (e.g. a wedged GPU driver whose every later allocation | ||
| # fails); recycle it so future tests get a fresh process | ||
| Malt.stop(wrkr) | ||
| end | ||
| else | ||
| # One of Malt.TerminatedWorkerException, Malt.RemoteException, or ErrorException | ||
|
|
@@ -1430,6 +1459,7 @@ function _runtests(mod::Module, args::ParsedArgs; | |
| end | ||
| end | ||
| catch err | ||
| interrupted = true | ||
| if !(err isa InterruptException) | ||
| println(io_ctx.stderr, "\nCaught an error, stopping...") | ||
| end | ||
|
|
@@ -1467,6 +1497,73 @@ function _runtests(mod::Module, args::ParsedArgs; | |
| end | ||
| end | ||
|
|
||
| # retry failed tests, if requested: sequentially, on a single fresh worker, with every | ||
| # other worker gone — tests that failed due to resource pressure (e.g. GPU memory | ||
| # oversubscription from concurrent workers) reliably pass on an otherwise-idle system. | ||
| # only the retried result is reported; persistent failures fail again and are reported | ||
| # exactly once. | ||
| if retries > 0 && !interrupted && args.quickfail === nothing | ||
| local retry_wrkr = nothing | ||
| for round in 1:retries | ||
| retryable = [r.test for r in results.value | ||
| if r.result isa Exception || anynonpass(r.result[])] | ||
| isempty(retryable) && break | ||
| println(io_ctx.stdout) | ||
| printstyled(io_ctx.stdout, | ||
| "Retrying $(length(retryable)) failed test(s) on a fresh worker...\n"; | ||
| color = :yellow) | ||
| for test in retryable | ||
| # pass in init_worker_code to custom worker function if defined | ||
| wrkr = if init_worker_code == :() | ||
| test_worker(test) | ||
| else | ||
| test_worker(test, init_worker_code) | ||
| end | ||
| if wrkr !== nothing && !Malt.isrunning(wrkr) | ||
| wrkr = nothing | ||
| end | ||
| custom = wrkr !== nothing | ||
| if !custom | ||
| if retry_wrkr === nothing || !Malt.isrunning(retry_wrkr) | ||
| retry_wrkr = addworker(; init_worker_code, io_ctx.color, exename, | ||
| exeflags, env) | ||
| end | ||
| wrkr = retry_wrkr | ||
| end | ||
| test_t0 = time() | ||
| result = try | ||
| Malt.remote_call_fetch(invokelatest, wrkr.w, runtest, | ||
| RecordType, testsuite[test], test, | ||
| init_code, test_t0, custom_args) | ||
| catch ex | ||
| isa(ex, InterruptException) && rethrow() | ||
| ex | ||
|
Comment on lines
+1539
to
+1540
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the worker be stopped here?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The worker does get stopped, just a few lines further down (the diff hunk cuts off before it): a non-interrupt exception is captured as |
||
| end | ||
| test_t1 = time() | ||
| output = @lock wrkr.io String(take!(wrkr.io[])) | ||
| filter!(r -> r.test != test, results.value) | ||
| push!(results.value, (; test, result, output, test_t0, test_t1)) | ||
| if result isa AbstractTestRecord && !anynonpass(result[]) | ||
| printstyled(io_ctx.stdout, " $test passed on retry\n"; color = :green) | ||
| else | ||
| printstyled(io_ctx.stdout, " $test failed again\n"; color = :red) | ||
| if !custom | ||
| # don't let a failure contaminate the next retry | ||
| Malt.stop(retry_wrkr) | ||
| retry_wrkr = nothing | ||
| end | ||
| end | ||
| # get rid of the custom worker | ||
| if custom && Malt.isrunning(wrkr) | ||
| Malt.stop(wrkr) | ||
| end | ||
| end | ||
| end | ||
| if retry_wrkr !== nothing && Malt.isrunning(retry_wrkr) | ||
| Malt.stop(retry_wrkr) | ||
| end | ||
| end | ||
|
|
||
| # print the output generated by each testset | ||
| # (`@sync` above joined all writers, so `results` is quiescent from here on) | ||
| for (testname, result, output, _start, _stop) in results.value | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These arguments aren't documented in the signature and list of arguments above