Re: A Critique of Shepherd Design

[raid5atemyhomework]

Sent with ProtonMail Secure Email.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Tuesday, March 23, 2021 1:02 AM, Ludovic Courtès <ludo@gnu.org> wrote:

> Hi,
>
> raid5atemyhomework raid5atemyhomework@protonmail.com skribis:
>
> > I'm not sure you can afford to keep it simple.
>
> It has limitations but it does the job—just like many other init systems
> did the job before the advent of systemd.
>
> > Consider: https://issues.guix.gnu.org/47253
> > In that issue, the `networking` provision comes up potentially before the 
> > network is, in fact, up. This means that other daemons that require 
> > `networking` could potentially be started before the network connection is 
> > up.
>
> The ‘networking’ service is just here to say that some network interface
> is up or will be up. It’s admittedly vague and weak, but it’s enough
> for most daemons; they just need to be able to bind and listen to some
> port.
>
> > One example of such a daemon is `transmission-daemon`. This daemon will 
> > bind itself to port 9091 so you can control it. Unfortunately, if it gets 
> > started while network is down, it will be unable to bind to 9091 (so you 
> > can't control it) but still keep running. On my system that means that on 
> > reboot I have to manually `sudo herd restart trannsmission-daemon`.
>
> Could you report a bug for this one? I don’t see why it’d fail to bind.

Let me see if I can still get to the old syslogs where `transmission-daemon` 
claims it cannot bind, but it still keeps going anyway.  I've pulled my 
`transmission-daemon` directly into my `configuration.scm` so I can edit its 
`requirement` to include a custom `networking-online` service that does 
`nm-online`.


> > In another example, I have a custom daemon that I have set up to use
> > the Tor proxy over 127.0.0.1:9050. It requires both `networking` and
> > `tor`. When it starts after `networking` comes up but before the
> > actual network does, it dies because it can't access the proxy at
> > 127.0.0.1:9050 (apparently NetworkManager handles loopback as well).
>
> Loopback is handled by the ‘loopback’ shepherd service, which is
> provided via ‘%base-services’. Perhaps you just need to have your
> service depend on it?
>
> > Switching to a concurrent design for Shepherd --- any concurrent design --- 
> > is probably best done sooner rather than later, because it risks strongly 
> > affecting customized `configuration.scm`s like mine that have almost a half 
> > dozen custom Shepherd daemons.
>
> I suspect the main issue here is undeclared dependencies of some of the
> Shepherd services you mention.
>
> I like the “sooner rather than later” bit, though: it sounds like you’re
> about to send patches or announce some sponsorship program?… :-)

Not particularly, but I *have* looked over Shepherd and here are some notes.  
Maybe I'll even send patches, but the reaction to the ZFS patches makes me just 
shrug; I'd need to devote more time than what I spent on ZFS, and the ZFS 
patches aren't getting into Guix, so why bother.  If I get annoyed enough I'll 
just patch my own system and call it a day.  My own system has `nm-online` and 
I don't expect to not have networking, so the `nm-online` delay is unlikely to 
be an issue, and I don't intend to mess with the `configuration.scm` anymore 
because it's just too brittle, I'll just host VMs instead and use a SystemD 
fully-free OS like Trisquel, I only need Guix for the ZFS (which Trisquel does 
not have, for some reason).

Anyway...

It seems to me that a good design to use would be that each `<service>` should 
have its own process.  Then the big loop in `modules/shepherd.scm` will then be 
"just" a completely event-based command dispatcher that forwards commands to 
the correct per-`<service>` process.

Now, one feature Shepherd has is that it can be set with `--socket-file=-`, 
which, if specified, causes GNU Shepherd to enable GNU readline and use the 
readline library to read `-herd`-like (?) commands.

Unfortunately the `readline` interface is inherently blocking instead of 
event-based.  The C interface of GNU readline has an alternative interface that 
is compatible with event-based (and I've used this in the past to create a toy 
chat program that would display messages from other users while you were typing 
your own) but it looks like this interface is not exposed.  I checked 
`readline-port` as well, but the code I could find online suggests that this 
just uses the blocking `readline` interface, and would (?) be incompatible with 
the Guile `select`. (side note: the `SIGCHLD` problem could probably be fixed 
if Guile had `pselect` exposed, but apparently it's not exposed and I'm not 
prepared to dedicate even more time fiddling with the lack of syscalls in 
Guile.  Maybe a signal-via-pipe technique would work as an alternative though 
since that supposedly works on every UNIX-like --- but presumably the Shepherd 
authors already knew that, so maybe there is good reason not to use it).

Since `readline` is blocking, one possibility would be to *fork off* the 
process that does the stdin communication.  Then it can continue to use the 
blocking `readline`.  It just needs to invoke `herd stop root`when it gets EOF 
(note: need to check how commands are sent, for now it looks to me (not 
verified, just impression) that commands are sent as plain text).

Since the goal is to make the mainloop into a very parallel dispatcher, we need 
some way to somehow send commands in stdin-mode.  We can take advantage of the 
little-known fact that UNIX domain sockets can pass file descriptors across 
processes, with the file descriptor number being remapped to the receiving 
process via magic kernel stuff.  So, we create a `socketpair` (NOTE: CHECK IF 
GUILE HAS `socketpair`!!!  Also review how the fd-passing gets done, maybe 
Guile doesn't expose the needed syscalls either, sigh), then each time the 
`readline`-process receives a command, it creates a new `socketpair`, sends 
over one end to the mainloop, sends the command via the other end, then  waits 
for a response and prints it.  This should make it very near to in experience 
as the blocking Shepherd.

If the above pattern is workable, ***we can use the same pattern for 
`--socket-file=/file/path`***.  We ***always*** fork off *some* process to 
handle `--socket-file`, whether `stdin`-mode or not.  In 
`--socket-file=/file/path` mode, the `socket-file` process binds the socket 
file, `listen`s on it on a loop, and then just passes the opened socket over to 
the mainloop.

We also need this pattern as a general feature of the mainloop.  An action on 
one `<service>` can trigger actions on another service (in theory; my cursory 
review of the Guix code suggests that current services only trigger actions on 
themselves (`ganeti`, `guix-daemon`; but this is not a full review and there 
may be other services in Guix that do other stuff)); note in particular that 
`start` causes every `requirement` to be started anyway.  So I think we need a 
general mechanism for the mainloop to receive commands from remote processes; 
we might as well use the same mechanism for both the Shepherd<->user 
interaction and the Shepherd<->service interaction.

So for clarity of exposition, let me then list down the processes created by 
Shepherd:

* The `mainloop` process which handles the main massively-parallel event loop.  
This is PID 1.
* The `socket-file` process which either gets commands from `stdin`, or via the 
`socket-file`.
* Multiple per-`<service>` process.

Now, the mainloop has to parse the command in order to learn which 
per-`<service>` process the command should get forwarded to.  And as mentioned, 
each per-`<service>` process also needs a command-sending socket to go to the 
mainloop.  So for each per-`<service>` process:

* The mainloop maintains a mainloop->service socket to send commands over.
* The mainloop maintains a service->mainloop socket it receives command 
socketfds over.

The mainloop process also special-cases the `root` service --- it handles 
commands to those directly (BUT this probably needs a lot of fiddling with the 
data structures involved --- `herd status root` can now occur while a `herd 
start <whatever>` is still running, so we need status reporting for "being 
started up" and "being stopped" as well --- `/` for "starting up", `\` for 
"stopping"?).

Now, the `action` and other procedures need to be special-cased.  We need a 
global variable indicating:

* The current process is `root` ie the mainloop process.
* The current process is some `<service>`.

Every `action` is different depending on this variable (`%process-identity`?).

* IF the action is going to the same `<service>` (including `root`):
  * Just tail-call into the action code.
* If the current process is `root` and a non-`root` action is being performed:
  * Check if the per-`<service>` process has been started, and start if needed.
  * Schedule the command to be sent via the event loop.
  * Keep operating the mainloop until the command has completed.
    * Use an event-loop stepper function (i.e. just calls `select` and 
dispatches appropriately, then returns, so caller has to implement the loop).
    * Initially set a mutable variable to `#f.
    * Schedule the command with a callback that sets the above variable to `#t`.
    * Call the event-loop stepper function until the mutable variable is true.
    * This implements the current semantics where a `.conf` file running an 
action will block until the action finishes, while still allowing commands to 
be sent to the Shepherd daemon.
* If the current process is not `root` and the action to be performed is of a 
different process:
  * Create a socketpair to send the command over to the mainloop and send it 
(blocking).
  * Send the command to the mainloop (blocking).
  * Wait for completion (blocking).

Each per-`<service>` process has a simple blocking loop: It waits for commands 
from the mainloop process, executes those commands, then loops again.

In particular, it means that any `start` actions in the `.conf` file will block 
(which is the expected behavior of legacy `.conf` files, but even so, the 
Shepherd will be able to handle commands even while it is still loading `.conf`.


=== Concurrency is Like Regexps, Now You Have Two Problems ===

But take not that this means that it is possible to deadlock services in this 
way:

* There are two services `A` and `B`.
  * `A` has an action `AtoB` which invokes an action `BtoA` on service `B`.
  * The `B` `BtoA` action invokes an action `Anoop` on service `A`.
    * In the above structure, because the `A` per-`<service>` process is 
waiting on the `BtoA` action on service `B`, it cannot handle the `Anoop` 
action!
    * In the current single-threaded Shepherd, such a thing is actually 
possible and would not cause a deadlock if the `A` `Anoop` terminates normally.

HOWEVER, this is probably a very unusual setup!  It may be tolerable to simply 
require that a service that performs actions on another service should have an 
acyclic relationship (it is Turing-complete --- consider the case where the `B` 
`BtoA` action reinvokes the `A` `AtoB` with the same arguments causing it to 
invoke `B` `BtoA` action again ad infinitum --- whereas an acyclic relationship 
requirement would provably terminate; it may even be possible, by passing a 
"stack" (i.e. list of service names that caused a particular action of a 
particular service to be invoked) when passing commands across, with the `root` 
service always passing an empty stack, and each `<service>`-to-`<service>` 
action prepends the name of the calling service, so the mainloop can detect a 
dynamic cycle and just fail the command without forwarding).

IF this restriction is too onerous, then it may be possible to use an event 
loop in the per-`<service>` process as well, and use the same 
wait-for-events-while-blocking logic as on the mainloop --- the code might even 
be reusable.  BUT I worry about this bit, as it could potentially mean that an 
action is invoked in the dynamic context (including fluids, sigh) of another 
on-going completely-unrelated action, which is BAAAAAD.  This is fine for the 
`root` service since the `root` service is Shepherd itself (? I think) and we 
can ensure that the Shepherd code does not depend on dynamic context.


Another is that in the current single-threaded Shepherd, any service's action 
can (re-)register a new `<service>`.  This is problematic since a 
per-`<service>` process will obviously not affect the mainloop process 
post-fork.

Again this is probably a very unusual setup.  While such a thing would be cute, 
I think it would be tolerable to simply require that only the mainloop process 
(which is what loads the `.conf` file) is the only one that is allowed to 
(re-)register `<service>`s.


=== Taking Advantage of Concurrency ===

Now that each `<service>` gets its own process, we can add a new 
`force-destroy-service` action on `root`.

    herd force-destroy-service root <some-service>

This forces the per-`<service>` process, and that of every dependent 
`<service>`, as well as all processes in the process trees of tho affected 
per-`<service>` processes, to be force-killed (IMPORTANT: check how to get the 
process tree, also look into "process groups", maybe that would work better, 
but does it exist on Hurd?).

This new command helps mitigate the issue where Shepherd `start` actions are 
Turing-complete and potentially contain infinite-loop bugs.  If this happens, 
the sysad can `herd force-destroy-service root <some-service>` the misbehaving 
service, reconfigure the system to correct the issue, and restart.


Another issue is that the current Guix startup is like this (paraphrased):

    (for-each (lambda (service) (start service)) '(guix-daemon #;...))

Now consider this in the context of the `nm-online` example above.  The reason 
why Guix+Shepherd cannot just ***always*** use `nm-online` like SystemD does is 
because `nm-online` can delay boot for an arbitrary number of seconds.  And 
since `start` is a known-blocking legacy interface, it should remain a 
known-blocking interface (back compatibility is a bitch).

This means that for the Guix startup, we should expose a new Shepherd API:

    (start-multiple '(guix-daemon #;...))

This basically does this:

* Set a mutable variable to the length of the input list.
* For each service, if the per-`<service>` process isn't started yet, start it.
* For each service, schedule to `start` the service; have the callback 
decrement the above variable (maybe also set another variable to a list of 
services that fail to start).
* In a loop:
  * If the above mutable variable is zero, exit.
  * Otherwise, call the mainloop stepper function, then loop again.

This allows multiple `start` to be executed at once (which could trigger a 
`start` again of their requirements, but if a service is already started then 
its `start` action should do nothing) in parallel.  There may be a thundering 
herd effect on the mainloop though because of hammering `start` on the 
requirements.  Hmm.  Concurrency is hard.

Then Guix also has to be modified to use `start-multiple` instead.

This further step ensures as well that infloop problems in custom service 
definitions do not delay startup --- startup is fully parallel (modulo 
thundering herds of daemons).


=== Overall ===

It seems to me that with the above redesign, there would be very little code 
left of the original Shepherd, making this more of a reimplementation than a 
patch or even a fork.


Thanks
raid5atemyhomework