RedHatInsights / yggdrasil

GNU General Public License v3.0
21 stars 37 forks source link

feat: Add more events for starting and stopping of worker #187

Closed jirihnidek closed 11 months ago

jirihnidek commented 11 months ago
jirihnidek commented 11 months ago

Output on terminal with echo worker:

jhnidek@thinkpad-p1:~/github/redhat-insights/yggdrasil$ go run ./worker/echo -log-level trace
2023/11/13 09:18:02 connecting to session bus: unix:path=/run/user/1000/bus
2023/11/13 09:18:02 emitting event STARTED
^C2023/11/13 09:18:08 emitting event STOPPED

Output on terminal with yggd:

[yggd] 2023/11/13 09:18:02 /home/jhnidek/github/redhat-insights/yggdrasil/internal/work/dispatcher.go:113: received signal: &dbus.Signal{Sender:":1.203", Path:"/com/redhat/Yggdrasil1/Worker1/echo", Name:"com.redhat.Yggdrasil1.Worker1.Event", Body:[]interface {}{0x4, "", "", map[string]string{}}, Sequence:0x7}
[yggd] 2023/11/13 09:18:02 /home/jhnidek/github/redhat-insights/yggdrasil/cmd/yggd/client.go:191: emitted event: {Worker:echo Name:STARTED MessageID: ResponseTo: Data:map[]}
[yggd] 2023/11/13 09:18:08 /home/jhnidek/github/redhat-insights/yggdrasil/internal/work/dispatcher.go:113: received signal: &dbus.Signal{Sender:":1.203", Path:"/com/redhat/Yggdrasil1/Worker1/echo", Name:"com.redhat.Yggdrasil1.Worker1.Event", Body:[]interface {}{0x5, "", "", map[string]string{}}, Sequence:0xa}
[yggd] 2023/11/13 09:18:08 /home/jhnidek/github/redhat-insights/yggdrasil/internal/work/dispatcher.go:117: cannot find sender: cannot get name for sender: :1.203
subpop commented 11 months ago

If I'm not mistaken, this PR fixes #135, correct?

jirihnidek commented 11 months ago

If I'm not mistaken, this PR fixes #135, correct?

No, it does not. It could be used as a first step of getting correct list of workers, but you proposed better approach (easier to implement and more reliable).

Your idea was something like this: "It will be easier to list all workers using interface com.redhat.Yggdrasil1.Worker1.*"

You are welcome,

Your External Memory ;-)

jirihnidek commented 11 months ago

Works great! Can we follow this up with a PR that re-emits these events on the public com.redhat.Yggdrasil1 D-Bus interface?

Do you mean that other workers could have some benefits from the fact that there is another buddy processing dispatched messages? Interesting idea, but I would implement that, when it will be really needed. We have many more urgent card in backlog.

subpop commented 11 months ago

If I'm not mistaken, this PR fixes #135, correct?

No, it does not. It could be used as a first step of getting correct list of workers, but you proposed better approach (easier to implement and more reliable).

Your idea was something like this: "It will be easier to list all workers using interface com.redhat.Yggdrasil1.Worker1.*"

You are welcome,

Your External Memory ;-)

Oh yes, I remember that now. I just commented on #135 with that suggestion so that Future Link can be reminded as well.

subpop commented 11 months ago

Works great! Can we follow this up with a PR that re-emits these events on the public com.redhat.Yggdrasil1 D-Bus interface?

Do you mean that other workers could have some benefits from the fact that there is another buddy processing dispatched messages? Interesting idea, but I would implement that, when it will be really needed. We have many more urgent card in backlog.

No, yggd implements the com.redhat.Yggdrasil1 interface and exports itself on the main system bus. This is the API through which yggctl communicates with yggd. Part of this interface is a signal, WorkerEvent. That signal is emitted when workers emit events. Looking at the code that emits the signals in yggd's client.go, the STARTED and STOPPED signals will be emitted, but STARTED and STOPPED will be undocumented signals until the com.redhat.Yggdrasil1 D-Bus interface specification is updated to mention them.

jirihnidek commented 11 months ago

Works great! Can we follow this up with a PR that re-emits these events on the public com.redhat.Yggdrasil1 D-Bus interface?

Do you mean that other workers could have some benefits from the fact that there is another buddy processing dispatched messages? Interesting idea, but I would implement that, when it will be really needed. We have many more urgent card in backlog.

No, yggd implements the com.redhat.Yggdrasil1 interface and exports itself on the main system bus. This is the API through which yggctl communicates with yggd. Part of this interface is a signal, WorkerEvent. That signal is emitted when workers emit events. Looking at the code that emits the signals in yggd's client.go, the STARTED and STOPPED signals will be emitted, but STARTED and STOPPED will be undocumented signals until the com.redhat.Yggdrasil1 D-Bus interface specification is updated to mention them.

New signals are documented here: https://github.com/RedHatInsights/yggdrasil/pull/187/files#diff-ec9037acd507f9680945dd78261e1be61e857c68fa392d3bcba5e10332e0e2a9 Should these signals be documented elsewhere?

So, do I understand you correctly that you want to transmit STARTED and STOPPED signals in the same ways as WORKING signal is transmitted here: https://github.com/RedHatInsights/yggdrasil/blob/cc61d418ff13edad67b35d22d01ac9e67c8d9160/cmd/yggd/client.go#L180-L193

subpop commented 11 months ago

New signals are documented here: https://github.com/RedHatInsights/yggdrasil/pull/187/files#diff-ec9037acd507f9680945dd78261e1be61e857c68fa392d3bcba5e10332e0e2a9 Should these signals be documented elsewhere?

Yes. That is the D-Bus API used by workers to communicate with their "dispatcher". That API could run on any bus: the system bus or a private session bus. The D-Bus interface specification I linked to is the "public" D-Bus API that is exported by yggd in order to enable yggctl communication.

So, do I understand you correctly that you want to transmit STARTED and STOPPED signals in the same ways as WORKING signal is transmitted here:

https://github.com/RedHatInsights/yggdrasil/blob/cc61d418ff13edad67b35d22d01ac9e67c8d9160/cmd/yggd/client.go#L180-L193

Given the way that code is written, they will be emitted, but their "Name" value might be unknown. We should also ensure that they're documented in the externally facing D-Bus API so that clients and code generation utilities can expect additional values beyond 1, 2 and 3.