sigp / lighthouse

Ethereum consensus client in Rust
https://lighthouse.sigmaprime.io/
Apache License 2.0
2.93k stars 745 forks source link

Optimize Redb temporary state cleanup #6332

Open eserilev opened 2 months ago

eserilev commented 2 months ago

Description

4718 Introduces Redb as another database implementation for the beacon node. According to most metrics we've seen so far, its performance is on-par with leveldb in most cases. One case in which it is drastically underperforming is at temp state cleanup during a node restart. The latest restart on holesky took roughly 1.5 hours!

The issue can be found in do_atomically

https://github.com/sigp/lighthouse/blob/06490d4c274bd7c7f52e70f0c302513072e5355c/beacon_node/store/src/database/redb_impl.rs#L166-L203

The way this function is structured, we are constantly opening and closing write transactions during each iteration. Since Redb only allows for one open write transaction at a time, and write transactions can only be opened against individual tables, we will need to refactor our garbage collection logic to conform to Redb functionality.

My best idea so far is to make the garbage collection logic atomic on a per table basis. Instead of passing in an ops vec that contains transactions across multiple tables, we create a vec for each table. We can then pass each vec into a new function that keeps a single write transaction open across the full vec of ops. As long as we're ok with garbage collection only being atomic across individual tables, this should help us get to the performance were looking for.

I think Michael also mentioned that tree-states will help us reduce the amount of temp states being stored in general.

eserilev commented 1 month ago

Been mucking around with different optimizations to temp state cleanup. I did get it down from 2hrs to like 30min by doing the delete ops on a per table basis. Using Redb's remove method is still pretty slow though. I dont think this method is great for bulk operations.

Theres another method retain which applies a predicate to each key value pair in the table. All entries for which predicate are evaluated to false are removed. (https://docs.rs/redb/latest/redb/struct.Table.html#method.retain)

I have a garbage collection implementation that takes all the temp state roots & pushes them into a HashSet. The predicate in retain checks if a key entry exists in the hashset. If the entry exists in the set it returns false, removing that record from the table. Results so far are promising, garbage collection took something on the order of a few seconds (I've so far only tested this on a small batch of temp state roots)

I'm going to let the gown BN run until monday and try redeploying. That should give us enough temp state roots to really stress test these changes.

Another further optimization is to simply drop/recreate the BeaconStateTemp table after deleting the temp state roots from the other relevant tables.

michaelsproul commented 1 month ago

I've made a change here that I think will help: