Initial sync as state machine #1961

dapplion · 2021-01-17T19:35:17Z

Refactors initial sync with a heavily Lightouse inspired sync.

TLDR: higher sync speeds - works as a state machine: easier testing, debugging and predictability

Record of successfully completing initial sync on cloud instances

lodestar-cloud-01: Pyrmont on container lodestar_lodestar_1: 2 times
lodestar-cloud-02: Mainnet on container lodestar_lodestar_1: 3 times

The Sync has two integral components:

`Batch`

Represent a fix range of slots to fetch and process. A batch is a state machine that transitions through multiple status with public methods:

AwaitingDownload: Initial state, or the batch has failed either downloading or processing, but can be requested again.
Downloading: The batch is being downloaded
AwaitingProcessing: The batch has been completely downloaded and is ready for processing
Processing: The batch is being processed.
AwaitingValidation: The batch was successfully processed and is waiting to be validated. It is not sufficient to process a batch successfully to consider it correct. This is because batches could be erroneously empty, or incomplete. Therefore, a batch is considered valid, only if the next sequential batch imports at least a block.

Tracking the status in this way simplifies a lot the logic to concurrently handle and download multiple batches at once. The AwaitingValidation also allow to handle infinitely long periods of completely skipped slots without issues.

Batches always start a slot 1 to ease committee forecasting when batch signature verification is ON.

 Epoch boundary |                                   |
  ... | 30 | 31 | 32 | 33 | 34 | ... | 61 | 62 | 63 | 64 | 65 |
       Batch 1       |              Batch 2              |  Batch 3

Batches also track the failed attemps which would allow sofisticate peer scoring in upcomming PRs. So far it's used to reduce the chance of re-requesting a batch to a peer with a failed attempt.

`SyncChain`

Main driver that handles a map of Batches. It uses an AsyncIterable batchProcessor to processes batches to be processed in order and only one at once. All the state necessary to chose the next to be downloaded batch and when syncing is done is derived from the batches. Plus startEpoch which tracks the latest validated epoch.

It interfaces with the sync manager through 3 methods only

processChainSegment
downloadBeaconBlocksByRange
getPeersAndTargetEpoch: Abstracts the peer handling logic upstream. Allows to sync manager to enforce minPeers even throughout the sync

SyncChain instead of start() stop() exposes a single sync() method that resolves when sync. It uses an abort signal for stopping.

SyncChain is able to detect that the sync has stalled with a recurring maybeStuckTimeout. It will print detailed data to help debugging and try to re-fetch new batches and reprocess pending. It also detects specific invalid internal state combinations and will nuke itself if those happen.

codeclimate · 2021-01-17T19:35:47Z

Code Climate has analyzed commit 35dae04 and detected 8 issues on this pull request.

Here's the issue category breakdown:

Category	Count
Complexity	6
Duplication	2

View more on Code Climate.

twoeths · 2021-01-18T10:06:35Z

this is great, the sync speed is really impressive to me! /eth/v1/node/syncing does not seem to be correct anymore during initial sync.

twoeths · 2021-01-18T10:11:44Z

packages/lodestar/src/tasks/tasks/archiveBlocks.ts

@@ -57,8 +56,7 @@ export class ArchiveBlocksTask implements ITask {
      i = upperBound;
    }
    await this.deleteNonCanonicalBlocks();
-    this.logger.profile("Archive Blocks epoch #" + this.finalized.epoch);


why do you remove this? it's used to track how much time we use to archive blocks

As it's currently it's very noisy to log that to info. I agree it's valuable info but it should not be logged to info always. We should profile both the archive blocks task, the state transition and everything else but on demand only.

For now I've remove it since there also this statement

this.logger.verbose("Archiving of finalized blocks complete"

As discussed in the meeting we will implement profiling through metrics in an upcoming PR

twoeths · 2021-01-18T10:19:09Z

packages/lodestar/src/sync/range/chain.ts

+
+      // At least one block was successfully verified and imported, so we can be sure all
+      // previous batches are valid and we only need to download the current failed batch.
+      if (e instanceof BlockProcessorError && e.importedBlocks.length > 0) {


I don't see this BlockProcessorError being thrown anywhere

Right, I'm waiting for Cayman to tweak the chain segment processor and add this error. The error is not necessary to work, it just adds a performance boost in some situations

looks like we'll use ChainSegmentError? https://github.com/ChainSafe/lodestar/blob/master/packages/lodestar/src/chain/errors/blockError.ts#L125

will change in a separate PR

packages/lodestar/src/sync/range/chain.ts

packages/lodestar/src/sync/range/batches.ts

dapplion · 2021-01-18T17:13:30Z

/eth/v1/node/syncing does not seem to be correct anymore during initial sync.

What behavior specifically do you see that does not match expectations? I've changed the target to just be target = clock.currentSlot (as Lighthouse does) as an interim solution for a future PR to handle the case of skipped slots on target.

twoeths · 2021-01-21T10:45:21Z

/eth/v1/node/syncing does not seem to be correct anymore during initial sync.

What behavior specifically do you see that does not match expectations? I've changed the target to just be target = clock.currentSlot (as Lighthouse does) as an interim solution for a future PR to handle the case of skipped slots on target.

it returns {"data":{"head_slot":"438615","sync_distance":"422901"}} (head_slot is wrong) although I just started the sync in 30 minutes

dapplion · 2021-01-22T10:49:19Z

it returns {"data":{"head_slot":"438615","sync_distance":"422901"}} (head_slot is wrong) although I just started the sync in 30 minutes

Thanks for testing it out! Fixed with 88feee9. After a couple of minutes of testing it returns

/usr/app # curl http://localhost:9596/eth/v1/node/syncing
{"data":{"head_slot":"386","sync_distance":"467256"}}

wemeetagain · 2021-01-25T18:24:52Z

packages/lodestar/src/sync/sync.ts

-    await this.regularSync.stop();
-    await this.initialSync.start();
-  }
+  private processChainSegment: ProcessChainSegment = async (blocks) => {


Remove in a future PR

dapplion added 17 commits January 16, 2021 20:06

Add range sync batch class as a state machine

ea7248d

Add TimeSeries to track syncing speed

cd741ea

Add shuffle array util

6374e76

Remove unused utility util/sort

ed89582

Add sortBy utility

00eb9cd

Add itTrigger utility

89b0706

Move all logging to batch

dfcaaba

Add ChainPeersBalancer for range sync

0127a41

Allow to customize BatchOpts for testing

0843cca

Less noisy logging on initial sync

3f3068e

Add getPeersInitialSync logic

54a30ef

Add range SyncChain

f46b63f

Use SyncChain in sync as initial sync

2280c25

Add assertSequentialBlocksInRange

87a10b4

Print sync time left in human readable form

e15dd84

Remove previous initial sync code

4f57e34

Add tests for new sync utilities

b904766

dapplion requested review from mpetrunic, twoeths and wemeetagain as code owners January 17, 2021 19:35

github-actions bot added the scope-networking All issues related to networking, gossip, and libp2p. label Jan 17, 2021

Fix unnecessary math with localFinalizedEpoch - thanks @lgtm-com ;)

df85f38

twoeths reviewed Jan 18, 2021

View reviewed changes

packages/lodestar/src/sync/range/chain.ts Outdated Show resolved Hide resolved

twoeths reviewed Jan 18, 2021

View reviewed changes

packages/lodestar/src/sync/range/batches.ts Show resolved Hide resolved

Fix typo in toBeProcessedStartEpoch

5a2c58c

Add unit tests for sync/range/batches

db8c69f

ChainSafe deleted a comment from lgtm-com bot Jan 18, 2021

dapplion added 2 commits January 18, 2021 22:20

Move to verbose extremely noisy gossip logs

e7e1584

Better syncMaybeStuck accept long processing times

e097c19

dapplion force-pushed the dapplion/initial-sync-as-state-machine branch from d8e9587 to e097c19 Compare January 18, 2021 22:12

Skip initial sync if minPeers=0

35dae04

wemeetagain added this to the 0.15.0 milestone Jan 19, 2021

dapplion added 3 commits January 22, 2021 09:14

Fix getSyncStatus logic

88feee9

Update Grafana dashboard

623ee07

Reduce the chance of TimeSeries test randomly failing

dffe81b

wemeetagain reviewed Jan 25, 2021

View reviewed changes

wemeetagain approved these changes Jan 25, 2021

View reviewed changes

wemeetagain merged commit 8bb6e91 into master Jan 25, 2021

wemeetagain deleted the dapplion/initial-sync-as-state-machine branch January 25, 2021 18:28

dapplion mentioned this pull request Apr 22, 2021

Implement one-range-ahead strategy for initial sync #1921

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial sync as state machine #1961

Initial sync as state machine #1961

dapplion commented Jan 17, 2021 •

edited

codeclimate bot commented Jan 17, 2021 •

edited

twoeths commented Jan 18, 2021

twoeths Jan 18, 2021

dapplion Jan 18, 2021 •

edited

dapplion Jan 19, 2021

twoeths Jan 18, 2021

dapplion Jan 18, 2021

twoeths Jan 25, 2021

wemeetagain Jan 25, 2021

dapplion commented Jan 18, 2021

twoeths commented Jan 21, 2021

dapplion commented Jan 22, 2021 •

edited

wemeetagain Jan 25, 2021

Initial sync as state machine #1961

Initial sync as state machine #1961

Conversation

dapplion commented Jan 17, 2021 • edited

Batch

SyncChain

codeclimate bot commented Jan 17, 2021 • edited

twoeths commented Jan 18, 2021

twoeths Jan 18, 2021

Choose a reason for hiding this comment

dapplion Jan 18, 2021 • edited

Choose a reason for hiding this comment

dapplion Jan 19, 2021

Choose a reason for hiding this comment

twoeths Jan 18, 2021

Choose a reason for hiding this comment

dapplion Jan 18, 2021

Choose a reason for hiding this comment

twoeths Jan 25, 2021

Choose a reason for hiding this comment

wemeetagain Jan 25, 2021

Choose a reason for hiding this comment

dapplion commented Jan 18, 2021

twoeths commented Jan 21, 2021

dapplion commented Jan 22, 2021 • edited

wemeetagain Jan 25, 2021

Choose a reason for hiding this comment

dapplion commented Jan 17, 2021 •

edited

`Batch`

`SyncChain`

codeclimate bot commented Jan 17, 2021 •

edited

dapplion Jan 18, 2021 •

edited

dapplion commented Jan 22, 2021 •

edited