Replies: 4 comments 9 replies
-
Hi, Love the idea, thanks for sharing, comments: Dispatch(worker->ygdd):
Love this one, but:
On the dispatch, I would all the following methods:
This is to retrieve the config used by ygdd, we need to get access to the
This is to reload config on ygdd, the main use case is when updating certs, and This is important for us. Worker(ygdd->worker)I miss why this can be used, I cannot see anything that can be used for us if
Maybe the GetConfig should dispatch an event that it's ConfigUpdated, and this |
Beta Was this translation helpful? Give feedback.
-
I see this design removes the registration method from the protocol. How such call would be implemented using the new protocol? |
Beta Was this translation helpful? Give feedback.
-
@subpop Do we have any time line for this change? |
Beta Was this translation helpful? Give feedback.
-
I recently picked this work back up and while working on it, I came to the conclusion that VARLINK does not bring much additional benefit over gRPC to the protocol. If anything, it's actually less featured. While I was thinking over what protocols do bring additional functionality, I decided to look into using D-Bus as a worker/dispatcher IPC protocol. This proved to be very effective, and brings a number of nice features to the protocol: SignalsWe added a rudimentary "event" API in #54, but D-Bus has first-class support for signals. This means we can define in the protocol specification, signals like: <signal name="Event">
<arg type="u" name="name" />
</signal> Emitting this signal is as simple as: conn.Emit("/com/redhat/yggdrasil/Dispatcher1", "com.redhat.yggdrasil.Dispatcher1.Event", DispatcherEventReceivedDisconnect) A client (i.e. worker) can register with the bus to receive notifications when these signals are emitted: conn.AddMatchSignal(dbus.WithMatchObjectPath("/com/redhat/yggrasil/Dispatcher1", dbus.WithMatchInterface("com.redhat.yggdrasil.Dispatcher1"), dbus.WithMatchMember("Event") PropertiesD-Bus has first-class support for object properties. Properties can be connected to in order to receive notifications when values change. I haven't added them yet, but with Properties, we'll be able to have Overall, I'm very pleased with the way the D-Bus implementation has turned out. I published a branch that includes a working implementation of a D-Bus Dispatcher service and an echo worker. Please take a look and let me know if you see any glaring omissions or oversights. One other thing to note here is that for the sake of simplicity, I dropped gRPC from the codebase with this branch. While supporting two protocols is technically possible, it added a fair amount of complexity for little benefit. |
Beta Was this translation helpful? Give feedback.
-
This proposal is superseded by #93, however the proposal will remain here for posterity.
I'm planning on writing a new RPC protocol for workers to communicate with
yggd
. The current protocol has reached the limit of usefulness, and it's time to improve.Removing Registration
I am deprecating the Registration method in favor of explicitly defined worker service files (see PR #37). These configuration files include everything necessary for a worker to register with the dispatcher, so an RPC method call is no longer necessary.
Adding a VARLINK interface
Currently, the only supported RPC framework is gRPC. This has proved very effective, but has led to some issues with Python workers. I've explored VARLINK some, and have had good results with some basic POC tests. I plan to support gRPC and VARLINK, and allow workers to select the protocol they use to communicate.
Proposal
Using the VARLINK notation, here's my current proposed protocol. A similar protocol interface could be written for gRPC.
com.redhat.yggdrasil.dispatch
These are methods implemented by
yggd
that workers can invoke.com.redhat.yggdrasil.worker
These are methods implemented by workers that
yggd
can invoke.I'm sure this protocol is missing something. What is it leaving out? What other methods might we want to communicate between worker and dispatcher? I've deliberately not defined a
Data
type here with the idea of not exposing the concept of "messages" to workers. Good idea? Bad idea?Beta Was this translation helpful? Give feedback.
All reactions