Skip to content

[Merged by Bors] - Add block roots heal logic in v18 schema migration.#4875

Closed
jimmygchen wants to merge 1 commit intosigp:unstablefrom
jimmygchen:heal-block-roots
Closed

[Merged by Bors] - Add block roots heal logic in v18 schema migration.#4875
jimmygchen wants to merge 1 commit intosigp:unstablefrom
jimmygchen:heal-block-roots

Conversation

@jimmygchen
Copy link
Copy Markdown
Member

@jimmygchen jimmygchen commented Oct 24, 2023

Issue Addressed

Fixes #4697.

This also unblocks the state pruning PR (#4835). Because self healing breaks if state pruning is applied to a database with missing block roots.

Proposed Changes

  • Fill in the missing block roots between last restore point slot and split slot when upgrading to latest database version.

@jimmygchen jimmygchen mentioned this pull request Oct 24, 2023
Closed
8 tasks
michaelsproul
michaelsproul approved these changes Oct 25, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't fault this, and I think the tests are great.

Let's merge!

beacon_node/store/src/hot_cold_store.rs
last_restore_point_slot.as_usize(),
)?;

for slot in (last_restore_point_slot.as_u64()..split.slot.as_u64()).map(Slot::new) {
Copy link
Copy Markdown
Member

@michaelsproul michaelsproul Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noting that this is the correct upper-bound, because the invariant is exclusive of the split slot. We see that here:

lighthouse/beacon_node/store/src/hot_cold_store.rs

Lines 1764 to 1768 in 441fc16

// Chunk writer for the linear block roots in the freezer DB.
// Start at the new upper limit because we iterate backwards.
let new_frozen_block_root_upper_limit = finalized_state.slot().as_usize().saturating_sub(1);
let mut block_root_writer =
ChunkWriter::<BlockRoots, _, _>::new(&store.cold_db, new_frozen_block_root_upper_limit)?;

This also matches the invariant description on the PR.

I've opened #4878 to track the broader DB invariant project.

Copy link
Copy Markdown
Member Author

@jimmygchen jimmygchen Oct 25, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for checking!
Great team work on the tests 😄

@michaelsproul michaelsproul added ready-for-merge This PR is ready to merge. v4.6.0 ETA Q1 2024 labels Oct 25, 2023
@michaelsproul
Copy link
Copy Markdown
Member

michaelsproul commented Oct 25, 2023

bors r+

bors bot pushed a commit that referenced this pull request Oct 25, 2023
@jimmygchen
Add block roots heal logic in v18 schema migration. (#4875)
d4f26ee 
## Issue Addressed

Fixes #4697. 

This also unblocks the state pruning PR (#4835). Because self healing breaks if state pruning is applied to a database with missing block roots.

## Proposed Changes

- Fill in the missing block roots between last restore point slot and split slot when upgrading to latest database version.
@bors
Copy link
Copy Markdown

bors bot commented Oct 25, 2023

Pull request successfully merged into unstable.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot changed the title Add block roots heal logic in v18 schema migration. [Merged by Bors] - Add block roots heal logic in v18 schema migration. Oct 25, 2023
@bors bors bot closed this Oct 25, 2023
bors bot pushed a commit that referenced this pull request Nov 3, 2023
@jimmygchen @michaelsproul
Add commmand for pruning states (#4835)
36d8849 
## Issue Addressed

Closes #4481. 

(Continuation of #4648)

## Proposed Changes

- [x] Add `lighthouse db prune-states`
- [x] Make it work
- [x] Ensure block roots are handled correctly (to be addressed in 4735)
- [x] Check perf on mainnet/Goerli/Gnosis (takes a few seconds max)
- [x] Run block root healing logic (#4875 ) at the beginning
- [x] Add some tests
- [x] Update docs
- [x] Add `--freezer` flag and other improvements to `lighthouse db inspect`

Co-authored-by: Michael Sproul <michael@sigmaprime.io>
Co-authored-by: Jimmy Chen <jimmy@sigmaprime.io>
Co-authored-by: Michael Sproul <micsproul@gmail.com>
@jimmygchen jimmygchen mentioned this pull request Dec 4, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@michaelsproul michaelsproul michaelsproul approved these changes

Assignees

No one assigned

Labels

database ready-for-merge This PR is ready to merge. v4.6.0 ETA Q1 2024

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jimmygchen @michaelsproul