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

feat(stages): remove static files stage, run before loop #6763

Merged
merged 5 commits into from
Feb 26, 2024

Conversation

shekhirin
Copy link
Collaborator

@shekhirin shekhirin commented Feb 23, 2024

StaticFile stage doesn't fit well into the pipeline abstraction, because the expectation is that Headers executes first, converting the target block hash into the target block number, and all consequent stages execute on top of Headers stage checkpoint.

Instead, we run the StaticFileProducer before main pipeline loop to ensure that all data are copied from MDBX to static files, so the pipeline stages like Headers, Bodies and Execution could write directly to them.

h/t @rkrasiuk for the idea

@shekhirin shekhirin added A-static-files Related to static files C-enhancement New feature or request A-staged-sync Related to staged sync (pipelines and stages) labels Feb 23, 2024
@shekhirin shekhirin marked this pull request as ready for review February 23, 2024 18:52
Copy link
Member

@rkrasiuk rkrasiuk left a comment

Choose a reason for hiding this comment

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

great

Comment on lines +200 to +207
Pipeline::builder().build(
provider_factory.clone(),
StaticFileProducer::new(
provider_factory.clone(),
provider_factory.static_file_provider(),
PruneModes::default(),
),
),
Copy link
Member

@gakonst gakonst Feb 23, 2024

Choose a reason for hiding this comment

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

would like to improve the builder pattern here such that we don't have 3 mentions of provider factory in the call -- not a blocker

Copy link
Collaborator Author

@shekhirin shekhirin Feb 23, 2024

Choose a reason for hiding this comment

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

it's a bit of an unreal example because of debug cmd. in practice, it looks like this

// configure static_file_producer
let static_file_producer = reth_static_file::StaticFileProducer::new(
provider_factory.clone(),
provider_factory.static_file_provider(),
config.prune_config()?.unwrap_or_default().segments,
);

.build(provider_factory, static_file_producer);

but I agree that we can pass only provider_factory and prune_modes inside the StaticFileProducer::new

@shekhirin shekhirin merged commit 81c10b4 into feat/static-files Feb 26, 2024
23 of 25 checks passed
@shekhirin shekhirin deleted the alexey/remove-static-files-stage branch February 26, 2024 13:03
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 C-enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants