search

decred / dcrdata

Decred block explorer, with packages and apps for data collection and storage. Written in Go.
https://dcrdata.decred.org/
ISC License
129 stars 128 forks source link

handling lost dcrd RPC connection #1541

Open chappjc opened 5 years ago

chappjc commented 5 years ago

Historically, dcrdata has used rpcclient with the autoreconnect option. This was an issue because when a disconnect-reconnect happened, missed notifications don't just flood in. dcrdata would need to be notified of a reconnect so that it can check the chain state to see what happened while it was disconnected (e.g new blocks or a reorg).

However, there is no disconnect or reconnect notification from rpcclient that can assist with this. In the absence of such notifications, dcrdata would need to wrap rpcclient.

JoeGruffins commented 5 years ago

rpcclient does have a connect, reconnect notification

https://github.com/decred/dcrd/blob/38203c8f4d5673d3c855d6408998948d335b25c8/rpcclient/notify.go#L83-L86

would this and a similar disconnect notification be sufficient? Did you also want an on-demand ping?

It appears that rpcclient is receiving disconnects as soon as gorilla notices them here https://github.com/decred/dcrd/blob/38203c8f4d5673d3c855d6408998948d335b25c8/rpcclient/infrastructure.go#L444-L450 And it seems pretty simultaneous. It also looks like it should not be logging disconnects, but it is.

chappjc commented 5 years ago

The disconnect notification seems to be a missing piece. Also, as jrick mentioned on matrix, rpcclient could use a ping the server function, with some configurable period and timeout. I suppose making it possible to enable/disable the ping could be desirable, but I don't see a downside to having it extra chatting by default.

Depending on how gorilla notices the disconnects, it might always not be quite so prompt. To ensure timely disconnect ntfns, a frequent ping would be required.

A diff from jrick: https://github.com/jrick/wsrpc/commit/8f4555f8a726b7b1c1e753b33ad268fcc7955946

JoeGruffins commented 5 years ago

OnDisconnect notification a no-go, see https://github.com/decred/dcrd/issues/1877

jrick commented 5 years ago

If you are going to use wsrpc as an example for ping-pong, see https://github.com/jrick/wsrpc/compare/v2.0.0...v2.1.1 which is most of that diff but also fixes a couple oversights.

chappjc commented 5 years ago

I can see why my comments above are misleading. It's true that only the reconnect notification is needed to know when to check on chain state. I still believe being notified of disconnects when and if rpcclient know about it would be helpful for a variety of reasons