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

#1901 remove PID file from service #1930

Closed
wants to merge 1 commit into from
Closed

#1901 remove PID file from service #1930

wants to merge 1 commit into from

Conversation

rootzoll
Copy link
Collaborator

getting rid of the PID file bitcoind is creating to prevent problems like #1901

@rootzoll rootzoll added the teamdiscuss discuss options in next team meetup label Jan 10, 2021
@openoms
Copy link
Collaborator

openoms commented Jan 10, 2021

It seems the PID file is needed to be able to spawn more instances of the process: https://stackoverflow.com/questions/47826123/what-is-systemd-pid-file

Looking at htop I have multiple bitcoimd processes running unsurpisingly since many services are relying on it.

It might be a bad idea to remove that PID file, but will need to try to confirm.

@rootzoll rootzoll added this to the 1.7 Release milestone Jan 12, 2021
@rootzoll
Copy link
Collaborator Author

OK pushing this to next release until we can confirm.

@openoms
Copy link
Collaborator

openoms commented Mar 9, 2021

I think we would need to remove the -daemon entry from the bitcoin.service to get rid of the PID file. Its shoul not be needed if running with systemd.

From bitcoind -help:

  -daemon
       Run in the background as a daemon and accept commands

  -pid=<file>
       Specify pid file. Relative paths will be prefixed by a net-specific
       datadir location. (default: bitcoind.pid)

Extract from a chat on #cl-lightning cdecker:

Never tried setting `--daemon` in a config, setting it from the command line should work though
I see I try to put it to the service, but if all it does is suppress the output it might be not needed
it does not seem to like the daemon setting with multiple processes, complaining about left-overs and unable to lock the pidfile
is there any problem with running with running without daemon=true?
Nope, `--daemon` just detaches from the terminal and runs in the background, so if you're using systemd, init, supervisor, or any other service/process manager it won't change anything (well except a bit more work forking the process in `--daemon` mode)
thank you for the explanation. I was only curious because @darosior mentioned that it would deafult to create a logfile in daemon mode, but I rather just use log-file= option in the config.

@openoms
Copy link
Collaborator

openoms commented Mar 9, 2021

Tested and the bictoind.service does not start without the -daemon option.

The PID file (/home/bitcoin/.bitcoin/bitcoind.pid) is recreated automaically on restart if the -dameon option is present so the PID file path does not need to be defined.
The line
PIDFile=/home/bitcoin/.bitcoin/bitcoind.pid

can be removed also.

If the process type is set to
Type=simple
instead of Type=forking
the -daemon option can be removed.

Currently testing ok with the settings:

[Unit]
Description=Bitcoin daemon on signet

[Service]
User=joinmarket
Group=joinmarket
Type=simple
ExecStart=/home/joinmarket/bitcoin/bitcoind -signet
KillMode=process
Restart=always
TimeoutSec=120
RestartSec=30
StandardOutput=null
StandardError=journal

[Install]
WantedBy=multi-user.target

Will apply the changes to a mainnet node as well.

@openoms
Copy link
Collaborator

openoms commented Mar 9, 2021

On mainnet the change is not going well:

# RaspiBlitz: systemd unit for bitcoind

[Unit]
Description=Bitcoin daemon
Wants=bootstrap.service
After=bootstrap.service

# for use with sendmail alert (coming soon)
#OnFailure=systemd-sendmail@%n

[Service]
User=bitcoin
Group=bitcoin
Type=simple
#PIDFile=/home/bitcoin/.bitcoin/bitcoind.pid
ExecStartPre=-/home/admin/config.scripts/blitz.systemd.sh log blockchain STARTED
ExecStart=/usr/local/bin/bitcoind -conf=/home/bitcoin/.bitcoin/bitcoin.conf
# -daemon -pid=/home/bitcoin/.bitcoin/bitcoind.pid
KillMode=process
Restart=always
TimeoutSec=120
RestartSec=30
StandardOutput=null
StandardError=journal

[Install]
WantedBy=multi-user.target
Mar 09 11:29:12 raspberrypi systemd[1]: bitcoind.service: Found left-over process 2808 (bitcoind) in control group while starting unit. Ignoring.
Mar 09 11:29:12 raspberrypi systemd[1]: This usually indicates unclean termination of a previous run, or service implementation deficiencies.
Mar 09 11:29:12 raspberrypi systemd[1]: Starting Bitcoin daemon...
Mar 09 11:29:12 raspberrypi systemd[1]: bitcoind.service: Found left-over process 2808 (bitcoind) in control group while starting unit. Ignoring.
Mar 09 11:29:12 raspberrypi systemd[1]: This usually indicates unclean termination of a previous run, or service implementation deficiencies.
Mar 09 11:29:12 raspberrypi systemd[1]: Started Bitcoin daemon.
Mar 09 11:29:12 raspberrypi bitcoind[3368]: Error: Cannot obtain a lock on data directory /mnt/hdd/bitcoin. Bitcoin Core is probably already running.
Mar 09 11:29:12 raspberrypi systemd[1]: bitcoind.service: Main process exited, code=exited, status=1/FAILURE
Mar 09 11:29:12 raspberrypi systemd[1]: bitcoind.service: Failed with result 'exit-code'.

@openoms
Copy link
Collaborator

openoms commented Mar 9, 2021

When applying only the change in this PR it works:

# RaspiBlitz: systemd unit for bitcoind

[Unit]
Description=Bitcoin daemon
Wants=bootstrap.service
After=bootstrap.service

# for use with sendmail alert (coming soon)
#OnFailure=systemd-sendmail@%n

[Service]
User=bitcoin
Group=bitcoin
Type=forking
PIDFile=/home/bitcoin/.bitcoin/bitcoind.pid
ExecStartPre=-/home/admin/config.scripts/blitz.systemd.sh log blockchain STARTED
ExecStart=/usr/local/bin/bitcoind -conf=/home/bitcoin/.bitcoin/bitcoin.conf -daemon
#-pid=/home/bitcoin/.bitcoin/bitcoind.pid
KillMode=process
Restart=always
TimeoutSec=120
RestartSec=30
StandardOutput=null
StandardError=journal

[Install]
WantedBy=multi-user.target

The line PIDFile=/home/bitcoin/.bitcoin/bitcoind.pid still defines the PID file which was likely overridden before.

It is ok to merge this, but not expecting any change in the behaviour.

@rootzoll
Copy link
Collaborator Author

Yeah you seem to be right ... this might not change anything in behaviour - so why changing and risking side effects. Closing without merging. Thanks for investigating.

@rootzoll rootzoll closed this Mar 23, 2021
@rootzoll rootzoll removed this from the 1.7 Release milestone Mar 23, 2021
@rootzoll rootzoll deleted the 1901removepid branch May 10, 2022 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
teamdiscuss discuss options in next team meetup
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants