Immediate remote cancels #245
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
goodboy
added
supervision
cancellation
|
Lul and so the bloodbath begins. |
This was referenced Oct 14, 2021
As for `Actor.cancel()` requests, do the same for `Actor._cancel_task()` but use `_invoke()` to ensure correct msg transactions with caller. Don't cancel task cancels on a cancel-all-tasks operation in attempt at more determinism.
This is actually surprisingly easy to grok having gone through a lot of pain understanding edge cases in the zombie lord dev branch. Basically we just need to make sure actors are managed in a 2 step reap sequence. In the "soft" reap phase we wait for the process to terminate on its own concurrently with (maybe) waiting for its portal's final result (if it's a `.run_in_actor()`). If this path is cancelled or errors, then we do a "hard" reap where we timeout and send a signal to the proc to terminate immediately. The only last remaining trick is to tie in the root-is-debugger-aware logic to yet again avoid tty clobbers.
With the new fixes to the trio spawner we can expect that both root *and* depth > 1 nursery owning actors will now not clobber any children that are in debug (either via breakpoint or through crashing). The tests changed now include more checks which ensure the 2nd level parent-ish actors also bubble up through into `pdb` and don't kill any of their (crashed) children before they're done themselves debugging.
goodboy
force-pushed
the
immediate_remote_cancels
branch
from
9b3e0a5
to
51259c4
Compare
goodboy
force-pushed
the
immediate_remote_cancels
branch
from
12675f4
to
4f222a5
Compare
goodboy
force-pushed
the
immediate_remote_cancels
branch
2 times, most recently
from
34ed193
to
3f4384b
Compare
goodboy
force-pushed
the
immediate_remote_cancels
branch
from
3f4384b
to
b3c4851
Compare
goodboy
commented
Oct 17, 2021
| log.cancel( | ||
| f"Actor {self.uid} was remotely cancelled; " | ||
| "waiting on cancellation completion..") | ||
| await _invoke(self, cid, chan, func, kwargs, is_rpc=False) |
There was a problem hiding this comment.
Just for reference, this is the crux of the change: instead of scheduling Actor.cancel() and ._cancel_task() as is done for rpc endpoints, we invoke these methods (which really are handlers for special messages that we should add to our SC protocol) immediately via await.
goodboy
commented
Oct 17, 2021
| ) | ||
| await self.cancel_rpc_tasks(chan) | ||
|
|
||
| # end of async for, channel disconnect vis ``trio.EndOfChannel`` |
goodboy
commented
Oct 17, 2021
| ): | ||
| ''' | ||
| Connect to the root actor via a ctx and invoke a task which locks | ||
| a root-local TTY lock. |
overclockworked64
added a commit
to overclockworked64/tractor
that referenced
this pull request
Oct 22, 2021
This was referenced Oct 23, 2021
goodboy
pushed a commit
that referenced
this pull request
Oct 23, 2021
Merged
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.
This adds greedy remote cancellation to the msg loop making both task and actor-runtime cancel requests more immediate in the sense they are no longer scheduled for run with
.start()in the service nursery and instead are_invoke()d directly and served asap.These are pieces from an ongoing rework of the actor nursery going on over in https://github.com/goodboy/tractor/tree/zombie_lord_infinite, which is basically moving away entirely from the original multiplexed error handling style to per-task-managing-spawned-process style. The idea here is to make the actual nursery implementation much simpler whilst keeping the harder stuff (like was this actor-task cancelled, and do we care?) is done in the spawning back-end on a per task level thereby minimizing global state in supervisor strategy implementations.
Mostly putting this up independent of the spawning/nursery rework stuff to see how CI responds to the timing changes in the core msg loop.