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

Check P-chain ShouldPrune during Initialize #1836

Merged
merged 3 commits into from
Aug 10, 2023
Merged

Conversation

StephenButtolph
Copy link
Contributor

Why this should be merged

Ensures the proposervm doesn't repair its own index multiple times.

How this works

Verifies the platformvm height index on initialize.

How this was tested

CI

Comment on lines -2346 to -2359
shouldPrune, err := s.shouldPrune()
if err != nil {
lock.Unlock()
return fmt.Errorf(
"failed to check if the database should be pruned: %w",
err,
)
}
if !shouldPrune {
lock.Unlock()

log.Info("state already pruned and indexed")
return nil
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This check was moved into vm.Initialize

}
if !shouldPrune {
chainCtx.Log.Info("state already pruned and indexed")
vm.pruned.Set(true)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

By setting pruned here - we guaranteed that we will return nil from VerifyHeightIndex if the P-chain has already been pruned (even if it is called immediately after Initialize).

@StephenButtolph StephenButtolph added the vm This involves virtual machines label Aug 10, 2023
@StephenButtolph StephenButtolph added this to the v1.10.8 milestone Aug 10, 2023
Copy link
Contributor

@patrick-ogrady patrick-ogrady left a comment

Choose a reason for hiding this comment

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

Makes sense to me

@StephenButtolph StephenButtolph merged commit c29ad76 into dev Aug 10, 2023
13 checks passed
@StephenButtolph StephenButtolph deleted the sync-should-prune branch August 10, 2023 02:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vm This involves virtual machines
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants