Skip to content
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

Get rid of static/singletons ? #431

Closed
jrouaix opened this issue Sep 27, 2018 · 24 comments
Closed

Get rid of static/singletons ? #431

jrouaix opened this issue Sep 27, 2018 · 24 comments

Comments

@jrouaix
Copy link
Contributor

jrouaix commented Sep 27, 2018

Hello @rogeralsing, @cpx86

Can I ask why you design such static/singletons stuffs ?

I was the Remote unit test, confirming that the only way to test such thing is to run another process.

Don't you think it could be better if we get rid if all these singletons and encapsulate them into some root "ActorSystem" instance, so we can start mutliple ActorSystem in a unit test more easily ?

Would it then possible to have multiple GRPC endpoint (associated with distinct/isolated ActorSystems) again in order to easier Remote tests ?

@cpx86
Copy link
Contributor

cpx86 commented Sep 28, 2018

I've thought about changing that as well. I think Roger and I have discussed it at some point, but tbh I can't remember why we decided not to change it 😅

I'd say that if we do change it then it should be for solving an important use case, e.g. if we want to have support for isolating actors or exposing multiple GRPC endpoints within a single process, and not out of language/design purism.

@rogeralsing
Copy link
Contributor

TLDR;

if you have instances of remoting, then the serialization layer needs to understand what instance it should deserialize its messages for, context aware messages.
e.g. an PID.

This puts an enormous burden on the serializer and forces you to use custom serialization schemes.

@rogeralsing
Copy link
Contributor

Thinking about it, now with the new API with context.Send. this might not be an issue as the context will know the context so to say :-)

@jrouaix
Copy link
Contributor Author

jrouaix commented Sep 28, 2018

Ok, it's on my "will do if time" list.

@cpx86
Copy link
Contributor

cpx86 commented Jun 11, 2019

@rogeralsing Not sure if you remember exactly what you were thinking 256 days ago 😄 But how would we go about this more precisely? Should we just move e.g. the static logger factory to the context instance? In that case what about remoting? Would we have a "remote context" inheriting from the root context but adding the remote-specific instances like the server, serialization config, internal processes, etc?

@rogeralsing
Copy link
Contributor

Let's see if I remember the reasoning here.

Deserilizing ActorRef in Akka.NET was a pain as it needs to be attached to the ActorSystem it belonged to.
So instead, ProtoActor went for a simpler approach where PID is like a phone number, one dont need to know anything more about its content or context.

This lead to the static API.
PID.Tell/Send would know where to forward the message by the processregistry.

But, now that Tell has moved onto the context, e.g RootContext, ActorContext.
We already have all the information needed for passing the message to the correct instances.

e.g. if the process registry was not static, but an instance.
as long as we could share that instance between RootContexts and ActorContexts, everything would be fine.
The process registry would be sort of the ActorSystem in Akka.
But without the magic.

@rogeralsing
Copy link
Contributor

Pseudo:

var registry = new ProcessRegistry();
var rootContext = new RootContext(registry);
var pid = rootContext.SpawnNamed(props, "MyActor");
rootContext.Send(pid, message);

Through all of this code, we know the context and what registry to use.
rootContext.Spawn would associate the registry with the spawned actor.
rootContext.Send would know which registry to look in when sending the message

@rogeralsing
Copy link
Contributor

This would even work for remoting.

if remoting is started on a given port for a given processregistry.
the endpoint reader for this system would pick up any incoming messages, and know how to forward those to the correct actors in the current process registry.

@cpx86
Copy link
Contributor

cpx86 commented Jun 11, 2019

The process registry would be sort of the ActorSystem in Akka. But without the magic.

Yes! Feels very much in line with the protoactor approach. I always kind of liked the ActorSystem in Akka since it's a fully isolated encapsulation of a set of actors. One thing though - is there any reason to separate the registry from the root context (by new RootContext(registry))? E.g. would you ever want to re-use the same registry across different root contexts? A simpler approach could be to let the root context be the root of everything and let it manage it's process registry (and other objects) on it's own.

@rogeralsing
Copy link
Contributor

E.g. would you ever want to re-use the same registry across different root contexts

You can set up different middleware for the rootcontext.
Not sure if that is an actual issue, but lets say you have one rootcontext for your Asp.NET API endpoints, and another for something else.
You might want to configure and provide middleware for tracing differently in those cases

@alexeyzimarev
Copy link
Member

I like the idea of the ActorSystem. Haven't worked with Akka but used it in Orleankka. One of the issues I have with Proto.Actor is that I cannot get an existing actor from the system by name unless I keep track of spun up actors myself. I had to work around this issue by creating my own ActorRegistry.

Concerning the ability to have multiple actor systems or root contexts in one application, I am not sure. Is there a use-case for it. to have two separate actor systems in one app? Isn't that the idea to have all actors to talk to each other within and outside the process boundaries?

@rogeralsing
Copy link
Contributor

Isn't that the idea to have all actors to talk to each other within and outside the process boundaries?

Yes and this is AFAIK how it works in Erlang.
But for Testing, being able to have isolated islands that dont interfere (too much) with eachother is nice.

Also, having things static, makes it hard to configure things.
even if you have a single system, if it is an instance, it can be configured more easily.

@cpx86
Copy link
Contributor

cpx86 commented Jun 11, 2019

You might want to configure and provide middleware for tracing differently in those cases

So would the use case be if you have the same set of actors exposed on different endpoints (possibly different transports) and you want to configure different actor middlewares per endpoint?

I'm not disregarding the idea completely, but I'm unsure if there is a case for it. As I recall it, the reason for introducing middlewares originally was basically to have reusable "behaviors" that you can easily inject via the Props, whereas the reason for the root context was to be able to propagate metadata within messages sent between actors and across remote boundaries. I don't see either of those cases being hurt by having a 1:1 relationship between registry and context.

@rogeralsing
Copy link
Contributor

Let's say you have Web API endpoints and NServiceBus endpoints, and you use Zipkin.
Now your endpoints needs to collect zipkin span data from the payload, and use that to configure your rootcontext.

in one case you would have something like:

//WebAPI endpoint

var spanid = Request.HttpHeaders.blablablsa
//config rootcontext and middlewares with the data from the above
rootContext.Send(...);

And in the other case:

//NServiceBus endpoint:

var spanid = message.MessageHeaders.blabla
//config rootcontext and middlewares with the data from the above
rootContext.Send(...);

@cpx86
Copy link
Contributor

cpx86 commented Jun 11, 2019

Isn't that the idea to have all actors to talk to each other within and outside the process boundaries?

Yes and this is AFAIK how it works in Erlang.
But for Testing, being able to have isolated islands that dont interfere (too much) with eachother is nice.

But isn't Erlang a very different beast since the VM itself has a process abstraction such that the Erlang processes don't need to cater for out-of-(OS-)-process communications? I'm all for emulating the good parts about Erlang but replicating the (quite peculiar) behaviors of it's VM in other VM's feels like it might be taking it too far.

I believe we said at some point that the core concepts of protoactor should remain the same across languages/platforms, but it should also stay idiomatic to those. :)

@cpx86
Copy link
Contributor

cpx86 commented Jun 11, 2019

@rogeralsing I might be missing something, but in that example wouldn't you anyway be manually picking up a request/message from WebAPI/NSB and forwarding it to an actor? If so, couldn't you just wrap it in a MessageEnvelope yourself and add the appropriate headers?

@rogeralsing
Copy link
Contributor

If so, couldn't you just wrap it in a MessageEnvelope yourself and add the appropriate headers?

Maybe :-)
Let's say you have some internal usages of root context then, there are some in remoting and cluster.
You likely don't want to apply application level middlewares in the internals of ProtoActor itself.

@cpx86
Copy link
Contributor

cpx86 commented Jun 11, 2019

You likely don't want to apply application level middlewares in the internals of ProtoActor itself.

True but internally we can do whatever we want 😉 We should be able to quite easily bypass app-level middlewares for system-level actors/processes, no?

@rogeralsing
Copy link
Contributor

Yes absolutely.
I'm all for having a single root context, it seems odd to have to create new ones everywhere.

But, fiddling with the MessageEnvelope is smelly IMO.
Something cleaner there would be neat

@cpx86
Copy link
Contributor

cpx86 commented Jun 11, 2019

But, fiddling with the MessageEnvelope is smelly IMO.

From the user/app perspective yes. But wouldn't you typically wrap that in your own middleware/glue-code anyway such that it doesn't touch your app code? E.g. let's say your app subscribes to messages from and routes them to internal actors; for any non-trivial app I'd prolly write a small middleware layer to handle that routing; would it really be smelly to handle MessageEnvelopes within that routing layer?

Something cleaner there would be neat

I do agree though that if we could hide/avoid the need for fiddling with the MessageEnvelope directly and create some sort of abstraction around it, that'd prolly be better :)

@adamhathcock
Copy link
Member

As far as statics are concerned, I may have started making an ActorSystem off with the extensions: https://github.com/AsynkronIT/protoactor-dotnet/blob/dev/src/Proto.Actor.Extensions/ActorFactory.cs

What else should be done?

@sudsy
Copy link
Contributor

sudsy commented Sep 1, 2019

I can only speak for the issues I encountered in #462

At the moment it is difficult (not impossible) to write good tests to simulate remote failure scenarios.

Being able to spin up multiple actor systems within the same process and be able to connect / disconnect them from each other to ensure that remote systems recover correctly from failure would be of great benefit IMHO.

Looking back through the comments on this issue it seems like the process registry needs to be tied to the actor system and that Proto.Remote needs to be refactored to allow two actor systems to talk to each other within the same process (presumably listening on different ports)

I am not sure if this is what is needed to support #467 too.

@rogeralsing
Copy link
Contributor

#497

@rogeralsing
Copy link
Contributor

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants