Comment 61 for bug 1874075

Revision history for this message
Dan Streetman (ddstreet) wrote :

@mruffell thanks! Only a few comments below:

> Note because of this bug, groovy and upstream has now been changed to 10 min timeout, down from 1hr.

We should decide if this is really what we want to do. And if it should revert to the longer 1hr timeout, propose that upstream.

I don't really know, is either default timeout better, 10 minutes or 1 hour? @nicolasbock did you have specific reasoning for the upstream reduction to 10 minutes?

If 10 minutes is what we want, then we should be ok upstream and in Groovy, and in Focal with the current code in -proposed.

> On Eoan: Assuming same behaviour as focal due to systemd service file. Untested.

yeah, it FTBFS in Eoan unfortunately; there is bug 1843761, and also I detailed why it fails in the description for bug 1773324. As Eoan is almost EOL, my opinion is it's safer to simply leave it untouched there.

> If this ExecStartPost script times out (which it does after 90 seconds it seems, even though documentation suggests infinite timeout)

yep, systemd has DefaultTimeoutStartSec set to 90s (man systemd-system.conf for more details), so if TimeoutStartSec isn't specified for a service unit, it will default to 90 seconds (and I believe the timeout period includes the ExecStartPre, ExecStart, and ExecStartPost actions, but I'd have to specifically check the code to verify that).

> we need to add a dependency to the package, socat

well, this is usually a problem for SRU releases. Unfortunately, adding new deps for SRU releases causes 'sudo apt-get upgrade' to *not* upgrade any package that pulls in new (not currently installed) deps. While 'sudo apt upgrade' *does* pull in new deps, the ~ubuntu-sru team typically rejects adding new runtime deps to any SRU, without a very strong reason.

Instead of pulling the entire service file back into Bionic, I think it might be enough to only add 'TimeoutStartSec=600', which should cover the timeout for the ExecStart= and ExecStartPost= actions. It may be also worth adding the Restart=on-failure and RestartSec=10 params. Could you test with the TimeoutStartSec param in bionic to see if that's enough to SRU?

If pulling back only the TimeoutStartSec=600 param to Bionic works, that will hopefully be enough for Xenial, too.

Thanks!