[WIP] Cluster singleton

[yim-lee]

Motivation:
Add feature similar to Akka's cluster singleton.

Modifications:
Implement a simple version of the feature based on membership. The singleton will run on the leader.

Result:
Resolves #273.

[yim-lee]

⚠️ This is WIP (no tests yet). I would like to get feedback to make sure this is the direction we want to go.

[ktoso]

The shape looking good! Some things to polish and discuss but the shape is looking fine :)

[ktoso]

Starts to take good shape :)

[ktoso]

The plugins I think we might need to reshape a bit...
I started trying to make it possible to have plugins be other modules: ktoso@17d7645 but ran out of time and brain power for tonight.

I think either works: trying to polish the plugins and get them in along with the singleton, or have the singleton be manually spawned for now and we do the plugins and "plugin-ify" the singleton -- your call which you'd prefer.

The main thing about the plugins for now would be to ensure they can be added as other modules -- a few spots in the current design make that not a thing. Sorry to be a bother about this :)

OR we could make the singleton work with this state and then rip it out into separate module as we mature the Plugins?

[ktoso]

I think we should be able to get away with just one "starting(system)" and "shuttingDown(system)" (names to be bikeshed 😉), without the init complete/init (the just onSystemInit i even feel is somewhat unsafe, since not all fields are populated on system yet...).

Why the API as Result<Void, Error>? That's equivalent to throws I guess...

I'm ok with crashing if any of the inits fails btw, that sounds good.

// In the long run would be nice to have those be async... Let's not go there now though; synchronous is fine 👍

[yim-lee]

Right, now there's just start and stop as in your PoC.

Why the API as Result<Void, Error>? That's equivalent to throws I guess...

I think Result is preferred over throws. Can change to throws if that's more consistent with the rest of the code base.

It results in fatalError anyway.

[ktoso]

I think Result is preferred over throws. Can change to throws if that's more consistent with the rest of the code base.

So result makes sense if it'd be directly passed around a lot, but otherwise throws is totally fine in Swift IMHO since it boils down to the same thing kind of (also performance wise and by forcing to handle it). If we specifically wanted to Error: SpecificThing that'd be different, but we dont :)

[yim-lee]

Singleton as separate module, following ktoso@17d7645: d3c6e97

let system = ActorSystem() { settings in
    var singletonPluginSettings = ActorSingletonPluginSettings()
    singletonPluginSettings.add("singleton-1", behavior)

    settings.plugins.add(Plugin.singleton(singletonPluginSettings))
}

let singletonRef: ActorRef<M> = system.singleton.ref("singleton-1")

[yim-lee]

OK, @ktoso this is a lot cleaner

[yim-lee]

not sure if there is another way to stop the proxy

[ktoso]

I see; so in general this is a good pattern to bind lifecycles; I am not sure yet if it's the best in this specific case if we'd do some other allocation strategies. Commented in another place about this some more.

Good to keep as is for now though :)

[yim-lee]

hand-off if asked to stop

[ktoso]

ok, and we'll fill in that handing off some day (ticket please perhaps?)

[yim-lee]

#329

[yim-lee]

So right now stopping the proxy is a consequence of stopping the manager.

[ktoso]

Such linking of lifetime in general is good 👍

I wonder if it is the right way... What if we want to have an allocation strategy that says that this node will never host the singleton, and thus we'd never start the manager? In our current scheme, does it mean we always spawn manager and proxy, even if the manager will never spawn the child?

Perhaps the watching needs to be the other way around?

Right now we don't support the "only find the singleton but never host it" and that's only then when this would show up... Let us do a separate ticket or this? I think your plan was to address this using allocation strategies -- I wonder if it means we'd alway spawn the manager anyway 🤔

[yim-lee]

Good point. I am going to reintroduce ActorSingletonShell and shift some responsibilities there:

• Subscribes to ClusterEvent
• Spawns manager (if needed, based on AllocationStrategy) and proxy
• Tells manager and proxy whenever singleton node changes

This would decouple manager and proxy and allow better lifecycle management (i.e., if shell stops then its children proxy and manager also stop). We also wouldn't be running manager on every node.

[yim-lee]

I wonder if it means we'd alway spawn the manager anyway

In the current scheme, yes. Because we need an actor subscribes to ClusterEvent so that it knows where the singleton is, and that's what the manager does right now so it can provide that information for the proxy.

IIUC in Akka the proxy and manager just go their own ways to identify the singleton node. I prefer one source of truth, so if we keep both proxy and manager then only one of them should make the decision and tell the other, or have someone else making the decision and tell both of them.

We could eliminate ActorSingletonManager and just have ActorSingletonProxy do it all (i.e., subscribe to ClusterEvent, spawn/stop singleton, etc.). The downside of it is we can't tell ActorSingletonProxy to "stop" because its message protocol is restricted to the singleton actor's.

ActorSingletonShell and ActorSingletonManager are the same thing really. If we stick with proxy + manager, then the question would be whether manager should spawn the proxy as child or not. If yes, we would have a clean way to stop proxy but then ActorSingleton (non-actor) would have to ask the manager for the proxy ref. If no, then proxy would have to watch manager and stop itself (i.e., current implementation).

[ktoso]

OK and we'll revisit the fixme during hardening 👍

[ktoso]

(I know it's "my" fixme :))

[yim-lee]

Is this a matter of failing if proxy is not yet defined?

[ktoso]

Yeah since there it's a internal private(set) var proxy: ActorRef<Message>! and I worry about thread-safety what if someone spawned an actor and reached to system.singleton.ref(...) and that ref was not initialized yet -- there's no strong guarantee we will get the memory visibility right if the proxy is initialized in T1, but we're accessing from some actor in T2... so perhaps those need locks around the accesses actually.

[ktoso]

Looking good, it's getting there :-)

[yim-lee]

Please don't review latest commits yet. I just wanted to push my local changes.

I would like to reduce to a single actor instead of having separate proxy, manager, etc., pending a new feature that Konrad added for his other works.

[ktoso]

I would like to reduce to a single actor instead of having separate proxy, manager, etc., pending a new feature that Konrad added for his other works.

This just landed, it is ActorRef(ActorRef.Personality) where one can implement any functionality using the CellDelegate: Personality. The conceptual idea is "it's not a real Cell, but can act as if" (cell is what holds the real "actor").

[ktoso]

The failure is something fixed now btw :)

[ktoso]

Some minor discussions in line but overall this is very good, let's take it from here! 👍

[ktoso]

Ok; I wonder if can be simplified but this can be good :)

[yim-lee]

Yeah, this is what I don't like about the current impl. It looks so difficult/complex to get the singleton ref.

[ktoso]

Yeah, I made it a ticket to revisit at some point #348

low prio since does not change functionality too much anyway

[ktoso]

@yim-lee I rebased and squashed it up into 6 commits or so, to keep its steps but not all the minor commits like refactoring.

Really good one, let's see it in action and add more tests and stuff as we go :)

[ktoso]

[yim-lee]

Thanks @ktoso 🙇‍♀

Opened #347 to address feedback.

[ktoso]

Cool! That improves things :)