Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize Redb temporary state cleanup #6332

Closed
eserilev opened this issue Aug 30, 2024 · 3 comments
Closed

Optimize Redb temporary state cleanup #6332

eserilev opened this issue Aug 30, 2024 · 3 comments
Assignees
Labels
database optimization Something to make Lighthouse run more efficiently.

Comments

@eserilev
Copy link
Collaborator

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

pub fn do_atomically(&self, ops_batch: Vec<KeyValueStoreOp>) -> Result<(), Error> {
let open_db = self.db.read();
let mut tx = open_db.begin_write()?;
tx.set_durability(self.write_options().into());
for op in ops_batch {
match op {
KeyValueStoreOp::PutKeyValue(column, key, value) => {
let _timer = metrics::start_timer(&metrics::DISK_DB_WRITE_TIMES);
metrics::inc_counter_vec_by(
&metrics::DISK_DB_WRITE_BYTES,
&[&column],
value.len() as u64,
);
metrics::inc_counter_vec(&metrics::DISK_DB_WRITE_COUNT, &[&column]);
let table_definition: TableDefinition<'_, &[u8], &[u8]> =
TableDefinition::new(&column);
let mut table = tx.open_table(table_definition)?;
table.insert(key.as_slice(), value.as_slice())?;
drop(table);
}
KeyValueStoreOp::DeleteKey(column, key) => {
metrics::inc_counter_vec(&metrics::DISK_DB_DELETE_COUNT, &[&column]);
let _timer = metrics::start_timer(&metrics::DISK_DB_DELETE_TIMES);
let table_definition: TableDefinition<'_, &[u8], &[u8]> =
TableDefinition::new(&column);
let mut table = tx.open_table(table_definition)?;
table.remove(key.as_slice())?;
drop(table);
}
}
}
tx.commit()?;
Ok(())
}

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 eserilev added optimization Something to make Lighthouse run more efficiently. database labels Aug 30, 2024
@eserilev eserilev self-assigned this Aug 30, 2024
@eserilev
Copy link
Collaborator Author

eserilev commented Sep 22, 2024

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
Copy link
Member

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

@eserilev
Copy link
Collaborator Author

we've introduced a delete_batch fn in redb that fixes the slow temp state cleanup issues in this PR #4718

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database optimization Something to make Lighthouse run more efficiently.
Projects
None yet
Development

No branches or pull requests

2 participants