Re-visit how well Erigon can restore values from 'p2p/enode/nodedb.go' after restart

[AskAlexSharov]

Problem: after restart Erigon loosing Good peers and very slow gathering them back
I see next message after node start (and sometime on shutdown also):

Successfully update p2p node database    updated=0 deleted=8206

Need re-check how well Erigon can restore values from 'p2p/enode/nodedb.go' after restart

Need check:

• can it re-use most of values?
• if can re-use - how fast such peers can become "good peers"?
• if Erigon has records in nodedb, can it start with empty Bootnodes list?

[battlmonstr]

@AskAlexSharov

I think that primarily it is a regression after introducing a nodedb cache.

There are multiple issues that cause this behaviour.

The cache is flushed to DB in 2 cases:

1. When reaching ~3-4K new peers due to this check. In practice this rarely happens, because getting to thousands of discovered peers is gonna take quite a while.
2. Upon a graceful shutdown. If the server is killed without a graceful shutdown, nothing will be persisted.

Answering to your questions:

updated=0 deleted=8206

This log message is misleading due to a typo - entriesUpdated/entriesDeleted variables should be reversed there for logging. The message happens normally upon shutdown and says that 8206 database cache entries were flushed to DB (corresponding to about 2K peers, because each peer has 4-5 related entries).

can it re-use most of values?

Yes, but there are caveats:

• The nodes are saved to DB only after seedMinTableTime=5 minutes of staying in the routing table. If you run erigon for less than 5 minutes the seed nodes are empty. It's possible to check if there are some nodes coming from DB after restart by setting a breakpoint here and watch seeds.

• QuerySeeds will poke 150 random entries in the whole node DB and ignore hitting "field" entries. In a long-running scenario each "node record" entry has 3-4 "field" entries (lastping, lastpong, findfail, seq), so making 150 attempts hopefully find about 30 "node record" entries. In a bootstrap scenario this fails, because initially it produces hundreds of pings, and the table has very few "node record" entries, so 150 random attempts are not enough. After running for 15 minutes I've got totalEntryCount=1508 nodeRecordCount=114 entries meaning that there's a 1/16 chance of hitting a "node record" entry, and it means finding just about 10 nodes of 114 total on average from 150 attempts.

how fast such peers can become "good peers"?

There's no prioritization of good peers (the ones that were just connected with Eth66 before shutdown) versus peers that replied to a Ping during the last 5 days (seedMaxAge). QuerySeeds happily returns 30 random peers to put into the table, and then it will start a random lookup. The first phase of that lookup will return 16 of them back from the table. After restarting it is likely to try to connect to a totally different set of peers.

can it start with empty Bootnodes list?

Yes, it can. I tried - it worked.

Possible solutions:

• Revert the nodedb cache optimization.
• Implement a different solution for reducing the number of commits to the nodedb. I feel that most commits are caused by lastping/lastpong updates. If we keep a transaction object for those (using BeginRw instead of Update), we can delay the commits on it. The commits could be done based on a timer, e.g. every 5 sec. This needs a bit of testing to confirm that writing to the other entries is rare.
• Split "node record" entries to a separate table such that QuerySeeds doesn't do idle cycle hits. Or find a different way to fix QuerySeeds, e.g. iterate over all the entries.

This solves the regression. When it comes to prioritizing good peers, this feels like a separate "feature" task that we could discuss and design separately.

CC @AlexeyAkhunov

[battlmonstr]

Yeah, so it is mostly updating lastping/lastpong/findfail:

  53 UpdateFindFails
  68 UpdateLastPingReceived
  77 UpdateLastPongReceived

[AskAlexSharov]

“Holding db tx object” - main problem here is - rwtx object can’t be moved between threads (means can’t “just hide it behind mutex and use from multiple goroutines”). And cache object introduced mostly to handle concurrent writes. There is some known design pattern to handle much parallel writes per second with lmdb, see .Batch method of bbolt https://github.com/etcd-io/bbolt#batch-read-write-transactions (if you like - can implement such in mdbx-go bindings, or kv_mdbx wrapper).
Also readTx will not see any updates until rwtx commit (i don’t know if it’s important here).

“ this feels like a separate "feature" task” - feel free to break it to smaller PR’s as you like.

I think we don’t need “peers prioritization”, but need just re-view that no obvious bugs that make us loose list of peers or too slow restore them.

[battlmonstr]

Mostly done for Erigon2 at 04f07a0 .

I've made a tentative revert PR #3675 for stable. Do we want to backport all the fixes to stable?

[AskAlexSharov]

@battlmonstr "Do we want to backport all the fixes to stable?" - no, only if it fixing some problem. "slow discovery"/"too much commits of enodedb" - is likely a problem.