[systemd-devel] Deadlocks with reloading jobs which are part of current transaction [was: [PATCH] Avoid reloading services when shutting down]

[Lennart Poettering]

On Wed, 04.02.15 23:48, Uoti Urpala (uoti.urpala at pp1.inet.fi) wrote:

Sorry for the late reply,

> On Wed, 2015-02-04 at 21:57 +0100, Lennart Poettering wrote:
> > OK, let's try this again, with an example:
> > 
> > a) you have one service mydaemon.service
> > 
> > b) you have a preparation service called
> >    mydaemon-convert-config.service that takes config from somewhere,
> >    converts it into a suitable format for mydaemon.service's binary
> > 
> > Now, you change that config that is located somewhere, issue a restart
> > request for m-c-c.s, issue a reload request for mydaemon.service.
> > 
> > Now, something like this should always have the result that your
> > config change is applied to mydaemon.service. Regardless if
> > mydaemon.service's start was queued, is already started or is
> > currently being started. You are suggesting that the reload can
> > suppressed when a start is already enqueued, but that's really not the
> > case, because you first have to run m-c-c.s, before you can reload...
> 
> I do not see why that would cause any problems with removing the
> blocking.
> 
> If you mean literally running "systemctl restart
> mydaemon-convert-config.service; systemctl reload mydaemon.service" then
> this should still work fine - the first "restart" will block until the
> operation is complete and new config exists, and then the "reload"
> guarantees that no old config is in use. 

No, the commands you suggest above don't just enqueue the operations,
they enqueue them and then wait for them to finish. Of course, if you
synchronously wait for them to finish then all races are gone, but
this really should work without that, so that things can be enqueued
and work correctly.

> However, I don't see why you'd
> include the part about creating the new configuration via
> mydaemon-convert-config.service in this case - the new configuration
> already exists before any "reload" functionality is invoked, so why the
> irrelevant complication of creating that configuration via another
> service? It seems you are implicitly assuming some kind of parallel
> execution of the restart and the reload?

Well, this is an example of the way people do this, and yes, i was
talking about "enqueuing", and that really means just that: it won't
be the client anymore that controls execution order, but it is solely
the queue and its semantics that enforce how things are run and what
guarantees are made.

> If you mean something like "systemctl restart --no-block
> mydaemon-convert-config.service; systemctl reload mydaemon.service", I
> don't see why you'd ever /expect/ this to work with any reload semantics
> - isn't this clear user error, and will be racy with current systemd
> code just as much as the proposed fix? 

Yupp, this is what I mean. (though I'd actually specify the --no-block
in the second command too, though this doesn't make much of a
difference...)

I am pretty sure that enqueueing these two commands should be sufficient
to get the config that was written out by the first service to be in
effect in the second service.

> There are no ordering constraints
> here, any more than there would be about starting two services and
> expecting the first request to be started first. 

hmm? if you start two services, and they are ordered against each
other, then yes, the second service should only be started after the
first one completed startup.

> And in any case I'd consider the semantics of reload to be "switch
> to configuration equal or newer than what existed *when the reload
> was requested*", without any guarantees that changes from operations
> queued but not finished before calling reload would be taken into
> account.

The queue is really a work queue, and the After= and Before= deps
dictate how the work can be parallelized or needs to be serialized. As
such if i have 5 jobs enqueued that depend on each other, i need to
make sure they are executed in the order specified and can operateon
the results of the previous job.

I hope this makes sense...

Lennart

-- 
Lennart Poettering, Red Hat