Introduce InvocationStatus::Killed #2335
Conversation
slinkydeveloper
force-pushed
the
issues/2331-second-attempt
branch
from
25a695d to
d57419d
Compare
There was a problem hiding this comment.
Thanks for adding InvocationStatus::Killed to enable the kill with retry feature @slinkydeveloper. Great that you also fixed the issues with killing/cancelling scheduled invocations! The changes look good to me. I left a few comments/suggestions. +1 for merging after resolving them.
crates/partition-store/src/tests/invocation_status_table_test/mod.rs
Outdated
Show resolved
Hide resolved
| /// If true, original status is Invoked, otherwise is Killed | ||
| pub is_invoked: bool, |
There was a problem hiding this comment.
If this were an enum with the explicit variants, then no comment would be needed to explain what true and false means.
There was a problem hiding this comment.
I think this is fine for the time being, this package is crowded with too many types. I tried to add some enum discriminator already, and it didn't end well naming wise.
Eventually I wanna simply rename InvocationStatus to Invocation, and have an InvocationStatus enum which is just the discriminants, and then here use that enum. I can pull this through in a subsequent PR, but it's a big change and i don't wanna do it now to avoid rebase hell.
slinkydeveloper
force-pushed
the
issues/2331-second-attempt
branch
from
98a00ee to
ee51359
Compare
|
First commit is just the squash of the previous stuff |
There was a problem hiding this comment.
Thanks for addressing my comments @slinkydeveloper. LGTM. +1 for merging.
slinkydeveloper
force-pushed
the
issues/2331-second-attempt
branch
from
25be1e3 to
a2ea0ca
Compare
Add Killed invocation status to CLI Fix cancellation/killing of scheduled invocations. Reply KILLED when receiving attach/get output and deduplication. Handle killed invocations during leadership change Plumb the new experimental feature invocation_status_killed. Also replace the individual booleans for experimental features with enum set. Add acknowledgment flag to invoker abort command. This makes sure that after a kill, the invoker will send a "terminal" effect to the state machine. Add experimental feature InvocationStatus::Killed Add InvocationStatus::Killed
slinkydeveloper
force-pushed
the
issues/2331-second-attempt
branch
from
4245662 to
bf605ea
Compare
Fix #2331. While at it I did some lil' refactorings + fixed killing/cancelling scheduled invocations