Shepherd doesn't restart previously running dependent services

  • Done
  • quality assurance status badge
Details
4 participants
  • Carlo Zancanaro
  • Clément Lassieur
  • Thompson, David
  • Ludovic Courtès
Owner
unassigned
Submitted by
Thompson, David
Severity
normal
T
T
Thompson, David wrote on 31 Mar 2016 15:23
(address . bug-guix@gnu.org)
CAJ=RwfZdqc6iqsRsay95oPZDd-hrqkaERYA5PYiXp_FwOO1-Qg@mail.gmail.com
If service 'foo' is depended on by service 'bar', and both are
running, and one runs 'herd restart foo', both 'foo' and 'bar' are
stopped, but then only 'foo' is restarted. I think that the dependent
services that were stopped should also be restarted, otherwise the
user has to manually start them back up one at a time.

- Dave
C
C
Carlo Zancanaro wrote on 25 Aug 2018 13:33
[PATCH shepherd] Restart dependent services on service restart
(address . 23170@debbugs.gnu.org)
874lfi65rv.fsf@zancanaro.id.au
I've written a patch to fix this. It's not super smart, but it
should do the job.

It currently targets the branch after my patch in #32408[1], but
it's technically an independent change.

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE1lpncq7JnOkt+LaeqdyPv9awIbwFAluBPpsACgkQqdyPv9aw
IbyeQw/7B/16oHu0N3vvfGPx1TJWfByvfjnU9jlo8p0psZqR8/fbGpqax2+/VdEP
zoL4XSUZMYTAeSOqFsFshHl9oyTnMInHN3+XyDCKRg8UiOj8FjQkb74ppxGdjTbj
mosb2gMl5I5cIRauH3aAq64DTeZx9PZRzN0OXXAGK0B8RrKUVu8Z0AC9q9kMZ4V+
Jp5d0hOG20WwV66thte/ctzhC7xF2/WgRzG/fwxnF2B3UEL2VdKanBwNq8rLBcvV
y4tW1OCWUCrdIZZWHxf6EoXckjiEgQBgUkmCOmqvur+AkD/eGtFzwMkrP0juJoMz
NiMuOwhj+SYLrhgaHcnwWRnOxiap/jzY6MDoxhbw0WFtitCHhm5Mz9oFJdw6vJht
fsrmcC2X4MfzLxRW4GwC3UqbGVKDC9SllFGRJVjwEQh0emShLXYGv88fVDGF3zXX
nmuQe1RHJYruNVjbzqWA+3KyBSHBPild/47dLmiFc6e+1VMYBpTUKdExOMXikEAp
3uZSATATO0Ziqcw+jzROo3IHwqhz0QsBYYjFWgV/cDdeHJn0r7BAkqWIi2j7cNSQ
Y2rQB41ZV7XlFMFg2RwSwXr/8hpMnrXsl0JlYKCaSwA+CMDSGBkDrYPU400Gn7c5
J1ghS+XPWYs/dVO4ssK9zpy4hl/c9dINjd3a/d6yBMFb9xspSD4=
=z5ll
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 25 Aug 2018 16:41
(name . Carlo Zancanaro)(address . carlo@zancanaro.id.au)(address . 23170@debbugs.gnu.org)
87bm9qbjb4.fsf@gnu.org
Hi again! :-)

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

Toggle quote (3 lines)
> I've written a patch to fix this. It's not super smart, but it should
> do the job.

I wonder if there are cases where one might want to restart a service
without restarting its dependent services. We can probably ignore it
for now, but perhaps we’ll need to add a flag or a separate action later.

Thoughts?

Toggle quote (13 lines)
> From 50dd3ef4888b04ea3b869da893b23ad69fad8971 Mon Sep 17 00:00:00 2001
> From: Carlo Zancanaro <carlo@zancanaro.id.au>
> Date: Sat, 25 Aug 2018 20:32:11 +1000
> Subject: [PATCH] service: Restart dependent services on service restart
>
> * modules/shepherd/service.scm (required-by?): New procedure.
> (stop): Return a list of canonical-names for stopped dependent services,
> including transitive dependencies.
> (action)[restart]: Start services based on the return value of stop.
> (fold-services): New procedure.
> * tests/restart.sh: New file.
> * Makefile.am (TESTS): Add tests/restart.sh.

[...]

Toggle quote (16 lines)
> +# Restart test1 and make sure that both services are still running (ie. that
> +# test2 hasn't been stopped)
> +$herd restart test1
> +$herd status test1 | grep started
> +$herd status test2 | grep started
> +
> +# Now let's test with a transitive dependency
> +$herd start test3
> +$herd status test3 | grep started
> +
> +# After restarting test1 we want test3 to still be running
> +$herd restart test1
> +$herd status test1 | grep started
> +$herd status test2 | grep started
> +$herd status test3 | grep started

For clarity, should we do an explicit “herd stop test1” followed by
“herd start test1”? I know it’s currently equivalent under the hood,
but it might be slightly clearer. WDYT?

Otherwise it LGTM. Thanks for addressing these longstanding issues!

Ludo’.
C
C
Carlo Zancanaro wrote on 26 Aug 2018 00:48
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 23170@debbugs.gnu.org)
871samqcz4.fsf@zancanaro.id.au
On Sun, Aug 26 2018, Ludovic Courtès wrote:
Toggle quote (7 lines)
> I wonder if there are cases where one might want to restart a
> service without restarting its dependent services. We can
> probably ignore it for now, but perhaps we’ll need to add a flag
> or a separate action later.
>
> Thoughts?

I think this is best served by 'herd stop', followed by 'herd
start'. This patch just special-cases the 'restart' action, so
manually stopping then starting a service will behave as the old
restart used to.

Toggle quote (4 lines)
> For clarity, should we do an explicit “herd stop test1” followed
> by “herd start test1”? I know it’s currently equivalent under
> the hood, but it might be slightly clearer. WDYT?

Hopefully the above also answers this, too. I did consider whether
it was worth adding a test for 'herd stop' to make sure it still
stops dependent services, and 'herd start' to make sure it doesn't
start dependent services, but in the end I decided not to. I'm
happy to send through another patch to test these cases, though,
if you think it would be worthwhile.

Carlo
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE1lpncq7JnOkt+LaeqdyPv9awIbwFAluB3M8ACgkQqdyPv9aw
IbzlkRAAuuV6Uvb+pDrPisIMqTsunJKVOkurgKSSw7mMe2QCurxTGENCkohtniNu
LSXR7M8J3TsmJgUxC3go4vc2koKDzBQj00mnFveRXI3LG4ZO3dPVLwtEt8OjxzRW
oRBMqb1jSn1qdsW/sEo5qZ7cJcSmJb1lChvzQEg3IOgRmm+7VWNZbfWCp9BmThFp
nBzTwZ7O/9wekWZuxaQ8xuWg3kFVf9CK2HsNs5gT2/QLqp9xC2mhlh6rx7pRmsBl
xT8qK5rF1V/FgVcdAufwNN7bx58pURpv11VTTySQNXytszoXUp/khOg7UJpLKnHA
bkkySPlnvIP3OHQlUuuR3TKpF3gs8AobJFqJmEPDGwcxDtIoZza4AOc7I2mfMo5a
jKXaBLsmSY79Eou0ySF3LU/wTyJmg2tcv8khTGIcLMeOGEW7wm6mI0v0iReW6VVo
UA5Ah8NI46WsXF1kT/lGnglrHTkmaiUqyhFkihxxiBlud6Q4Hq+ZZVKUBQvRbpZh
0p7MOPBlxRWDJ50oDxCwuxF7SZKMbfVzTsj1fH9nM1RSMWohOXVchQ7tlU2Nayhh
U1cqhbnEcwozAiSX0cdVPvO71SZ4i2mGoKuzDFmXVhZhDt2thypzuu2m6qckRBtK
JpDyXIaOGZaRf3VGyIT8pGGsvA8oB8LwF8ofnTHmAKXUW7tOh5E=
=jJMT
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 26 Aug 2018 23:08
(name . Carlo Zancanaro)(address . carlo@zancanaro.id.au)(address . 23170@debbugs.gnu.org)
87ftz07s4t.fsf@gnu.org
Carlo Zancanaro <carlo@zancanaro.id.au> skribis:

