Create ClusterControl in ActorSystem directly #74 #89
Conversation
| public var cluster: ClusterControl { | ||
| guard self.settings.cluster.enabled else { | ||
| fatalError("Tried to access cluster control, but clustering is not enabled.") | ||
| } |
There was a problem hiding this comment.
Seems a bit too aggressive 🤔
Main thing is that we don't have the ref we can't issue the commands; though could we not direct them to dead letters then or log a warning that cluster is disabled?
A bit iffy on crashing hard on this, hm... 🤔 Even a "single node cluster" sometimes is a thing, without binding, so one might want to kick off things automagically assuming we are "up" when we are not bound to any port, but want to test our other logic about lifecycle etc 🤔
| if let servant = self._servants[pid] { | ||
| self.system.cluster._shell.tell(.command(.downCommand(servant.node.node))) | ||
| self.system.cluster.down(node: servant.node.node) | ||
| self._servants.removeValue(forKey: pid) |
ktoso
left a comment
ktoso
left a comment
There was a problem hiding this comment.
In general great, though had some doubts -- inline :)
| public func shutdown() { | ||
| self.log.log(level: .debug, "SHUTTING DOWN ACTOR SYSTEM [\(self.name)]. All actors will be stopped.", file: #file, function: #function, line: #line) | ||
| if self.settings.cluster.enabled { | ||
| if let cluster = self._cluster { |
There was a problem hiding this comment.
good to move from the settings check to this style 👍
| // initialized during startup | ||
| internal var _cluster: ClusterShell? | ||
| internal var _clusterEvents: EventStream<ClusterEvent>? | ||
| internal var _clusterControl: ClusterControl? |
There was a problem hiding this comment.
Nice; I wonder if we need to store the _cluster as well though?
All usage I guess would rather be though _control? we can always _clusterControl._shell?
There was a problem hiding this comment.
(Thinking to have as little fields for the same thing as we can for the core actor system; we'd have exactly one way to access the shell then etc)
| public var cluster: ClusterControl { | ||
| guard self.settings.cluster.enabled else { | ||
| fatalError("Tried to access cluster control, but clustering is not enabled.") | ||
| } |
|
It's a good improvement, so let's keep moving :) The comment about removing |
|
Opened #111 for discussion / followup |
Motivation:
Avoid storing
clusterEventsonActorSystemand also re-creatingClusterControlevery time it's being used.Result: