Store finalized block roots in database (3s startup) #3320
Conversation
beacon_chain/beacon_chain_db.nim
Outdated
| yield res | ||
| elif (let blck = db.getMergeBlock(res.root); blck.isSome()): | ||
| res.summary = blck.get().message.toBeaconBlockSummary() | ||
| yield res |
There was a problem hiding this comment.
Looks like you can place a single yield after the if/elif/else section here. Reducing the number of yeilds in an inline iterator is beneficial for reducing the overall code size.
| @@ -433,19 +454,59 @@ proc init*(T: type ChainDAGRef, cfg: RuntimeConfig, db: BeaconChainDB, | |||
| withBlck(genesisBlock): BlockRef.init(genesisBlockRoot, blck.message) | |||
|
|
|||
| var | |||
| headRef: BlockRef | |||
| backfillBlocks = newSeq[Eth2Digest](tailRef.slot.int) | |||
There was a problem hiding this comment.
Doesn't this have to be tailRef.slot.int - backfill.slot semantically?
There was a problem hiding this comment.
no: the seq is there to answer by-slot requests from the rest and libp2p api and needs to cover the entire genesis->tail range (for which we have no blockref) - backfill.slot identifies the "valid" part of this seq.
in other words: after backfilling is done, this seq holds an in-memory mapping of slot->root which we then use to load blocks from the database with.
There was a problem hiding this comment.
btw, this is the seq we talked about that potentially could be removed in future releases and replaced with a database query, in a future release - it's still a bit early for that though
There was a problem hiding this comment.
Isn't the name misleading then? My first interpretation of backfillBlocks will be "the blocks that we backfilled"
There was a problem hiding this comment.
Isn't finalizedBlocks the sequence that starts from genesis?
There was a problem hiding this comment.
one, perhaps confusing inconsistency is that finalizedBlocks in the database covers all finalized blocks, while the in-memory structure is split in two, at the tail. I don't have good ideas for how to make that more clear though, right now
There was a problem hiding this comment.
But why does backfillBlocks need to go to genesis? Shouldn't it cover only the weak subjectivity horizon?
There was a problem hiding this comment.
maybe in the future? right now, the de-facto standard is to backfill to genesis because that's the only point from which you reliably can regenerate states (there's no standard way to distribute states, except the debug REST api) - also, it's needed for clients such as .. say .. nimbus, that don't support checkpoint syncing: a hard-fork is needed before genesis support is dropped.
There was a problem hiding this comment.
Well, and this gets us to my original question. Isn't the size of backfillBlocks semantically tailRef.slot.int - backfill.slot? (this works OK now because backfill.slot is zero and it will work OK in the future).
There was a problem hiding this comment.
still no - backfill.slot is the "first" slot for which we have a block in the database. on checkpoint sync start, it will have the value of tail.slot, then it moves towards genesis. we don't want to reallocate the seq every time a block is backfilled, so we allocate the full seq at once.
arnetheduck
force-pushed
the
fin-block-db
branch
from
b77dce7
to
5df81cd
Compare
When the chain has finalized a checkpoint, the history from that point onwards becomes linear - this is exploited in `.era` files to allow constant-time by-slot lookups. In the database, we can do the same by storing finalized block roots in a simple sparse table indexed by slot, bringing the two representations closer to each other in terms of conceptual layout and performance. Doing so has a number of interesting effects: * mainnet startup time is improved 3-5x (3s on my laptop) * the _first_ startup might take slightly longer as the new index is being built - ~10s on the same laptop * we no longer rely on the beacon block summaries to load the full dag - this is a lot faster because we no longer have to look up each block by parent root * a collateral benefit is that we no longer need to load the full summaries table into memory - we get the RSS benefits of #3164 without the CPU hit. Other random stuff: * simplify forky block generics * fix withManyWrites multiple evaluation * fix validator key cache not being updated properly in chaindag read-only mode * drop pre-altair summaries from `kvstore` * recreate missing summaries from altair+ blocks as well (in case database has lost some to an involuntary restart) * print database startup timings in chaindag load log * avoid allocating superfluos state at startup * use a recursive sql query to load the summaries of the unfinalized blocks
arnetheduck
force-pushed
the
fin-block-db
branch
from
105dd34
to
fba2cae
Compare
When the chain has finalized a checkpoint, the history from that point
onwards becomes linear - this is exploited in
.erafiles to allowconstant-time by-slot lookups.
In the database, we can do the same by storing finalized block roots in
a simple sparse table indexed by slot, bringing the two representations
closer to each other in terms of conceptual layout and performance.
Doing so has a number of interesting effects:
being built - ~10s on the same laptop
this is a lot faster because we no longer have to look up each block by
parent root
summaries table into memory - we get the RSS benefits of DAG loading: don't preload summaries in memory #3164 without
the CPU hit.
Other random stuff:
read-only mode
kvstoredatabase has lost some to an involuntary restart)
blocks