Toggle quote (13 lines)
> On Sun, Aug 26 2018, Ludovic Courtès wrote:
>> I wonder if there are cases where one might want to restart a
>> service without restarting its dependent services. We can probably
>> ignore it for now, but perhaps we’ll need to add a flag or a
>> separate action later.
>>
>> Thoughts?
>
> I think this is best served by 'herd stop', followed by 'herd
> start'. This patch just special-cases the 'restart' action, so
> manually stopping then starting a service will behave as the old
> restart used to.

Great, I had overlooked this.

Toggle quote (6 lines)
>> For clarity, should we do an explicit “herd stop test1” followed by
>> “herd start test1”? I know it’s currently equivalent under the
>> hood, but it might be slightly clearer. WDYT?
>
> Hopefully the above also answers this, too.

It does, thanks!

Toggle quote (6 lines)
> I did consider whether it was worth adding a test for 'herd stop' to
> make sure it still stops dependent services, and 'herd start' to make
> sure it doesn't start dependent services, but in the end I decided not
> to. I'm happy to send through another patch to test these cases,
> though, if you think it would be worthwhile.

No, that’s fine.

I forgot if this was already done, but perhaps you can add a bit in the
manual to insist that ‘restart’ is not quite the same as ‘stop’ +
‘start’.

Anyway, it all LGTM, thanks!

Ludo’.
C
C
Carlo Zancanaro wrote on 27 Aug 2018 00:05
(address . ludo@gnu.org)(address . 23170@debbugs.gnu.org)
7A4A9C8A-E937-44A9-B941-B8263B00093A@zancanaro.id.au
Hey Ludo’!


On 27 August 2018 7:08:34 am AEST, ludo@gnu.org wrote:
Toggle quote (2 lines)
>I forgot if this was already done, but perhaps you can add a bit in the manual to insist that ‘restart’ is not quite the same as ‘stop’ + ‘start’.

I hadn't done that, but I have now. There aren't many mentions of restart in the manual, but I changed the one that seemed most relevant.

Toggle quote (2 lines)
>Anyway, it all LGTM, thanks!

Pushed! Thanks for the review.

Carlo
Attachment: file
C
C
Carlo Zancanaro wrote on 27 Aug 2018 00:05
(address . ludo@gnu.org)(address . 23170@debbugs.gnu.org)
BC9EC36A-3916-4DB8-B70F-785B692BC4A4@zancanaro.id.au
Hey Ludo’!


On 27 August 2018 7:08:34 am AEST, ludo@gnu.org wrote:
Toggle quote (2 lines)
>I forgot if this was already done, but perhaps you can add a bit in the manual to insist that ‘restart’ is not quite the same as ‘stop’ + ‘start’.

I hadn't done that, but I have now. There aren't many mentions of restart in the manual, but I changed the one that seemed most relevant.

Toggle quote (2 lines)
>Anyway, it all LGTM, thanks!

Pushed! Thanks for the review.

Carlo
L
L
Ludovic Courtès wrote on 27 Aug 2018 13:05
(name . Carlo Zancanaro)(address . carlo@zancanaro.id.au)(address . 23170@debbugs.gnu.org)
87o9docboc.fsf@gnu.org
Hi,

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

Toggle quote (2 lines)
> Pushed! Thanks for the review.

