Please see the discussion of #15367 for detailed rationale. Essentially, the idea is to indicate, via creation of a file, that bitcoind is not only running but ready to actually use. This should provide a convenient way for scripts to reliably start bitcoind, wait for it to become ready, and then subsequently start any other services (e.g. lnd) that depend on it.
This PR also adds a -ready argument for specifying the name of the ready file (default: $DATADIR/.ready)
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
229 | @@ -194,6 +230,7 @@ void Interrupt() 230 | void Shutdown(InitInterfaces& interfaces) 231 | { 232 | LogPrintf("%s: In progress...\n", __func__); 233 | + RemoveFile(GetReadyFile());
If the computer crashes, the file won't get deleted...
Fair point, but the external command approach would likely have the same limitation. Perhaps we could remove any existing ready file early in AppInitMain() (or earlier) to mitigate this?
I am very uncomfortable with adding the ability to exec() external programs. It just feels like an attack vector.
-blocknotify and -walletnotify already exist.
I think executing a command is a better approach. You can then have it execute echo >/path/to/.ready if you like.
@murrayn Is it worth writing the PID to the readiness file to allow the consumer to verify that the file is not stale?
Would it be worth to allow for also the "waiting for startup" scenario by writing to say .bitcoind-state.initializing immediately on startup-up and then rename it to .bitcoind-state.ready when the initialization is done?
140 | + return InitError(strprintf(_("Unable to create the ready status file '%s': %s"), 141 | + GetReadyFile().string(), std::strerror(errno))); 142 | + } 143 | +} 144 | + 145 | +static bool RemoveFile(fs::path file)
Should be const ref? :-)
Yes, fixed.
@practicalswift Thanks for the comments. I did consider both of those approaches. But the PID is already available in bitcoind.pid, so doing so here seems redundant. I also considered having a 'status' file of some type, instead of 'ready', but I think the only interesting status is likely to be 'ready' anyway.
NACK, sorry.
the idea is to indicate, via creation of a file, that
bitcoindis not only running but ready to actually use
I'd say just use it. A good script should handle retrying, not only when it's starting up, but also when it crashed or it's being updated or something. This approach also works if the other service is not in the same host, which this PR can't handle. See the test framework, it checks if it's ready by calling getblockcount a couple of times:
https://github.com/bitcoin/bitcoin/blob/2d46f1be0c3c8b7287aa1f62bb1f5b4a8d00ff6e/test/functional/test_framework/test_node.py#L206-L219
@murrayn I think a better modification is perhaps adding support for sd_notify (behind a configure flag) that would tell systemd when things are ready. Or doing something like a systemd socket configuration. But of course all of those are systemd specific.
Doesn't seem to be much traction for this, so I'll close it.