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

fix: copy and prune data from database with move_to_static_files, before a pipeline run/unwind #8127

Merged
merged 13 commits into from
May 9, 2024

Conversation

joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented May 6, 2024

Before we were only copying data from database to static files with the expectation that during live sync the pruner would gradually delete it.

However, if there is an unwind that fails/crashes/panics for whatever reason, we end up in a situation where the database has supposedly unwounded data (potentially invalid), even though static file data has been unwound.

This PR makes sure that we delete whatever data we copied pre-pipeline run/unwind. So, in case of a failed unwind, there's no leftover database data

@joshieDo joshieDo added C-bug An unexpected or incorrect behavior A-staged-sync Related to staged sync (pipelines and stages) A-pruning Related to pruning or full node A-static-files Related to static files labels May 6, 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.

that makes sense to me,

the clone is a bit meh, not a big deal, but what if we Arc the DB in the producer instead?

@joshieDo joshieDo changed the title fix: move_to_static_files pre-pipeline copies and prunes data from database fix: pre-pipeline, move_to_static_files copies and prunes data from database May 6, 2024
@joshieDo joshieDo changed the title fix: pre-pipeline, move_to_static_files copies and prunes data from database fix: before a pipeline run/unwind move_to_static_files copies and prunes data from database May 6, 2024
@joshieDo joshieDo changed the title fix: before a pipeline run/unwind move_to_static_files copies and prunes data from database fix: copy and prune data from database with move_to_static_files, before a pipeline run/unwind May 6, 2024
@joshieDo joshieDo requested a review from rakita as a code owner May 6, 2024 18:19
@joshieDo joshieDo requested a review from mattsse May 6, 2024 18:20
Copy link
Collaborator

@shekhirin shekhirin left a comment

Choose a reason for hiding this comment

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

The change LGTM, only need to set the pruner timeout because IIUC otherwise we'll use the default 100ms one

@joshieDo joshieDo requested a review from shekhirin May 7, 2024 10:56
@joshieDo
Copy link
Collaborator Author

joshieDo commented May 7, 2024

tested with reth stage unwind and passed, but needed this added to the pruner 8b5444e , for when the tip is the highest static file block. Otherwise Pruner wouldn't be called

@joshieDo joshieDo requested a review from shekhirin May 7, 2024 21:18
@joshieDo
Copy link
Collaborator Author

joshieDo commented May 9, 2024

@shekhirin 8b5444e approve?

@joshieDo joshieDo added this pull request to the merge queue May 9, 2024
Merged via the queue into main with commit ad54af8 May 9, 2024
29 checks passed
@joshieDo joshieDo deleted the joshie/prune-before-pipeline branch May 9, 2024 18:38
Rjected pushed a commit that referenced this pull request May 21, 2024
mw2000 pushed a commit to mw2000/reth that referenced this pull request Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-pruning Related to pruning or full node A-staged-sync Related to staged sync (pipelines and stages) A-static-files Related to static files C-bug An unexpected or incorrect behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants