Change default implementation of Actor::stopping

[thomaseizinger]

Returning StopAll is a dangerous default because it makes supervisor
patterns hard to implement. With StopAll, spawning a new instance of
an actor in a context if the old one dies results in the newly created
actor to also be shut down.

See get10101/itchysats@dc5c6bd.

[Restioson]

I'm okay with this in principle, but there is one footgun that would be good if we could avoid: Context::stop. I think this is probably the default stop method for single-actor configurations, and in this case StopSelf is equivalent to StopAll. This is why I made StopAll the default - so that when you spawn extra actors onto the same address, you don't need to change existing invocations of Context::stop. I'm fine with having this change, but do you have any ideas on how this footgun could be alleviated somewhat? Maybe Context::stop could be renamed a bit?

[thomaseizinger]

It is interesting that you are calling this a footgun because I would have expected Context::stop to always just stop the current actor and not all of them.

Is that not what you intended with this function?

[Restioson]

It is interesting that you are calling this a footgun because I would have expected Context::stop to always just stop the current actor and not all of them.

Maybe this is a flaw in my mental model - originally, xtra could not have multiple actors on the same address, so I guess I still think of Context::stop in this light of not having multiple actors on the address (i.e, once Context::stop is called, usually the address will stop responding to messages). Maybe it's fine to keep it like this though as it is and merge the PR.

[thomaseizinger]

I haven't personally used multiple actors on the same address with xtra yet.

All of our actors track some kind of state and load-balancing all messages across all instances doesn't play well with that. For the case where we have multiple instances of the same actor running, I'd usually need to route to a very concrete instance (because its internal state is relevant to the incoming message).

It might be worthwhile to consider cloning the messages and sending it to all actors on an address. Then the instances that don't care could just drop the message.

Coming back to Context::stop: We mostly use this to shut down an actor if its internal state is somehow corrupted (and needs to be restarted by its supervisor). It would actually be even better if there would be a mechanism of terminating the actor and passing a value along in one go. These actors basically follow the RAII model.

Another use of stop for us is for actors that have a limited lifetime because they represent a "process" that is running and will eventually finish with some result, even in the happy path.

So, however it is exposed, there should IMO be a way of terminating just the one instance of the actor that is handling a message.

[Restioson]

All of our actors track some kind of state and load-balancing all messages across all instances doesn't play well with that. For the case where we have multiple instances of the same actor running, I'd usually need to route to a very concrete instance (because its internal state is relevant to the incoming message).

Ah, ok. My usecase was for database actors, or search actors where they share some data that is internally synchronised, so they are stateless in and of themselves.

It might be worthwhile to consider cloning the messages and sending it to all actors on an address. Then the instances that don't care could just drop the message.

This can be done with Context::broadcast or similar

It would actually be even better if there would be a mechanism of terminating the actor and passing a value along in one go

How do you mean passing along a value?

So, however it is exposed, there should IMO be a way of terminating just the one instance of the actor that is handling a message.

Of course :)

[thomaseizinger]

It would actually be even better if there would be a mechanism of terminating the actor and passing a value along in one go

How do you mean passing along a value?

Something like: Context::stop(ThisIsMyReturnValue).

I am not sure if it is easy to balance all the tradeoffs:

• If we pass a value, it would be good to make it typesafe to not being able to pass more than one
• What if the actor stops without stop being called?

So perhaps it is not great ideal overall. For RAII actors, it is usually pretty clear as to why they are stopping (some error occurred and the connection is unusable) and having to store that as state just to retrieve it again in stopping was mildly annoying.

[Restioson]

Would that return value then be passed to stopping? Is there any advantage (except ergonomics) to using the value to determine whether to stop before calling ctx.stop() in the first place, or is it mainly ergonomic?

[thomaseizinger]

Would that return value then be passed to stopping?

Either that or we directly return it.

or is it mainly ergonomic?

In some ways it is ergonomics but there is another subtly to it. Managing the Stop value for the user within Context would force us to handle all the cases why an actor can be stopped explicitly (i.e. if we stop an actor because all strong addresses are gone, stop is not called by the user but the actor will still shut down.). Thus, it could create a more easy to debug foundation for why an actor is stopping in the first place. At the moment, this is pretty intransparent and hard to debug if all you have is logs.

[Restioson]

That's true! I think it could definitely be a good change.

[thomaseizinger]

I just realized an unintended side-effect of this change: StopSelf does not call mark_as_disconnected on the address and thus, a stopped actor's address still returns true for is_connected even after it has been stopped.

I'll see to add a test for this and then check if we can still return StopSelf for this.

[thomaseizinger]

I just realized an unintended side-effect of this change: StopSelf does not call mark_as_disconnected on the address and thus, a stopped actor's address still returns true for is_connected even after it has been stopped.

I'll see to add a test for this and then check if we can still return StopSelf for this.

I've opened a PR that demonstrates what I would consider to be a bug: #60

[thomaseizinger]

Closing in favor of #64.