Great!

I see that you also reverted the patch that removed the ‘EINTR-safe’
workaround. Could you explain why that was necessary? (It should not
be necessary with current Guile versions.)

Thank you,
Ludo’.
C
C
Carlo Zancanaro wrote on 27 Aug 2018 14:42
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 23170@debbugs.gnu.org)
87mut8dlq7.fsf@zancanaro.id.au
On Mon, Aug 27 2018, Ludovic Courtès wrote:
Toggle quote (5 lines)
> I see that you also reverted the patch that removed the
> ‘EINTR-safe’ workaround. Could you explain why that was
> necessary? (It should not be necessary with current Guile
> versions.)

I'm not really sure of the details, but as I mentioned on IRC,
that commit stopped me from being able to boot. I grafted a new
version of the Shepherd and reconfigured my system, and when it
came up my screen was filled with messages complaining that it
couldn't create things in /dev because they already existed. I
tested the commits since the previous release and that was the one
that caused my problem.

I'd like to investigate further, but I have no idea what could be
causing it. All I have to go on so far is that I think the udev
service is getting respawned repeatedly, but I don't know why that
commit would cause that problem. After reverting that commit I
could successfully boot back into my system and everything is
working properly again.
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE1lpncq7JnOkt+LaeqdyPv9awIbwFAluD8cEACgkQqdyPv9aw
IbzA0RAAjkqR5kU3kf4wTEwk3NsdWOQQxZ37efl4Q5Jm0Z7Uygxj17mYH1paAC84
y9jVNVnOOMqdJz3OsfYyw73KyGo95dQJy9+bUNt3T4ruvVLSnZyNkmSSfQBlme0c
KSar7fvf2wr4oLMHr3q/4YEL0OT48yOFajHI6zDw7PaepFir6xfRK/S6M45x+1FV
Q9byDx+6siWwyH602cqj7nSl8wnhoZia8grbcV9zCz2bWCnR6OAtv1M12IQoKF63
E8xn25s3qdPBcN1p8QpiA0rXdyOZL0L3pFdHELtkWWV4MTxhPyC78J/Yeiz6KQJg
GMTTZ0vb/ikyFizo5yE1fUByKjIAVXYTefAMj/rLiz5uOW96y4UeDxGi4aTBWvyu
C/Jo14UuZr83t9q6qVqR5IKK0qZssN4GH+MrzdMeEj+Pl7rONrq46BFGMerT6lgY
hQdHabZkp934FGjQajfCqHjeMEcHFW8vNSgpIfUhtecVdq0ZN8cYDHZnLK8b2fU/
OBfw2uM8di7384MopGyH7iY5b3XftkaLm3JQArl3FaNkynsRwhoqLFE13gshwe8J
2URC21TO1eR0GBmTntgYg82BNNN9Lz6mPg2FbRKzHfXLKfJFXMhxjc1wY+Z/PpBH
0JVRuqETm5wIcMAhmQo0cZEKvZHc4nqdjOBGSK5RpOositHyZy8=
=O6mm
-----END PGP SIGNATURE-----

C
C
Clément Lassieur wrote on 27 Aug 2018 19:53
(name . Carlo Zancanaro)(address . carlo@zancanaro.id.au)
8736uzbssi.fsf@lassieur.org
Carlo Zancanaro <carlo@zancanaro.id.au> writes:

Toggle quote (6 lines)
>>Anyway, it all LGTM, thanks!
>
> Pushed! Thanks for the review.
>
> Carlo

Awesome! Thanks a lot Carlo for working on this :-)
L
L
Ludovic Courtès wrote on 5 Jan 2019 14:53
control message for bug #23170
(address . control@debbugs.gnu.org)
87y37z5hyn.fsf@gnu.org
tags 23170 fixed
close 23170
?
Your comment

This issue is archived.

To comment on this conversation send an email to 23170@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 23170
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch