Manual: why not restart service over killing the process

Manual: why not restart service over killing the process

[EuAndreh]

Hi!

The manual suggests a deploy-hook for the certbot-service-type that
looks like this:

(define %nginx-deploy-hook
  (program-file
   "nginx-deploy-hook"
   #~(let ((pid (call-with-input-file "/var/run/nginx/pid" read)))
       (kill pid SIGHUP))))

Instead of requiring the deploy-hook to know the path of the PID file,
why not restart the Shepherd service instead? Something like this:

(define %nginx-deploy-hook
  (program-file
   "nginx-deploy-hook"
   (with-imported-modules '((gnu services herd))
     #~(begin
         (use-modules (gnu services herd))
         (restart-service 'nginx)))))

If I understood correctly, those would result in equivalent outcomes,
and I tend to find the latter a more elegant approach. It is a bit
longer, but I like more restarting the service rather than killing the
process. Is there any downside I'm missing?

I can send a patch to the manual if this seems reasonable.

Re: Manual: why not restart service over killing the process

[Christopher Baines]

[-- Attachment #1: Type: text/plain, Size: 1449 bytes --]


EuAndreh <eu@euandre.org> writes:

> Hi!
>
> The manual suggests a deploy-hook for the certbot-service-type that
> looks like this:
>
> (define %nginx-deploy-hook
>   (program-file
>    "nginx-deploy-hook"
>    #~(let ((pid (call-with-input-file "/var/run/nginx/pid" read)))
>        (kill pid SIGHUP))))
>
> Instead of requiring the deploy-hook to know the path of the PID file,
> why not restart the Shepherd service instead? Something like this:
>
> (define %nginx-deploy-hook
>   (program-file
>    "nginx-deploy-hook"
>    (with-imported-modules '((gnu services herd))
>      #~(begin
>          (use-modules (gnu services herd))
>          (restart-service 'nginx)))))
>
> If I understood correctly, those would result in equivalent outcomes,
> and I tend to find the latter a more elegant approach. It is a bit
> longer, but I like more restarting the service rather than killing the
> process. Is there any downside I'm missing?

You're sort of right, but you've got the downsides the wrong way around.

The key bit with the kill call is the SIGHUP but, not that it's not
SIGKILL. The current situation won't kill the NGinx process, but instead
just get it to reload the certificate (at least that's the intention).

The restart action would "kill" the process, in that it would send it
SIGTERM and the the shepherd would start a new NGinx process, and this
has the potential of interrupting whatever is using NGinx.

Does that make sense?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 987 bytes --]

Re: Manual: why not restart service over killing the process

[EuAndreh]

Christopher Baines <mail@cbaines.net> writes:

> You're sort of right, but you've got the downsides the wrong way around.
>
> The key bit with the kill call is the SIGHUP but, not that it's not
> SIGKILL. The current situation won't kill the NGinx process, but instead
> just get it to reload the certificate (at least that's the intention).
>
> The restart action would "kill" the process, in that it would send it
> SIGTERM and the the shepherd would start a new NGinx process, and this
> has the potential of interrupting whatever is using NGinx.
>
> Does that make sense?

Hmm, great answer!

Makes sense, and the SIGHUP signal really is indeed better for this
case. Thanks for the informational response!

Re: Manual: why not restart service over killing the process

[Jason Conroy]

Perhaps a third option is adding a Shepherd "reload" action for nginx to
perform the SIGHUP, similar to this?

https://git.savannah.gnu.org/cgit/guix.git/commit/gnu/services/base.scm?id=d3f75179e5741db29358e3e723146fd20ec79de9

I'm curious whether this approach has trade-offs compared to what's
documented in the manual.

Jason

On Sat, Nov 28, 2020 at 10:44 AM Christopher Baines <mail@cbaines.net>
wrote:

>
> EuAndreh <eu@euandre.org> writes:
>
> > Hi!
> >
> > The manual suggests a deploy-hook for the certbot-service-type that
> > looks like this:
> >
> > (define %nginx-deploy-hook
> >   (program-file
> >    "nginx-deploy-hook"
> >    #~(let ((pid (call-with-input-file "/var/run/nginx/pid" read)))
> >        (kill pid SIGHUP))))
> >
> > Instead of requiring the deploy-hook to know the path of the PID file,
> > why not restart the Shepherd service instead? Something like this:
> >
> > (define %nginx-deploy-hook
> >   (program-file
> >    "nginx-deploy-hook"
> >    (with-imported-modules '((gnu services herd))
> >      #~(begin
> >          (use-modules (gnu services herd))
> >          (restart-service 'nginx)))))
> >
> > If I understood correctly, those would result in equivalent outcomes,
> > and I tend to find the latter a more elegant approach. It is a bit
> > longer, but I like more restarting the service rather than killing the
> > process. Is there any downside I'm missing?
>
> You're sort of right, but you've got the downsides the wrong way around.
>
> The key bit with the kill call is the SIGHUP but, not that it's not
> SIGKILL. The current situation won't kill the NGinx process, but instead
> just get it to reload the certificate (at least that's the intention).
>
> The restart action would "kill" the process, in that it would send it
> SIGTERM and the the shepherd would start a new NGinx process, and this
> has the potential of interrupting whatever is using NGinx.
>
> Does that make sense?
>