Skip to content

Conversation

@taratorio
Copy link
Member

@taratorio taratorio commented Aug 6, 2024

Before this PR we called heimdall.Synchronize as part of heimdall.CheckpointsFromBlock and heimdall.MilestonesFromBlock. The previous implementation of Synchronize was waiting on all scrappers to be synchronised.

This is inefficient because heimdall.CheckpointsFromBlock needs only the checkpoints scrapper to be synchronised. For the initial sync we first only need to wait for the checkpoints to be downloaded and then we can start downloading blocks from devp2p. While we are doing that we can let the spans and milestones be scrapped in the background. Note this is based on the fact that fetching checkpoints has been optimised by doing bulk fetching and finishes in seconds, while fetching Spans has not yet been optimised and for bor-mainnet can take a long time.

Changes in the PR:

  • splits Synchronize into 3 more fine grained SynchronizeCheckpoints, SynchronizeMilestones and SynchronizeSpans calls which are invoked by the Sync algorithm at the right time
  • Optimises SynchronizeSpans to check if it already has the corresponding span for the given block number before blocking
  • Moves synchronisation point for Spans and State Sync Events in Sync.commitExecution just before we call ExecutionEngine.UpdateForkChoice to make it clearer what data is necessary to be sync-ed before calling Execution
  • Changes EventNotifier and Synchronize funcs to return err if ctx is cancelled or other errors have happened
  • Input consistency between the heimdallSynchronizer and bridgeSynchronizer - use blockNum instead of *type.Header
  • Interface tidy ups

taratorio added 30 commits July 19, 2024 19:13
Base automatically changed from astrid-span-accum-producer-priorities to main August 6, 2024 11:42
@taratorio taratorio marked this pull request as ready for review August 6, 2024 16:56
@taratorio taratorio merged commit df04b78 into main Aug 7, 2024
@taratorio taratorio deleted the astrid-heimdall-tidy-up-synchronise branch August 7, 2024 13:39
somnergy pushed a commit that referenced this pull request Aug 16, 2024
Before this PR we called heimdall.Synchronize as part of
heimdall.CheckpointsFromBlock and heimdall.MilestonesFromBlock. The
previous implementation of Synchronize was waiting on all scrappers to
be synchronised.

This is inefficient because `heimdall.CheckpointsFromBlock` needs only
the `checkpoints` scrapper to be synchronised. For the initial sync we
first only need to wait for the checkpoints to be downloaded and then we
can start downloading blocks from devp2p. While we are doing that we can
let the spans and milestones be scrapped in the background. Note this is
based on the fact that fetching checkpoints has been optimised by doing
bulk fetching and finishes in seconds, while fetching Spans has not yet
been optimised and for bor-mainnet can take a long time.

Changes in the PR:
- splits Synchronize into 3 more fine grained SynchronizeCheckpoints,
SynchronizeMilestones and SynchronizeSpans calls which are invoked by
the Sync algorithm at the right time
- Optimises SynchronizeSpans to check if it already has the
corresponding span for the given block number before blocking
- Moves synchronisation point for Spans and State Sync Events in
`Sync.commitExecution` just before we call
ExecutionEngine.UpdateForkChoice to make it clearer what data is
necessary to be sync-ed before calling Execution
- Changes EventNotifier and Synchronize funcs to return err if ctx is
cancelled or other errors have happened
- Input consistency between the heimdallSynchronizer and
bridgeSynchronizer - use blockNum instead of *type.Header
- Interface tidy ups
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants