More thorough basic supervision tests #91
Merged
Conversation
This file contains 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
In an effort towards #43. This completes the first major bullet's worth of tests described in that issue.
`trio.MultiError` isn't an `Exception` (derived instead from `BaseException`) so we have to specially catch it in the task invocation machinery and ship it upwards (like regular errors) since nurseries running in sub-actors can raise them.
Add a test to verify that `trio.MultiError`s are properly propagated up a simple actor nursery tree. We don't have any exception marshalling between processes (yet) so we can't validate much more then a simple 2-depth tree. This satisfies the final bullet in #43. Note I've limited the number of subactors per layer to around 5 since any more then this seems to break the `multiprocessing` forkserver; zombie subprocesses seem to be blocking teardown somehow... Also add a single depth fast fail test just to verify that it's the nested spawning that triggers this forkserver bug.
If a nursery fails to cancel (some sub-actors presumably) then hard kill the whole process tree to avoid hangs during a catastrophic failure. This logic may get factored out (and changed) as we introduce custom supervisor strategies.
|
And there you have it. Windows doesn't like "nested sub-processes" even with the spawn method... |
goodboy
force-pushed
the
more_thorough_super_tests
branch
2 times, most recently
from
3ffab4b to
421459d
Compare
Seems like we've probably got some greater limitations with Windows and "nested" spawned sub-processes...
goodboy
force-pushed
the
more_thorough_super_tests
branch
from
421459d to
2d4b6de
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Attention all lurkers: code reviews and specifically criticisms are appreciated!
This is mostly to resolve #43 but I created some related issues (#87, #88, #89) after thinking and working through other types of failure cases we need to handle.
The main solution to get this working was adding proper
MultiErrorpropagation support in the core.Interestingly writing the nested sub-actor error propagation tests actually found some serious holdups in the
multiprocessingforkserver code. Even with my patches to the stdlib (which are pretty questionable) the server breaks pretty easily (causing hangs) when spawning any more then a couple "levels" (really lifetime scopes) of process tree. It's all good fodder for #84 and either writing our own forkserver or looking at the proper way to get the stdlib's version to play with our model. Surprisingly the spawn method works just fine so trying outtrio_run_in_processhas some more incentive; interested to see how it performs vs. the stdlib's version.