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

Improve bitcoind systemd service file #10529

Merged
merged 1 commit into from Nov 9, 2017

Conversation

Flowdalic
Copy link
Contributor

Add comment how further options can be added or existing ones
modified. Use /run/${RuntimeDirectory} for PID file.

Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
StartLimitBurst directives as those should be set indivdually.

Remove Group to user the bitcoin user's default group.

Changed Restart from 'always' to 'on-failure' (can also be overwritten
individually).

Add comment how further options can be added or existing ones
modified. Use /run/${RuntimeDirectory} for PID file.

Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
StartLimitBurst directives as those should be set indivdually.

Remove Group to user the bitcoin user's default group.

Changed Restart from 'always' to 'on-failure' (can also be overwritten
individually).
@TheBlueMatt
Copy link
Contributor

It seems weird to move the datadir without the user noticing?

@laanwj
Copy link
Member

laanwj commented Jun 7, 2017

It seems weird to move the datadir without the user noticing?

/var/lib/bitcoind seems to make more sense too. Without specifying it it will move to somewhere under /home, which is not recommended for a system-wide daemon.

@luke-jr
Copy link
Member

luke-jr commented Jun 7, 2017

Context: bitcoind's service scripts (at least the openrc one) has always used the bitcoin user's default datadir. This systemd script was incompatible with this for some reason.

At least on Gentoo systems, the bitcoin user's home directory is itself /var/lib/bitcoin, so the datadir ends up as /var/lib/bitcoin/.bitcoin/

@Flowdalic
Copy link
Contributor Author

Besides what @luke-jr said, my rationale was that as few things as possible should be specified in the systemd service, all bitcoind specifics should go into /etc/bitcoin/bitcoin.conf. Thus the bare minium the service file has to specify how bitcoind should be invoked is

  1. -daemon
  2. -conf=
  3. -pid=

I'm not sure if it's the case, but if command arguments override bitcoin.conf settings, then the user would not be able to change datadir without fiddling with the systemd service file.

But I'm happy to re-add -datadir if it's deemed necessary.

@laanwj
Copy link
Member

laanwj commented Jun 8, 2017

What about TimeoutStartSec? #10531 increases it to 120s, this removes it completely. I'm not sure what you mean with 'should be set individually' in the OP. What is the default?

@Flowdalic
Copy link
Contributor Author

Flowdalic commented Jun 8, 2017

What about TimeoutStartSec? #10531 increases it to 120s, this removes it completely.

The default is 90s, which is sufficient for most bitcoind setups I know. If a users has a particular cause for a different value (like a very slow system), then he can simply to override it, using the systemctl edit bitocind.service approach mentioned in the added comment block. That is what I meant with "should be set individually". The idea is that systemd's default are reasonable for most cases, and if it doesn't work for someone, then systemd allows you override the defaults. I could be wrong, but I don't see a reason why bitcoind should always need TimeoutStartSec set to 120s, instead of the default value (90s).

@laanwj
Copy link
Member

laanwj commented Jun 8, 2017

What does this timeout measure in the first place? Until RPC becomes reachable? Or until it becomes usable?

@Flowdalic
Copy link
Contributor Author

What does this timeout measure in the first place?

Good question. I can't find anything in the documentation. And a short look at the source also didn't help.

Until RPC becomes reachable? Or until it becomes usable?

I doubt that, since systemd has no specific knowledge of the service. I'd guess that for 'forking' type services like bitcoind, it's the time the initial process needs to terminate.

@laanwj
Copy link
Member

laanwj commented Jun 25, 2017

So I'm not clear what to do here. This isn't gathering any (ut)ACKs and neither is #10531.
Who is using these files and has an opinion about them?

@laanwj
Copy link
Member

laanwj commented Jul 25, 2017

This seems inactive, closing for now

@laanwj laanwj closed this Jul 25, 2017
@Flowdalic
Copy link
Contributor Author

What a pity. Needless to say that I believe this PR was an improvement over what is currently in the repo. ;)

