Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Support incremental snapshots and run turbine while blockstore processing at validator boot#23852

Merged
mvines merged 4 commits intosolana-labs:masterfrom
mvines:blocksnap
Apr 23, 2022
Merged

Support incremental snapshots and run turbine while blockstore processing at validator boot#23852
mvines merged 4 commits intosolana-labs:masterfrom
mvines:blocksnap

Conversation

@mvines
Copy link
Copy Markdown
Contributor

@mvines mvines commented Mar 22, 2022 •
edited
Loading

The blockstore processor module that reads the ledger at validator boot has its own full snapshot handling, and no support for incremental snapshots.

Rather than plumbing incremental snapshots, rip out blockstore processor's snapshot handling entirely and start the real snapshot packager earlier in the boot chain.

Additionally, start gossip/turbine before blockstore processing so that new blocks can be ingested into rocksdb as early as possible during validator startup

@mvines mvines force-pushed the blocksnap branch 4 times, most recently from 7fcef98 to 1a77d0a Compare March 24, 2022 17:58
@mvines mvines changed the title Support incremental snapshots during blockstore processing at validator boot Support incremental snapshots and run turbine while blockstore processing at validator boot Mar 24, 2022
@mvines mvines force-pushed the blocksnap branch 4 times, most recently from fb527bf to 1f55e5e Compare March 24, 2022 19:17
@mvines mvines marked this pull request as ready for review March 24, 2022 19:20
@mvines
Copy link
Copy Markdown
Contributor Author

mvines commented Mar 28, 2022

This probably implicitly fixes #23976

@brooksprumo brooksprumo mentioned this pull request Mar 29, 2022
Closed
brooksprumo
brooksprumo reviewed Mar 30, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

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

I like it! Can I help in any way to get this merged?

Comment thread local-cluster/tests/local_cluster.rs Outdated
Comment thread ledger/src/blockstore_processor.rs
@mvines
Copy link
Copy Markdown
Contributor Author

mvines commented Apr 3, 2022

I like it! Can I help in any way to get this merged?

I quickly rebased this guy again. There's just one (or two?) local-cluster tests that need to be massaged to be happy with this PR. I've run it on mainnet and it seems to work just fine.

I'm still dealing with spring break kids for another week, I was going to finally get this thing landed when I return the week of the 11th but if you're interested in picking this up in the meantime please go for it!

@brooksprumo brooksprumo mentioned this pull request Apr 5, 2022
Closed
@stale
Copy link
Copy Markdown

stale Bot commented Apr 16, 2022

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale Bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Apr 16, 2022
@stale stale Bot removed the stale [bot only] Added to stale content; results in auto-close after a week. label Apr 18, 2022
@mvines mvines force-pushed the blocksnap branch 2 times, most recently from 7274b08 to 8566a30 Compare April 19, 2022 21:51
@mvines mvines mentioned this pull request Apr 19, 2022
Merged
@brooksprumo
Copy link
Copy Markdown
Contributor

brooksprumo commented Apr 20, 2022 •
edited
Loading

FYI this PR should've fixed y the intermittent failed for the snapshot local cluster test: #24440

Could also be backported to make your PR here easier.

@mvines mvines force-pushed the blocksnap branch 2 times, most recently from 7e79be7 to 463f8b3 Compare April 20, 2022 16:09
@mvines
Copy link
Copy Markdown
Contributor Author

mvines commented Apr 20, 2022

FYI this PR should've fixed y the intermittent failed for the snapshot local cluster test: #24440

Could also be backported to make your PR here easier.

Ah, thanks. I'll remove the commit in here related to that test then and see if we're still green. The other local-cluster test failures should be resolved now as well.

@mvines mvines force-pushed the blocksnap branch 9 times, most recently from bbd62a8 to 66f118c Compare April 22, 2022 20:17
@mvines mvines merged commit 78d9bd6 into solana-labs:master Apr 23, 2022
@mvines mvines deleted the blocksnap branch April 23, 2022 04:33
ryoqun
ryoqun reviewed Jun 21, 2022
View reviewed changes
Comment thread core/src/validator.rs
}
}

impl<'a> From<ProcessBlockStore<'a>> for Tower {
Copy link
Copy Markdown
Contributor

@ryoqun ryoqun Jun 21, 2022

Choose a reason for hiding this comment

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

here

@ryoqun ryoqun mentioned this pull request Jun 21, 2022
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@ryoqun ryoqun ryoqun left review comments

@brooksprumo brooksprumo brooksprumo left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mvines @brooksprumo @ryoqun