-
Notifications
You must be signed in to change notification settings - Fork 13
idea: an ActorCancelled(ContextCancelled) exc type for better OoB cancelled handling
#403
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
Draft
goodboy
wants to merge
8
commits into
main
Choose a base branch
from
actor_cancelled_exc_type
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or 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
As in a layer "above" a KBI/SIGINT but "below" a `ContextCancelled` and
generally signalling an interrupt which requests cancellation of the
actor's `trio.run()`.
Impl deats,
- mk the new exc type inherit from our ctxc (for now) but overriding the
`.canceller` impl to,
* pull from the `RemoteActorError._extra_msgdata: dict` when no
`._ipc_msg` is set (which is always to start, until we incorporate
a new `CancelActor` msg type).
* not allow a `None` value since we should key-error if not set per
prev bullet.
- Mk adjustments (related) to parent `RemoteActorError.pformat()` to
accommodate showing the `.canceller` field in repr output,
* change `.relay_uid` to not crash when `._ipc_msg` is unset.
* support `.msg.types.Aid` and use its `.reprol()` from `._mk_fields_str()`.
* always call `._mk_fields_str()`, not just when `tb_str` is provided,
and for now use any `._message` in-place of a `tb_str` when
undefined.
Defining how an actor-nursery should emit an eg based on non-graceful cancellation in a new `test_actor_nursery` module. Obviously fails atm until the implementation is completed.
Attempting a rework of the post-cancellation "raising semantics" such
that subactors which are `ActorCancelled` as a result of a non-graceful
in-scope error, are acked via a re-raised
`ExceptionGroup[ActorCancelled*N, Exception]`
*outside the an-block*. Eventually, the idea is to have `ActorCancelled`
be relayed from each subactor in response to any
`Actor.cancel()/Portal.cancel_actor()` request much like
`Context.cancel()/ContextCancelled`.
This is a WIP bc it does break a few tests and requires related
`_spawn`-mod-machinery changes to match some of which I'm not yet sure
are required; need to dig into to the details of the currently failing
suites first.
`._supervise` patch deats,
- add `ActorNursery.maybe_error` which delivers the maybe-EG or
`._scope_error` depending on `.errors` (now `._errors`, a mapping from
`Aid`-keys) has entries seet for subs.
- raise ^ if non-null in a new outer-`finally` in
`_open_and_supervise_one_cancels_all_nursery()`; an "outer" block is
added to ensure all sub-actor-excs are emited/captured as part of
`ActorNursery.cancel()` being called (as prior) as well as the
`da_nursery` being explicitly cancelled alongside it (to unblock the
tn-block, but still not sure why this is necessary yet?..).
- (now masked) tried injecting actorcs from `.cancel()` loop, but (again
per more explanation in section below) seems to be suffering a race
issue with RAE relay?
- left in buncha notes obvi for all this..
`._spawn` patch deats,
- as above, expect `errors: dict` to map from `Aid`-keys.
- pass `errors: dict` into `soft_kill()` since it seemed like we'd want
to (for now) inject `ActoreCancelled` in some cases (but now i'm not
sure XD).
- tried out a couple spots (which are now masked) to inject
`ActorCancelled` after calling `Portal.cancel()` in various
subactor-supervision routines whenev an RAE is not set..
- oddly seems to be overwriting actual errors (likely due to racing
with RAE receive and/or actorc-request timeout?) despite the guard
logic..which clearly doesn't resolve the issue..
- buncha `tn`-style renaming.
Such that we don't require every single src/relay_uid in the final output but instead at some point in the pre-output of some prompt. Added some comments to match each actor sub-layer.
Since with the new actorc injection seems to be hanging? Not sure what exactly the issue is but likely races again during teardown between the `.run_in_actor()` remote-exc capture and any actorc after the `portal.cancel()`.. Also tossed in a bp to figure out why actorcs aren't actually showing outside the `trio.run()`..?
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.
Per recent testing effort, issue-docs drafting and generally ranting in the
triomatrix room 🫣 here is the recent content driving this idea,Cancelledmight not be enough? #400 attempts to justify the need for more then justtrio.Cancelledtrio.Cancelled-unmasking helper #391 demos whytrio.Cancelledand so called "exception masking" can be problematic in a distributed system where error/cancel relay is important for distributed-sys feats.New supporting feats in
triowe should try hereWith release
0.31.0we get a newCancelled.reason: strwhich can be set at the.cancel(reason)call,Cancelledpython-trio/trio#3232CancelScope.cancel()CancelReasonstruct._child_finished()