[phased-restart] add support for systemd type notify

[josef-krabath]

The PR #2438 added support for systemd services of type notify.

# lib/puma/systemd.rb
module Puma
  class Systemd
    ...
    
    def hook_events
      @events.on_booted { SdNotify.ready }
      @events.on_stopped { SdNotify.stopping }
      @events.on_restart { SdNotify.reloading }
    end
    
    ...
  end
end

The Problem:
The systemd status gets updated as expected when performing a hot restart.
But the status stays active when performing a phased restart.
In the current implementation, SdNotify.reloading is never called during a phased restart.

A Common Scenario:
If we perform a phased restart, Puma terminates an old worker and starts a new one.
And if Puma is not able to boot up a new worker for some reason, it keeps trying.

In this case, the systemd status shouldn't stay active.
During a phased restart the status should be reloading.

Additional Context:
We monitor the status of all our systemd processes.
If the status of a process is not active any more (for a certain period of time), we get notified.

It would be awesome to keep this monitoring approach for Puma as well.

[MSP-Greg]

@josefbilendo

To clarify, with a phased-restart, would you like on_restart to be called when Puma starts the restart process, and on_booted to be called when all workers are booted? Or maybe when the first worker is booted?

[josef-krabath]

@MSP-Greg

In my opinion we should invoke on_restart when Puma starts the phased-restart
and call on_booted when all workers are booted up.

I think it makes sense to keep the "reloading" state till the phased-restart is complete – because that's a more accurate representation of the real state.

[MSP-Greg]

@josefbilendo

I've got an 'almost finished' patch for this.

I'm wondering if it would be better to add another hook, maybe 'on_restart_*', where * is one of 'done', 'finished', or 'complete'?

[josef-krabath]

@MSP-Greg
Thanks a lot!

I don't think there is anything against implementing another hook as long as we update the service state.

[MSP-Greg]

@josefbilendo Thanks for the reply. Right now I've got it working as you mentioned, iow, both hot & phased restart (both cluster & single) are calling @events.on_booted when the workers have booted.

Not sure about adding another method name...

@nateberkopec Thoughts? The passing test file in the new system:
https://github.com/MSP-Greg/puma/blob/test-update/test/test_popen_systemd.rb

[nateberkopec]

@MSP-Greg thoughts on adding a new method name/event type specifically, you mean?

[MSP-Greg]

@nateberkopec

Yes. I don't really have an opinion, wasn't sure if there was a need...

[josef-krabath]

@MSP-Greg

I've got an 'almost finished' patch for this.

How is your progress on this issue?
Can I help you somehow?

[MSP-Greg]

@josefbilendo

Sorry, the original work I did on this was tied into a bunch of other small changes...

Anyway, I'll push a PR today or tomorrow.

[josef-krabath]

@MSP-Greg
Thanks a lot!!