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

Add additional block lag for better sync mode switching for chain with shorter block processing time #7892

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

damian-orzechowski
Copy link
Contributor

@damian-orzechowski damian-orzechowski commented Dec 11, 2024

For linea (and probably other chains with short block time), there is a problem with switching between various phases of sync causing node to fall into a long running loop, though could already start processing blocks. The problem occurs for:

  • switching from fast headers to state sync (snap)
  • switching from state sync to block processing

In the case for linea (no beacon), best header is set based on latest downloaded header from peer. Target block is assumed to be best peer's head. When headers are downloaded, the FastSyncLag (32 blocks) is used as a distance from the target, so best header is always minimum 32 blocks behind target. Conditions for switching between fast sync, state sync and block processing also use the criteria of having block header >= target - FastSyncLag, which effectively means some transitions can only happen when header is exactly 32 blocks behind target. Similar case is with target state sync block which is set to best header upon setting pivot for sync. With 2s block processing time, chain can advance fast enough for this condition never to be fulfilled considering MulitSyncModeSelected check frequency (1s).

Changes

The fix is to add additional lag to be used when checking sync transition to allow more distance between target and header / state, so that MulitSyncModeSelected can evaluate the change conditions even when chain is progressing fast.

TODO - move this new lag to config and set only for linea?

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • [] Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

Optional. Remove if not applicable.

Documentation

Requires documentation update

  • Yes
  • No

If yes, link the PR to the docs update or the issue with the details labeled docs. Remove if not applicable.

Requires explanation in Release Notes

  • Yes
  • No

If yes, fill in the details here. Remove if not applicable.

Remarks

Optional. Remove if not applicable.

@LukaszRozmej
Copy link
Member

LukaszRozmej commented Dec 11, 2024

Maybe just make FastSyncLag configurable?
We could similarly make check frequency configurable.
Or we could make them dependent on SecondsPerSlot

@damian-orzechowski
Copy link
Contributor Author

Maybe just make FastSyncLag configurable? We could similarly make check frequency configurable. Or we could make them dependent on SecondsPerSlot

Changing FastSyncLag alone won't have an impact, because it is used for both downloading header and checking sync switch transition. We'll end up with the same - only switch when header == target - FastSyncLag.

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.

2 participants