Resolve cold-state migration boundaries using finalized canonical block root - #17345
Open
syjn99 wants to merge 4 commits into
Open
Resolve cold-state migration boundaries using finalized canonical block root#17345syjn99 wants to merge 4 commits into
syjn99 wants to merge 4 commits into
Conversation
The only reader/writer of the slot field in `finalizedInfo` is `MigrateToCold` method. And the slot is used as a cursor to track the migration progress. When there's no state in epoch boundary cache for specific finalized root, it refuses to save finalized information, and this makes the migration cursor not advance. This PR promotes `finalizedInfo.slot` to `State.migratedSlot` and makes it guarded by `migrationLock`.
syjn99
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
What does this PR do? Why is it needed?
This PR addresses two issues regarding cold DB migration.
Decouple
finalizedInfo.slotwith other fields infinalizedInfoWe only caches the finalized state information when epoch boundary cache has it (
s.epochBoundaryStateCache.getByBlockRoot(fRoot)).finalizedInfo.slotacts as a migration cursor though. So if the node doesn't have a cache - the migration progress cannot be saved, and it might need to do the same workload. Epoch boundary cache can be empty when the node is freshly started, or the chain is suffering from long unfinalization.This PR moves the slot field into
migratedSlot. Note thatmigratedSlotis safe undermigrationLock.Ensure
aRootas a canonical rootWe have an assumption above and return error when
HighestRootsBelowSlotreturns multiple block roots. Actually this claim is false, as the database is not pruned. So slots below the finalized slot can legitimately hold multiple roots. Also it's possible that the only returned root is actually from orphan block.This PR introduces
canonicalRootBelowSlothelper function so that we can ensure the root is canonical and finalized. It iterates maximummaxFinalizedRootSearch(= 1024) times to query the database. For the most of the case, a single attempt is fine but I put an reasonably arbitrary upper limit as it is essentially unbounded operation.I think the consistency is much more important when we ship state diff feature, and this PR guarantees some levels of consistency (the DB saved state/state diff data is canonical).
Regression tests are added:
TestMigrateToCold_RegeneratePath_IgnoresOrphan: Add one more orphan block and check whether saved state is not from the orphaned block.TestMigrateToColdHdiff_BoundaryCacheMiss_SelectsCanonicalRoot: Add an orphan block for slot 96.Which issue(s) does this PR fix?
N/A
Other notes for review
Tip
Please read commit by commit, with commit descriptions.
Acknowledgements