Added hook to lifecycle events #1150
Conversation
| public protocol PluginActorLifecycleHook { | ||
| func actorReady<Act: DistributedActor>(_ actor: Act) where Act: DistributedActor, Act.ID == ClusterSystem.ActorID | ||
| func resignID(_ id: ClusterSystem.ActorID) | ||
| } |
There was a problem hiding this comment.
Explicitly made non async as it's called synchronously in system.
This means though that if journal is an actor—those functions will be nonisolated. Thinking now if it's ok or not 🤔
There was a problem hiding this comment.
yeah those funcs being synchronous is good -- the plugin may have to use external synchronization, that's ok I guess -- we don't have a choice since the hooks are synchronous.
Naming nitpick, let's call them onActorReady and onResignID so it's easier to know which one we mean when we se this in code
|
Still not sure about |
| @@ -21,6 +21,7 @@ public struct PluginsSettings { | |||
| } | |||
|
|
|||
| internal var plugins: [BoxedPlugin] = [] | |||
| internal var actorLifecycleHooks: [PluginActorLifecycleHook] = [] | |||
There was a problem hiding this comment.
the front of the name is good; but maybe PluginActorLifecyclePlugin?
There was a problem hiding this comment.
Still not sure, maybe more like ActorLifecycleAwarePlugin? 🤔
There was a problem hiding this comment.
or just LifecyclePlugin... 🤔
There was a problem hiding this comment.
I see usage of ActorLifecycle term in the system, so will just call ActorLifecyclePlugin.
| await fulfillment(of: [hookFulfillment]) | ||
| } | ||
|
|
||
| actor TestClusterHookPlugin: _Plugin, PluginActorLifecycleHook { |
There was a problem hiding this comment.
Oh plugin was hidden still huh. About time to make it public -- maybe as we cleanup the how we use them with your work here!
There was a problem hiding this comment.
🫡
Will remove _ from Plugin
| @@ -114,3 +114,9 @@ internal struct AnyPluginKey: Hashable, CustomStringConvertible { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| /// Way to hook into ClusterSystem's actor lifecycle events, specifically to `ready` and `resign`. | |||
| public protocol PluginActorLifecycleHook { | |||
There was a problem hiding this comment.
Ah, so your idea is that it's still a plugin. Hm 🤔 I was more going into direction if you want to extend your current plugin, you can just conform it to additional protocol, like class CustomPlugin: Plugin, ActorLifecyleHookable or something.
Though also making it directly a plugin also makes sense, you basically distinguish between normal and hookable plugins 🤔
Tests/DistributedClusterTests/Plugins/ClusterSingleton/ClusterSingletonPluginTests.swift
Outdated
Show resolved
Hide resolved
Tests/DistributedClusterTests/Plugins/ClusterSingleton/ClusterSingletonPluginTests.swift
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| nonisolated func actorReady<Act>(_ actor: Act) where Act: DistributedActor, Act.ID == DistributedCluster.ClusterSystem.ActorID { | ||
| Task { try await self.onActorReady(actor) } |
There was a problem hiding this comment.
probably make use of a lock inside here and make it a class to avoid the racy nature of Task {}
There was a problem hiding this comment.
This is why I was worried about sync functions in protocol. Working with actors you'll mostly land in async anyway or your plugin could be an actor. And then you'll have nonisolated func actorReady with Task or some later stage.
Which is also ok though if you handle it inside your plugin I guess 🤔
There was a problem hiding this comment.
I know this is a very old discussion, but to clarify: the actor ready must be synchronous because we support synchronous init() in actors.
Co-authored-by: Konrad `ktoso` Malawski <[email protected]>
|
@swift-server-bot test this please |
|
@swift-server-bot add to allowlist |
|
@swift-server-bot test this please |
1 similar comment
|
@swift-server-bot test this please |
akbashev
force-pushed
the
plugin_lifecycle_hook
branch
from
1198b76 to
1679399
Compare
|
@ktoso thx for merging main, fixed issue with soundness, now should be fine |
Ability to listen to actors lifecycles in a plugin.
Motivation:
Plugins give an option to extend system with some logic. Problem is sometimes you need to hook into actor's creation/deletion, which currently is impossible. As an example—for actor recovery we need to restore state when it's created and ready for cluster system, which can be done by some potential
EventSourcingJournal.By adding simple hook protocol we can enable this.
Modifications:
PluginActorLifecycleHookprotocolactorLifecycleHooksin plugins settingsactorReadyandresignIDare called for each hookable plugin added to the system.