all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: "Clément Lassieur" <clement@lassieur.org>
To: Carlo Zancanaro <carlo@zancanaro.id.au>
Cc: 33508@debbugs.gnu.org
Subject: [bug#33508] [PATCH] gnu: Add ability to restart services on system reconfigure
Date: Mon, 26 Nov 2018 22:02:17 +0100	[thread overview]
Message-ID: <87k1kzd02u.fsf@lassieur.org> (raw)
In-Reply-To: <875zwj8uqz.fsf@zancanaro.id.au>

Hey Carlo!

Carlo Zancanaro <carlo@zancanaro.id.au> writes:

> I can add this, but I don't think this is as useful as it initially
> sounds. Most of our services are a specific version of a service pointing to a
> specific version of a configuration file (ie. that's in the store). That means
> that a "reload" shepherd action won't be able to know where the new
> configuration file is to load it.
>
> We could solve this in one of two ways:
>
> 1) by allowing an arbitrary procedure as the value of restart-strategy,
> because it can then call a shepherd action with the appropriate configuration
> file

> but then our action will have to detect whether the binary has been
> changed (which would also detect any dependencies changing).

I don't think it needs to detect whether the binary has changed, because
'reload' signals are usually implemented so that they can safely fail.
So, if the configuration file has changed in an incompatible way, the
'reload' action won't work, but the service will keep running.

> This may also lead to an inconsistent user experience where a
> "reconfigure" might lead to a reload, or might lead to a restart, and
> it's not obvious which it will be.

Your patch also leads to this inconsistency, because it allows a service
to either be restarted or not, in my opinion :-)

> 2) by changing our services to create configuration files in a known location
> (ie. /etc/nginx/nginx.conf). This would make it so a simple "reload" action in
> the service could meaningfully reload the service, but only if the binary was
> unchanged (because the old binary might not be able to read the new
> configuration format, for instance). This still leads to the above problem
> around the inconsistent user experience, and adds some complexity in terms of
> how configuration files are managed.
>
> I lean towards option (1), because it gives us the ability to call a shepherd
> action if we want, but also allows us to do arbitrary other things with the
> extra knowledge on the Guix side.

I think both (1) and (2) make sense because both kind of services (the
ones pointing to configuration files in the store and the ones using
/etc/some-file.conf) already exist.  Ideally, the mechanism should be
generic enough to handle both cases.

> In the end, though, I'm unconvinced that this is a useful thing to add. What
> do you think?

I don't agree :-).  A 'restart' is inherently dangerous because there is
a chance for the restart to fail (say, if the new configuration file is
erroneous), whereas the 'reload' action cannot fail (if it is correctly
implemented).

That being said, I agree that adding support for 'reload' would lead to
more complexity, and I would understand if you don't add it :-), but I
still think it's a very useful feature.

One question though: my understanding is that the default value for
'restart-strategy' is set in the Guix repository, but a user would be
able to customize it in their config.scm.  Can you confirm it?

Thank you,
Clément

  reply	other threads:[~2018-11-26 21:03 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-26 11:41 [bug#33508] [PATCH] gnu: Add ability to restart services on system reconfigure Carlo Zancanaro
2018-11-26 12:42 ` Clément Lassieur
2018-11-26 20:11   ` Carlo Zancanaro
2018-11-26 21:02     ` Clément Lassieur [this message]
2018-11-26 21:59       ` Carlo Zancanaro
2018-11-30 12:12         ` Clément Lassieur
2018-12-01  2:31           ` Carlo Zancanaro
2018-12-13 14:22             ` Clément Lassieur
2018-12-09 16:59 ` Ludovic Courtès
2019-01-01 11:25   ` Carlo Zancanaro
2019-01-05 14:00     ` Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k1kzd02u.fsf@lassieur.org \
    --to=clement@lassieur.org \
    --cc=33508@debbugs.gnu.org \
    --cc=carlo@zancanaro.id.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.