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

chore: remove static file commit() from inside the stage #6717

Merged
merged 4 commits into from
Feb 22, 2024

Conversation

joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented Feb 21, 2024

  • Moved out SnapshotProvider::commit() from inside DatabaseProviderRW::commit (smell)
  • Added SnapshotProvider::commit() before DatabaseProviderRW::commit on stage completion/unwind
  • Remove static file commit() from inside the Header and Bodies stage. It's redundant since the stage will commit it once the run is done. And doing too many times adds up on BodyStage, since underneath it will call fsync
  • Add commit to a bunch of tests. This is important to highlight: data written to static files needs to be committed before it can be used. Only then will it update the SnapshotProvider on the new available tx or block range.

@joshieDo joshieDo added A-staged-sync Related to staged sync (pipelines and stages) A-static-files Related to static files labels Feb 21, 2024
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm

pending @shekhirin

@@ -85,7 +85,6 @@ impl<DB: Database> DerefMut for DatabaseProviderRW<DB> {
impl<DB: Database> DatabaseProviderRW<DB> {
/// Commit database transaction and snapshot if it exists.
pub fn commit(self) -> ProviderResult<bool> {
self.0.snapshot_provider.commit()?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like this, this is a lot cleaner now imo

@joshieDo joshieDo merged commit b0eda75 into feat/static-files Feb 22, 2024
21 of 25 checks passed
@joshieDo joshieDo deleted the joshie/inner-stage-commit branch February 22, 2024 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-staged-sync Related to staged sync (pipelines and stages) A-static-files Related to static files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants