Fixes #30730 - Implement systemd notify support for dynflow-sidekiq

[adamruzicka]

No description provided.

[theforeman-bot]

Issues: #30730

[adamruzicka]

On my machine (4cpus, 16gb of ram) the service failed to start in time. Foreman service uses 300 so I set it here as well.

[ekohl]

Overall 👍 for the general design.

I am wondering if there's a useful status message we can share. Is it useful to share the queues it's handling? Whether it's remote? Most services don't imlpement this, but Apache shows Total requests: 0; Current requests/sec: 0; Current traffic: 0 B/sec when you run systemctl status apache.

Could you also submit the RPM packaging PR?

[adamruzicka]

I am wondering if there's a useful status message we can share.

Having it display the throughput/stats would be nice, however I'm afraid we'd have to implement this change in Dynflow or somehow hook onto sidekiq's internals. Let me think on this over the weekend if I can come up with something that would be useful

Is it useful to share the queues it's handling?

Could be to some users. However we'd have to do this sensibly since there is no limitation placed on the number of queues being handled by a single process.

Whether it's remote?

What do you mean by this?

Could you also submit the RPM packaging PR?

On my todo list.

[ekohl]

Could be to some users. However we'd have to do this sensibly since there is no limitation placed on the number of queues being handled by a single process.

Perhaps if it's more than x (3?) then you can print Handling x queues instead of the actual values.

What do you mean by this?

Essentially the value of ::Rails.application.dynflow.config.remote. Not sure if it's interesting. Perhaps this is always true in production setups and thus irrelevant.

[adamruzicka]

Essentially the value of ::Rails.application.dynflow.config.remote. Not sure if it's interesting. Perhaps this is always true in production setups and thus irrelevant.

In production setups the orchestrator should have it false and workers true. Not sure if that's interesting to anyone

[ekohl]

Probably not. And perhaps all of these things should rather be logged during start up and that's sufficient. I certainly don't want to force you to implement the status. Just make you think about the use case.

[ekohl]

👍 code wise pending packaging merge. My only concern is that we should somehow note that sidekiq 6 has native support for systemd. If we upgrade, we should be sure to use that. Thoughts on how to do so?

[adamruzicka]

Sidekiq seems to implement ready and stopping state notifications and watchdog. Once we start using Sidekiq 6, I will have to look what is the ordering between our notifications and the ones in Sidekiq. For ready, we should drop ours since Sidekiq's will happen later, however for stopping I'm not sure, but here we should pick the one that happens earlier.

[ekohl]

I just noticed you created a new issue rather than reusing mine. Left a note @ https://projects.theforeman.org/issues/30275#note-4 and related both issues. I think that should be sufficient.

[ehelms]

Packaging of the gem has been merged. A follow up PR will be needed to update Foreman requirements on the gem and then we can orchestrate merging this.

[ekohl]

Merging. Automated tools work better after it's merged.

[ekohl]

theforeman/foreman-packaging#5735