-
Notifications
You must be signed in to change notification settings - Fork 60
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
swift-clusterd PoC which serves as seed node #1155
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some formatting issues (guess better run swift format) and nitpicky comments, otherwise really like the idea.
As for follow ups would be nice to have it in docs and maybe some small demo.
@Option(help: "The host address to bid the cluster daemon on.") | ||
var host: String = ClusterDaemon.defaultEndpoint.host | ||
|
||
mutating func run() async throws { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should not be mutating
? 🤔
daemon.system.log.warning("RUNNING ClusterD DEBUG binary, operation is likely to be negatively affected. Please build/run the ClusterD process using '-c release' configuration!") | ||
#endif | ||
|
||
try await daemon.system.park() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
await is not needed, for now at least
@@ -404,31 +404,65 @@ protocol ClusterSystemInstrumentationProvider { | |||
/// all the nodes of an existing cluster. | |||
public struct ServiceDiscoverySettings { | |||
let implementation: ServiceDiscoveryImplementation | |||
private let _initialize: (ClusterSystem) -> Void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit unclear naming, took some time to figure out, guess something more specific about cluster daemon, like _initializeClusterd
. Wonder also if it should be optional and only set in init(clusterdEndpoint: Cluster.Endpoint)
, otherwise again—not very clear function.
} | ||
|
||
/// Locate the default `ClusterD` process and use it for discovering cluster nodes. | ||
public static func clusterd(endpoint: Cluster.Endpoint?) -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's neat, my initial impression was it's for local clustering, but guess we can define any endpoint.
"MultiNodeTestKit", | ||
.product(name: "ArgumentParser", package: "swift-argument-parser"), | ||
] | ||
), | ||
|
||
.executableTarget( | ||
name: "Clusterd", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rename to swift-clusterd
now? Or at least lowercased clusterd
would be better.
Quick PoC how a
clusterd
daemon could look like.Usage in practice would be to just ensure we start the daemon OR if ANY of the nodes is using daemon discovery make sure one of them spawns it or something like that.
This way nodes can join without hardcoding discovery of peer nodes when playing around locally.
This is inspired by https://forums.swift.org/t/beginner-advice-for-distributed-distributedcluster-modules/72391/11 and erlang's shortnames + https://www.erlang.org/docs/25/man/epmd
Creating a few nodes in a system would literarily be:
this basically just does:
This is the same as epmd, it has to be running somewhere.
And applications just do:
and you're done:
they'll join eachother.
Follow up here:
ClusterD
should be a "HIDDEN" member in the membershipswift-clusterd
(the cluster daemon for the swift distributed cluster)resolves #1152