Skip to content

Revert "Add readiness coordination for systemd notifications" - #107

Open
bastelfreak wants to merge 5 commits into
mainfrom
ezbake5
Open

Revert "Add readiness coordination for systemd notifications"#107
bastelfreak wants to merge 5 commits into
mainfrom
ezbake5

Conversation

@bastelfreak

Copy link
Copy Markdown
Contributor

This reverts #87 + contains #106

Signed-off-by: Tim Meusel <tim@bastelfreak.de>
This codepath is triggered during a `systemctl restart puppetserver` or
`systemctl stop puppetserver`.

Signed-off-by: Tim Meusel <tim@bastelfreak.de>
…e restart is ignored"

This reverts commit ba1c807.

Signed-off-by: Tim Meusel <tim@bastelfreak.de>
This reverts commit e548da1.

Signed-off-by: Tim Meusel <tim@bastelfreak.de>
@bastelfreak bastelfreak added the bug Something isn't working label Aug 21, 2026
@bastelfreak

bastelfreak commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

I guess I missed something during the revert, but I'm not sure what :(
edit: fixed it

This reverts commit 7ab2994.

Signed-off-by: Tim Meusel <tim@bastelfreak.de>
@bastelfreak
bastelfreak marked this pull request as ready for review August 21, 2026 14:11
@bastelfreak

Copy link
Copy Markdown
Contributor Author

If we revert this, we also need to remove the readiness call from openvox-server: OpenVoxProject/openvox-server#610.

@Sharpie Sharpie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this moves us in the right direction as it cuts a bunch of complexity that isn't solving the reload problem.

However, the end result seems very similar to the initial PR #23, so it feels as though there is a missing piece.

@Sharpie

Sharpie commented Aug 22, 2026

Copy link
Copy Markdown
Member

Ok. I think I found the problem(s). It might be a combination of:

  • When Type=notify-reload, SystemD will send SIGHUP by its self. Having ExecReload=kill -HUP $MAINPID causes it to send an additional SIGHUP and also return early from systemctl reload when kill finishes running. So, the app is stuck in the middle of handling that second reload when systemctl returns and isn't actually ready.

  • When using Type=notify-reload, SystemD expects to see a MONOTONIC_USEC= line included after the RELOADING=1 line in the message sent to the socket. It ignores any RELOADING=1 that does not include this and subsequent READY=1, and just waits until TimeoutStartSec elapses when it then fails. Having ExecReload= set short-circuits this as kill returning is enough to satisfy the timer.

Docs:

Behavior of notify-reload is similar to notify, with one difference: the SIGHUP UNIX process signal is sent to the service's main process when the service is asked to reload and the manager will wait for a notification about the reload being finished.

When initiating the reload process the service is expected to reply with a notification message via sd_notify(3) that contains the "RELOADING=1" field in combination with "MONOTONIC_USEC=" set to the current monotonic time (i.e. CLOCK_MONOTONIC in clock_gettime(2)) in μs, formatted as decimal string. Once reloading is complete another notification message must be sent, containing "READY=1". Using this service type and implementing this reload protocol is an efficient alternative to providing an ExecReload= command for reloading of the service's configuration.

https://www.freedesktop.org/software/systemd/man/latest/systemd.service.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants