Skip to content
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

Subscriptions with JobNew and JobRemoved #377

Open
WhyNotHugo opened this issue Aug 18, 2021 · 2 comments
Open

Subscriptions with JobNew and JobRemoved #377

WhyNotHugo opened this issue Aug 18, 2021 · 2 comments

Comments

@WhyNotHugo
Copy link

I'm looking at how SubscribeUnitsCustom is implemented, and it seems to continuously poll for the status of units and compare what's changed.

I think that the JobNew() and JobRemoved() D-Bus signals can be used to listen to this; whenever there's a request to start or stop a service, that creates a new Job and these signals trigger (I haven't found many references to jobs in systemd's docs, but did test this manually). This also applies to when a job dies (e.g.: kill -9).

This approach is a lot faster than polling (e.g.: we get notified when something happens immediately) and a lot less CPU intense (due to no polling).

My concern right now is how to exactly implement this. The current API takes an interval, but that would be unused with this change. I'm thinking that an interval of 0 would be an option, but that's a bit hacky; maybe simply have a new API and deprecate the existing one would be best.

Do you have any thoughts on this?

@lucab
Copy link
Contributor

lucab commented Dec 13, 2021

I think that the JobNew() and JobRemoved() D-Bus signals can be used to listen to this

I'm not very familiar with that portion of the systemd DBus API, but from what I'm seeing I think I agree it could be feasible that way.
One possible concern though is that this may have the opposite outcome of what you expect: on a system with a lot of dynamic jobs scheduling, this may increase CPU and I/O usage due to processing single wakeup events without batching.

maybe simply have a new API and deprecate the existing one would be best.

The existing API is quite nasty. It is at least missing a Context to stop the inner goroutine. So yes feel free to mark it with a deprecation notice and add a new method on the side. Adding an Evented label somewhere into its name would be good.

@WhyNotHugo
Copy link
Author

One possible concern though is that this may have the opposite outcome of what you expect: on a system with a lot of dynamic jobs scheduling, this may increase CPU and I/O usage due to processing single wakeup events without batching.

Right, that's true. The signals are optimised for receiving the event as soon as possible, at the cost of more CPU usage if there's lots of events. This is not usually the case though, generally system's don't have a large amount of jobs being scheduled.

I'm curious if anyone actually relies on the current batching/polling mechanics and would be disrupted if it were no longer an option -- I can't imagine, but lack of imagination doesn't mean it's not a thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants