Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup Systemd unit file #301

Merged
merged 2 commits into from
Dec 13, 2016

Conversation

amiryal
Copy link
Contributor

@amiryal amiryal commented Dec 13, 2016

Prompted by @solarkennedy’s reply, here is how I think the Systemd unit file should look.

One thing that is not addressed in the very detailed commit message is the request for “some good data on what [the value of RestartSec] should be”. My reasoning for removing this option and keeping the default is as follows:

  1. My intuition tells me that we want to respawn Consul immediately on failure, without any delay. This would have meant setting RestartSec=0ms.

  2. The creators of Systemd, and Systemd itself by extension, are notorious for being very opinionated. If they deemed that the default should be 100ms and not 0ms, and given that I don’t have data to support my preference, I will rather trust their judgement.

The full commit message follows:

Remove references to basic.target: Systemd automatically adds
dependencies of the types Requires= and After= for this target unit to
all services, so KISS and DRY.

Remove KillMode= option: Consul itself runs entirely in one process, so
the default of killing all processes in the control group would not make
a difference. If Consul does spawn processes, e.g. for health checks,
the previous setting would have left them running behind when the
service is stopped.

Remove RestartSec= option: There is no reason to override the default of
100ms.

Remove references to basic.target: Systemd automatically adds
dependencies of the types Requires= and After= for this target unit to
all services, so KISS and DRY.

Remove KillMode= option: Consul itself runs entirely in one process, so
the default of killing all processes in the control group would not make
a difference. If Consul does spawn processes, e.g. for health checks,
the previous setting would have left them running behind when the
service is stopped.

Remove RestartSec= option: There is no reason to override the default of
100ms.
@solarkennedy solarkennedy merged commit 132a80c into voxpupuli:master Dec 13, 2016
spuder pushed a commit to spuder/puppet-consul that referenced this pull request Feb 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants