-
Notifications
You must be signed in to change notification settings - Fork 1k
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
add WatchAysnc
methods for monitoring actor lifecycles outside of Akka.NET
#6102
Conversation
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.
Reviewed my own changes
@@ -1882,6 +1882,10 @@ namespace Akka.Actor | |||
public Akka.Actor.IStash Stash { get; set; } | |||
} | |||
public delegate void UntypedReceive(object message); | |||
public class static WatchAsyncSupport | |||
{ | |||
public static System.Threading.Tasks.Task<bool> WatchAsync(this Akka.Actor.IActorRef target, System.Threading.CancellationToken token = null) { } |
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.
Just a new extension method for working with IActorRef
@@ -51,7 +51,7 @@ public async Task GracefulStopShouldReturnTrueForAlreadyDeadActor() | |||
|
|||
private class CustomShutdown{} | |||
|
|||
[Fact(DisplayName = "GracefulStop should return false if shutdown goes overtime", Skip = "GracefulStop currently throws a TaskCancellationException, which seems wrong")] |
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.
Enabled this spec as I went back and addressed this behavior
public class WatchAsyncSpecs : AkkaSpec | ||
{ | ||
[Fact(DisplayName = "WatchAsync should return true when actor is terminated")] | ||
public async Task WatchAsync_should_return_true_when_actor_is_terminated() |
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.
Happy path use case test.
} | ||
|
||
[Fact(DisplayName = "WatchAsync should return true when called on actor that is already terminated")] | ||
public async Task WatchAsync_should_return_true_when_actor_is_already_terminated() |
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.
Idempotent use case - watching an already dead actor.
} | ||
|
||
[Fact(DisplayName = "WatchAsync should return false when cancellation token is cancelled")] | ||
public async Task WatchAsync_should_return_true_when_cancelled() |
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.
Negative case - cancellation.
{ | ||
var dw = message as DeathWatchNotification; |
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.
pattern matching refactoring
{ | ||
//TODO: find a way to get access to logger? | ||
Console.WriteLine("BUG: illegal Watch({0},{1}) for {2}", watch.Watchee, watch.Watcher, this); | ||
if (!AddWatcher(watch.Watcher)) |
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.
More automatic refactoring
var unwatch = message as Unwatch; | ||
if (Equals(unwatch.Watchee, this) && !Equals(unwatch.Watcher, this)) RemoveWatcher(unwatch.Watcher); | ||
else Console.WriteLine("BUG: illegal Unwatch({0},{1}) for {2}", unwatch.Watchee, unwatch.Watcher, this); | ||
case Unwatch unwatch when Equals(unwatch.Watchee, this) && !Equals(unwatch.Watcher, this): |
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.
Automatic refactoring
} | ||
catch | ||
{ | ||
// no need to throw here - the returned `false` status does the job just fine |
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.
Behavioral change for GracefulStop
- no longer throws TaskCancelledException
s when cancelled. Why? Because returning false
does this in a much more easily handleable way. The exception adds no value.
/// <param name="token">Optional - a cancellation token.</param> | ||
/// <returns><c>true</c> if the target was terminated, <c>false</c> otherwise.</returns> | ||
/// <exception cref="InvalidOperationException">Thrown if we attempt to watch a</exception> | ||
public static async Task<bool> WatchAsync(this IActorRef target, CancellationToken token = default) |
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.
After I refactored the guts of PromiseActorRef
implementing WatchAsync
was pretty easy to implement - it's just the GracefulStop
code but without sending a kill message to the actor and it uses CancellationToken
instead of a TimeSpan
. That's all.
WatchAysnc
methodsWatchAysnc
methods for monitoring actor lifecycles outside of Akka.NET
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.
LGTM
Changes
Prototyping a method to death watch an
IActorRef
from outside theActorSystem
.Checklist
For significant changes, please ensure that the following have been completed (delete if not relevant):
Documentation
I'll need to add a whole page on "working with actors outside Akka.NET" - this will get featured there. That'll also talk about Ask, GracefulStop, and so on.