The usual consumers are people who package bitcoind for their Linux distribution. For example, the Gentoo bitcoin overlay (which I'm involved with) and the Gentoo in-tree ebuilds would probably switch to the canonical bitcoind systemd files if they where in a good shape.

@laanwj
Copy link
Member

laanwj commented Jul 25, 2017

After discussion on IRC I'm reopening this.

(Apparently the current state is completely broken, so if there can't be agreement on how to improve it, it'd be better to remove it completely.)

@laanwj laanwj reopened this Jul 25, 2017
@ryanofsky
Copy link
Contributor

What does this timeout measure in the first place? Until RPC becomes reachable? Or until it becomes usable?

This is a forking service so timeout is just checking time until original foreground process exits successfully.

Copy link
Contributor

@ryanofsky ryanofsky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK 16be7dd. This does seem like an improvement and it is good to move configuration out of the service file. If overriding datadir is desirable, seems like that could happen in the bitcoin.conf file which the service file is referencing.

After=network.target

[Service]
ExecStart=/usr/bin/bitcoind -daemon -conf=/etc/bitcoin/bitcoin.conf -pid=/run/bitcoind/bitcoind.pid
Copy link
Contributor

@ryanofsky ryanofsky Jul 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably should use /etc/bitcoind to be consistent with /run/bitcoind or /run/bitcoin to be consistent with /etc/bitcoin

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've no strong feelings towards any of those options, besides that keeping /etc/bitcoin/ would make it a bit easier for the gentoo ebuilds, as that is what they currently use.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah actually I take back the suggestion. Seems fine to stick with /etc/bitcoin since the config there could potentially could be shared with different bitcoin tools. And /var/bitcoind at least keeps consistency with the binary and service names.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, at least hypothetically, bitcoin-cli would ideally be able to use it too.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm setting up systemd do I need to create the bitcoin user and group myself?

@@ -12,7 +12,7 @@ Description=Bitcoin daemon
After=network.target

[Service]
ExecStart=/usr/bin/bitcoind -daemon -conf=/etc/bitcoin/bitcoin.conf -pid=/run/bitcoind/bitcoind.pid
ExecStart=/usr/bin/bitcoind -daemon -conf=/etc/bitcoin/bitcoin.conf -pid=/run/bitcoind/bitcoind.pid -printtoconsole -logtimestamps=0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't combine -daemon and -printtoconsole - stdout will have nowhere to go after daemonization.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea was to let systemd handle bitcoind's log output. At first using -daemon and -printtoconsole appeared to do the trick, but on a second look it sees that the log output stops after bitcoind demonized (possibly after the fork()?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea was to let systemd handle bitcoind's log output. At first using -daemon and -printtoconsole appeared to do the trick, but on a second look it sees that the log output stops after bitcoind demonized (possibly after the fork()?)

You could make work this by removing -daemon and -pid options, removing PIDFile setting, and changing Type setting from forking to simple. Personally, I would just want to use bitcoin logging, though, so logs can be more easily deleted if they contain private information, or if I enable verbose debugging and want to reclaim space. I would also leans towards using bitcoin logging here just to avoid adding unnecessary config overrides and differences between the way bitcoin functions inside and outside of systemd.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your rationale, but on the other hand there are also good reasons to let systemd handle the log output of bitcoind. But since the this change was broken anyways (and since we can decide what's best later on,) I've reverted the commit with the hope that it would make it easier to get this PR merged.

@laanwj
Copy link
Member

laanwj commented Oct 2, 2017

Please squash before merging.

@Flowdalic
Copy link
Contributor Author

Squashed.

@ryanofsky
Copy link
Contributor

utACK 16be7dd again (the later commit fac2c22ce176529893bb41a1856e2854d1db5e20 which had the -printtoconsole stuff is now gone).

@laanwj laanwj merged commit 16be7dd into bitcoin:master Nov 9, 2017
laanwj added a commit that referenced this pull request Nov 9, 2017
16be7dd Improve bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Add comment how further options can be added or existing ones
  modified. Use /run/${RuntimeDirectory} for PID file.

  Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
  StartLimitBurst directives as those should be set indivdually.

  Remove Group to user the bitcoin user's default group.

  Changed Restart from 'always' to 'on-failure' (can also be overwritten
  individually).

Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4
@Flowdalic Flowdalic deleted the systemd-service branch November 9, 2017 12:50
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Dec 22, 2019
16be7dd Improve bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Add comment how further options can be added or existing ones
  modified. Use /run/${RuntimeDirectory} for PID file.

  Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
  StartLimitBurst directives as those should be set indivdually.

  Remove Group to user the bitcoin user's default group.

  Changed Restart from 'always' to 'on-failure' (can also be overwritten
  individually).

Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jan 2, 2020
16be7dd Improve bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Add comment how further options can be added or existing ones
  modified. Use /run/${RuntimeDirectory} for PID file.

  Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
  StartLimitBurst directives as those should be set indivdually.

  Remove Group to user the bitcoin user's default group.

  Changed Restart from 'always' to 'on-failure' (can also be overwritten
  individually).

Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jan 4, 2020
16be7dd Improve bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Add comment how further options can be added or existing ones
  modified. Use /run/${RuntimeDirectory} for PID file.

  Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
  StartLimitBurst directives as those should be set indivdually.

  Remove Group to user the bitcoin user's default group.

  Changed Restart from 'always' to 'on-failure' (can also be overwritten
  individually).

Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
16be7dd Improve bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Add comment how further options can be added or existing ones
  modified. Use /run/${RuntimeDirectory} for PID file.

  Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
  StartLimitBurst directives as those should be set indivdually.

  Remove Group to user the bitcoin user's default group.

  Changed Restart from 'always' to 'on-failure' (can also be overwritten
  individually).

Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
16be7dd Improve bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Add comment how further options can be added or existing ones
  modified. Use /run/${RuntimeDirectory} for PID file.

  Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
  StartLimitBurst directives as those should be set indivdually.

  Remove Group to user the bitcoin user's default group.

  Changed Restart from 'always' to 'on-failure' (can also be overwritten
  individually).

Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
16be7dd Improve bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Add comment how further options can be added or existing ones
  modified. Use /run/${RuntimeDirectory} for PID file.

  Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
  StartLimitBurst directives as those should be set indivdually.

  Remove Group to user the bitcoin user's default group.

  Changed Restart from 'always' to 'on-failure' (can also be overwritten
  individually).

Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
16be7dd Improve bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Add comment how further options can be added or existing ones
  modified. Use /run/${RuntimeDirectory} for PID file.

  Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
  StartLimitBurst directives as those should be set indivdually.

  Remove Group to user the bitcoin user's default group.

  Changed Restart from 'always' to 'on-failure' (can also be overwritten
  individually).

Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
16be7dd Improve bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Add comment how further options can be added or existing ones
  modified. Use /run/${RuntimeDirectory} for PID file.

  Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
  StartLimitBurst directives as those should be set indivdually.

  Remove Group to user the bitcoin user's default group.

  Changed Restart from 'always' to 'on-failure' (can also be overwritten
  individually).

Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jan 12, 2020
16be7dd Improve bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Add comment how further options can be added or existing ones
  modified. Use /run/${RuntimeDirectory} for PID file.

  Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
  StartLimitBurst directives as those should be set indivdually.

  Remove Group to user the bitcoin user's default group.

  Changed Restart from 'always' to 'on-failure' (can also be overwritten
  individually).

Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jan 16, 2020
16be7dd Improve bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Add comment how further options can be added or existing ones
  modified. Use /run/${RuntimeDirectory} for PID file.

  Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
  StartLimitBurst directives as those should be set indivdually.

  Remove Group to user the bitcoin user's default group.

  Changed Restart from 'always' to 'on-failure' (can also be overwritten
  individually).

Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jan 22, 2020
16be7dd Improve bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Add comment how further options can be added or existing ones
  modified. Use /run/${RuntimeDirectory} for PID file.

  Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
  StartLimitBurst directives as those should be set indivdually.

  Remove Group to user the bitcoin user's default group.

  Changed Restart from 'always' to 'on-failure' (can also be overwritten
  individually).

Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jan 29, 2020
16be7dd Improve bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Add comment how further options can be added or existing ones
  modified. Use /run/${RuntimeDirectory} for PID file.

  Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
  StartLimitBurst directives as those should be set indivdually.

  Remove Group to user the bitcoin user's default group.

  Changed Restart from 'always' to 'on-failure' (can also be overwritten
  individually).

Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4
PastaPastaPasta added a commit to PastaPastaPasta/dash that referenced this pull request Jan 29, 2020
16be7dd Improve bitcoind systemd service file (Florian Schmaus)

Pull request description:

  Add comment how further options can be added or existing ones
  modified. Use /run/${RuntimeDirectory} for PID file.

  Remove TimeoutStopSec, TimeoutStartSec, StartLimitInterval,
  StartLimitBurst directives as those should be set indivdually.

  Remove Group to user the bitcoin user's default group.

  Changed Restart from 'always' to 'on-failure' (can also be overwritten
  individually).

Tree-SHA512: f76674c11fd6e3faaf786aa05686926523d9c875aad6b776337f800108fdb716470286805c532b494f8cf713cb5eea6b735e1c7c238ffb407a5cc909dda41aa4
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants