Mempool syncing optimizations & fixups

shesek commented 3 months ago

Optimizations:

Send RPC requests in parallel over multiple TCP connections, without batching (38e73d9a2808586e202b16d619645e671817c055)

The number of parallel requests to send can be controlled with a new --daemon-parallelism CLI options (defaults to 4).
Continuously attempt to fetch a full mempool snapshot, keeping whatever transactions we were able to get each time (6a14f40d61317b11b6b3f0eeae1199d39585dda9)

Fixups (not strictly necessary for the other changes, but touches on related areas and the previous behavior was wrong):

Don't add transactions with unknown parents to ensure mempool consistency (c60207ad6c81f1ec05982babf7941be53ddcdb26)
Make sure the chain tip doesn't move while fetching the mempool (a6a796c69e5ea935ff67434633139f0dd214476e)

This will require adjusting bitcoind's rpcthreads and possibly rpcworkqueue configuration options upwards. The default rpcthreads is 4, which would be exhausted by mempool fetching alone (electrs keeps at least one other open connection, and we probably want to allow some room for other clients too).

Based on top of #76. (now merged)

philippem commented 3 months ago

Overall this MR seems to greatly improve the initial sync and steady state of electrs alongside a slower bitcoind.

some issues found during testing

the electrum method blockchain.transaction.get will panic on the handling thread if the transaction is not known by electrs (panic starts at https://github.com/shesek/electrs/blob/202405-mempool-sync/src/electrum/server.rs#L375
some requests from electrs to bitcoind will disconnect waiting for recv, at https://github.com/shesek/electrs/blob/202405-mempool-sync/src/daemon.rs#L210 Increasing the rpcthreads parameter in bitcoind does not seem to change this. Electrs itself seems to recover. These recv disconnects happen with different rpcs (usually getrawtransaction, but also getbestblockhash)

philippem commented 3 months ago

* some requests from electrs to bitcoind will disconnect waiting for recv, at https://github.com/shesek/electrs/blob/202405-mempool-sync/src/daemon.rs#L210
  Increasing the `rpcthreads` parameter in bitcoind does not seem to change this. Electrs itself seems to recover. These recv disconnects happen with different rpcs (usually `getrawtransaction`, but also `getbestblockhash`)

While tracing this with wireshark, I observed that electrs sends a request (such as getbestblockhash), and waits for a response. After 30 seconds (and a few TCP Keep-Alive interactions), bitcoind sends a FIN,ACK without any text response. Seems to be some behaviour in bitcoind.

shesek commented 1 month ago

the electrum method blockchain.transaction.get will panic on the handling thread if the transaction is not known by electrs

I was unable to reproduce this, the "missing transaction" message appears to be returned as a JSONRPC error without panicking:

# Client
$ echo '{"id": 1, "method": "blockchain.transaction.get", "params": ["0000000000000000000000000000000000000000000000000000000000000000"]}' | nc 172.17.0.3 60401
{"error":"missing transaction","id":1,"jsonrpc":"2.0"}

# Server logs
TRACE RPC Request("{\"id\": 1, \"method\": \"blockchain.transaction.get\", \"params\": [\"0000000000000000000000000000000000000000000000000000000000000000\"]}\n")
WARN rpc #1 blockchain.transaction.get [String("0000000000000000000000000000000000000000000000000000000000000000")] failed: Error: missing transaction
TRACE RPC Done
# ... keeps running

Seems to be some behaviour in bitcoind.

I'm seeing that behaviour on my local regtest env too.