chain: wait for bitcoind rpc to warm up #677

sangaman · 2020-02-18T07:07:41Z

This modifies the Start method to handle the RPC_IN_WARMUP error code of -28 by retrying the rpc call to determine the current network every second until it either succeeds or fails with a different code. The current behavior fails and terminates the connection upon receiving this error code. This change allows for connecting to a recently started bitcoind node and starting the client while bitcoind is still warming up.

Related issues: lightningnetwork/lnd#1533 & https://github.com/ExchangeUnion/xud-docker/issues/195

I first tried to approach this issue with changes to the lnd codebase, but the relevant code is what's being modified here. Attempting to call the Start() method from lnd a second time, for instance, doesn't work because of the up front check that determines whether Start() has already been called and immediately returns if so.

I've tested this locally by pointing btcwallet in go.mod to my local repository and unlocking lnd moments after starting bitcoind. Instead of shutting down, lnd will keep trying bitcoind until it gets a non-error response and then continue with its initialization process. This would save some grief when bringing a node online and starting bitcoind & lnd at roughly the same time.

halseth · 2020-03-24T11:58:12Z

chain/bitcoind_conn.go

+		} else if err.(*btcjson.RPCError).Code == -28 {
+			// Error code -28 indicates the bitcoind rpc is warming up.
+			// We will wait and recheck periodically until rpc is ready.
+			time.Sleep(time.Second * 1)


this looks okay, the only thing I'm worried about is bitcoind hanging, always returning this code. In that case it looks like we'll deadlock here.

The alternative would be to crash, which is what happens now. This approach looks good to me, maybe just add a log like Waiting for bitcoind RPC ready signal that's only logged once if we ever run into this error code.

There could perhaps be some upper limit of retries before returning an error. A couple of minutes perhaps? But yeah the alternative is crashing, and if ever bitcoind is stuck in this rpc warming up status, something is seriously wrong and requires intervention. I can't say I've ever seen bitcoind get stuck in this point, even if there's something wrong like db corruption it'll eventually crash in my experience.

@sangaman in your experience, what has been the longest bitcoind has been in this warmup state? Waiting something like 5 mins sounds good to me.

In my experience, even on a low-powered ARM machine this warm-up stage takes about a minute or two by default, rarely much more than than that. However, it's also possible to change the config to increase the amount of checks that are done before starting the bitcoind server, theoretically this could take hours if you had it do very deep/thorough checks.

In that case, I'm fine with blocking indefinitely as long as a log statement exists.

Yes, isn't bitcoind in that warmup state for the entire initial blockchain sync (which can take days, or more)?

I think it should wait indefinitely. However the sleep could maybe be increased slowly to a max of 10 minutes or so.

No I don't think so, the warm up state is just while it's verifying the db and blocks on disk. Once it starts accepting and validating new blocks, it's past the warm up state and will respond to rpc requests normally.

Roasbeef · 2020-11-04T02:11:27Z

What's needed to unblock this?

halseth · 2020-11-04T09:34:33Z

Add a log, then I think it's good.

ping @sangaman

sangaman · 2020-12-01T14:40:10Z

Just saw this ping, I'll add a log statement and review the feedback. Sorry this fell off my radar.

sangaman · 2020-12-02T05:21:33Z

I updated this PR to use the newly available btcjson.ErrRPCInWarmup variable (which equals -28) instead of writing in -28 and I added logic to log Waiting for bitcoind RPC to finish warming up... the first time we see that code and Bitcoind finished warming up and RPC is ready after the code goes away. Ping @halseth @wpaulino

halseth

LGTM 🌡️

wpaulino · 2020-12-02T23:43:24Z

chain/bitcoind_conn.go

+		if err == nil {
+			// No error means bitcoind is responsive and we can proceed.
+			break
+		} else if err.(*btcjson.RPCError).Code == btcjson.ErrRPCInWarmup {


We should do a soft type assertion here to prevent a panic if err is not of the type *btcjson.RPCError:

} else if rpcErr, ok := err.(*btcjson.RPCError); ok && rpcErr.Code == btcjson.ErrRPCInWarmup {

Thanks, makes sense. Done.

wpaulino · 2021-01-06T22:40:24Z

cc @Roasbeef

sangaman · 2021-06-08T03:37:58Z

Should I continue with this PR? I do think it's a useful feature. It looks like I'll need to resolve some conflicts with pruned node logic.

wpaulino · 2021-06-08T16:54:07Z

@sangaman yes! It's already gotten approvals, @Roasbeef is just the only one with merge access. I can take another look once rebased.

sangaman · 2021-06-09T18:48:59Z

@wpaulino @Roasbeef I rebased the changes, the code didn't really need to change I just had to move it to keep up with the updated structure.

wpaulino · 2021-06-10T19:29:35Z

chain/bitcoind_conn.go

+	var (
+		net wire.BitcoinNet
+	)


Nit: this can be collapsed into a single line var net wire.BitcoinNet.

Thanks, a leftover from when this also declared err which didn't exactly work any more since it's being used above.

wpaulino · 2021-06-10T19:30:33Z

chain/bitcoind_conn.go

+		if err == nil {
+			// No error means bitcoind is responsive and we can proceed.
+			break
+		} else if rpcErr, ok := err.(*btcjson.RPCError); ok && rpcErr.Code == btcjson.ErrRPCInWarmup {


Nit: line exceeds 80 character limit.

If I split this into two lines, for instance by moving everything after the semicolon to its own line, then make lint doesn't pass and tells me to run gofmt, but if I do that it moves this else if statement back to one line. So I'm not sure what to do, it feels like I'm fighting against the style rules defined by the project.

So the else if can just become an if since the clause above breaks out. We can then split the line at &&.

Thank you for that bit of hand holding, that works out.

This modifies the `Start` method to handle the `RPC_IN_WARMUP` error code of `-28` by retrying the rpc call to determine the current network every second until it either succeeds or fails with a different code. The current behavior fails and terminates the connection upon receiving this error code. This change allows for connecting to a recently started bitcoind node and starting the client while bitcoind is still warming up. Related issues: lightningnetwork/lnd#1533 & https://github.com/ExchangeUnion/xud-docker/issues/195

halseth reviewed Mar 24, 2020

View reviewed changes

sangaman force-pushed the rpc-warmup branch from 47ea1ec to c9f3980 Compare December 2, 2020 05:18

halseth approved these changes Dec 2, 2020

View reviewed changes

wpaulino reviewed Dec 2, 2020

View reviewed changes

sangaman force-pushed the rpc-warmup branch from c9f3980 to ad54c15 Compare December 3, 2020 14:20

wpaulino approved these changes Dec 3, 2020

View reviewed changes

sangaman force-pushed the rpc-warmup branch from ad54c15 to ca5c174 Compare June 9, 2021 18:47

sangaman force-pushed the rpc-warmup branch from ca5c174 to 176b13e Compare June 9, 2021 22:15

wpaulino reviewed Jun 10, 2021

View reviewed changes

sangaman force-pushed the rpc-warmup branch from 176b13e to aeac1a2 Compare June 10, 2021 19:58

wpaulino approved these changes Jun 10, 2021

View reviewed changes

sangaman mentioned this pull request Oct 14, 2021

panic if starting lnd while bitcoind is syncing lightningnetwork/lnd#4818

Closed

sangaman mentioned this pull request May 15, 2023

[Feature Request]: Wait for bitcoin-cli to warmup before crashing lightningnetwork/lnd#1533

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chain: wait for bitcoind rpc to warm up #677

chain: wait for bitcoind rpc to warm up #677

sangaman commented Feb 18, 2020

halseth Mar 24, 2020

wpaulino Mar 24, 2020

sangaman Mar 24, 2020

wpaulino Mar 25, 2020

sangaman Mar 25, 2020

wpaulino Mar 26, 2020

halseth Jul 17, 2020

schildbach May 15, 2023

sangaman May 15, 2023

Roasbeef commented Nov 4, 2020

halseth commented Nov 4, 2020

sangaman commented Dec 1, 2020

sangaman commented Dec 2, 2020

halseth left a comment

wpaulino Dec 2, 2020

sangaman Dec 3, 2020

wpaulino commented Jan 6, 2021

sangaman commented Jun 8, 2021

wpaulino commented Jun 8, 2021

sangaman commented Jun 9, 2021

wpaulino Jun 10, 2021

sangaman Jun 10, 2021

wpaulino Jun 10, 2021

sangaman Jun 10, 2021

wpaulino Jun 10, 2021

sangaman Jun 10, 2021

chain: wait for bitcoind rpc to warm up #677

Are you sure you want to change the base?

chain: wait for bitcoind rpc to warm up #677

Conversation

sangaman commented Feb 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Roasbeef commented Nov 4, 2020

halseth commented Nov 4, 2020

sangaman commented Dec 1, 2020

sangaman commented Dec 2, 2020

halseth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wpaulino commented Jan 6, 2021

sangaman commented Jun 8, 2021

wpaulino commented Jun 8, 2021

sangaman commented Jun 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment