From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58545) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gROvJ-0008SO-CD for guix-patches@gnu.org; Mon, 26 Nov 2018 17:00:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gROvE-0003Cr-CH for guix-patches@gnu.org; Mon, 26 Nov 2018 17:00:09 -0500 Received: from debbugs.gnu.org ([208.118.235.43]:45958) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gROvD-0003CV-59 for guix-patches@gnu.org; Mon, 26 Nov 2018 17:00:04 -0500 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1gROvC-00056b-RW for guix-patches@gnu.org; Mon, 26 Nov 2018 17:00:02 -0500 Subject: [bug#33508] [PATCH] gnu: Add ability to restart services on system reconfigure Resent-Message-ID: References: <87efb8m5gy.fsf@zancanaro.id.au> <871s78ypr9.fsf@lassieur.org> <875zwj8uqz.fsf@zancanaro.id.au> <87k1kzd02u.fsf@lassieur.org> From: Carlo Zancanaro In-reply-to: <87k1kzd02u.fsf@lassieur.org> Date: Tue, 27 Nov 2018 08:59:28 +1100 Message-ID: <87lg5fsdof.fsf@zancanaro.id.au> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: quoted-printable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: =?UTF-8?Q?Cl=C3=A9ment?= Lassieur Cc: 33508@debbugs.gnu.org Hey Cl=C3=A9ment, Thanks for your thoughts! I think you're right that the approach=20 I've implemented isn't flexible enough. I potentially haven't=20 thought through the failure cases enough. I was more thinking of=20 reload as providing "zero downtime" upgrades, rather than=20 providing a safer way to upgrade. I'll respond more specifically inline. On Tue, Nov 27 2018, Cl=C3=A9ment Lassieur wrote: > I don't think it needs to detect whether the binary has changed,=20 > because 'reload' signals are usually implemented so that they=20 > can safely fail. So, if the configuration file has changed in=20 > an incompatible way, the 'reload' action won't work, but the=20 > service will keep running. We do need to detect whether the binary has changed for the sake=20 of security updates, or similar. It would be bad if a user=20 reconfigured their system and it didn't upgrade the version of=20 nginx (or its dependencies) that they're running. Broadly speaking, I conceptualise reconfigure as "bring my system=20 into this state". Now, thus far we haven't been able to do that,=20 because we have lacked the ability to restart services properly,=20 but in my mind the ideal situation is that after running "guix=20 system reconfigure" our system is completely put into the state=20 specified by the config.scm file used. Although, now that I type that out, I notice that there is one=20 obvious way in which that is not true: the kernel. We can't=20 hot-swap the kernel, so there can always be a difference between=20 what the configuration file specifies and what the system is=20 actually running. At any rate, even if we give services the ability to reload=20 without restarting, they would need to print out a message to=20 prompt the user to manually restart them if the binary has=20 changed. I would also then expect the --restart-services flag to=20 fully restart those services, rather than just reloading them. > Your patch also leads to this inconsistency, because it allows a=20 > service to either be restarted or not, in my opinion :-) Yes, that's true, but then there's no middle-ground. Reloading is=20 "new configuration, old binary", whereas the current options are=20 "old configuration, old binary" or "new configuration, new binary"=20 (or, I guess, "not running because of a failed restart"). > I think both (1) and (2) make sense because both kind of=20 > services (the ones pointing to configuration files in the store=20 > and the ones using /etc/some-file.conf) already exist. Ideally,=20 > the mechanism should be generic enough to handle both cases. (1) actually subsumes (2), so I think I'll implement that. It=20 actually ends up being slightly easier, because the restart=20 strategy can just always be a procedure, with three predefined=20 procedures: always-restart, manually-restart, and never-restart. > That being said, I agree that adding support for 'reload' would=20 > lead to more complexity, and I would understand if you don't add=20 > it :-), but I still think it's a very useful feature. I think you've convinced me that there's value in having more=20 flexibility around restarts. I'll change the restart-strategy=20 value to be a procedure rather than a bare symbol. The downside is=20 that we'll lose the ability to statically analyse how services=20 will behave when restarting, but the increased flexibility is=20 useful to us. > One question though: my understanding is that the default value=20 > for 'restart-strategy' is set in the Guix repository, but a user=20 > would be able to customize it in their config.scm. Can you=20 > confirm it? That is not the current implementation. Individual services can=20 add a field to their configuration objects if it's meaningful for=20 them to customise their restart behaviour, but there isn't a=20 general-purpose mechanism for a user to change the restart=20 behaviour of any service (beyond the ability to write arbitrary=20 Scheme to do it). Carlo