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

Fix sync pivot not set when dbload #6080

Merged
merged 1 commit into from
Sep 7, 2023
Merged

Conversation

asdacap
Copy link
Contributor

@asdacap asdacap commented Sep 7, 2023

  • Fixes random Unable to find beacon header at height 18051001. This is unexpected, forcing a new beacon sync. and Peer sent orphaned blocks/headers inside the batch.
  • When BlockTree.AcceptVisitor take too long (I don't know how @kamilchodola did that), DbLoad in MSMS will get activated and pivot updater will get deactivated and disabled. This causes it to not update sync pivot. On restart, the pivot updated will reactivate and set a different sync pivot, but blocktree already initialized various pointers with respect to old pivot causing forward sync to start from before the sync pivot which causes the errors.
  • Step to reproduce:
    • Make sure CL is synced.
    • Add Thread.Sleep(1000) in ReviewBlockTree around line 67.
    • Add Thread.Sleep(1000) in BlockTree.AcceptVisitor around line 22.
    • Probably add some sleep in FastHeaderSyncFeed to slow it down or it might complete downloading header to the hardcoded sync pivot which causes it to work.
    • Start until it start FastHeader.
    • Stop it.
    • Remove thread sleep in BlockTree.AcceptVisitor.
    • Start again.
2023-09-06 08:07:06.2098|INFO|9|Waiting for Forkchoice message from Consensus Layer to set fresh pivot block [0s] 
2023-09-06 08:07:06.5426|INFO|9|Sync mode changed from Disconnected to UpdatingPivot 
2023-09-06 08:07:06.7182|INFO|21|Skipping pivot update 
2023-09-06 08:07:06.7182|INFO|21|Sync mode changed from UpdatingPivot to DbLoad 
2023-09-06 08:07:06.7637|INFO|22|Beacon Numbers resolved, level = 0, header = 0, body = 0 
2023-09-06 08:07:06.7637|DEBUG|22|Completed visiting blocks in DB at level 18051000 - best known 0 
2023-09-06 08:07:06.8955|DEBUG|24|Step ReviewBlockTree          executed in 1151ms 
2023-09-06 08:07:06.8955|DEBUG|24|ReviewBlockTree          complete 
2023-09-06 08:07:07.7418|INFO|25|Sync mode changed from DbLoad to Disconnected 
2023-09-06 08:07:15.9050|INFO|22|Sync mode changed from Disconnected to FastHeaders 
2023-09-06 08:07:17.4703|INFO|17|Old Headers           0 / 18,051,000 (  0.00 %) | queue         0 | current            0 Blk/s | total            0 Blk/s 

// After restart
2023-09-06 08:08:17.9008|INFO|10|Sync mode changed from Disconnected to UpdatingPivot 
2023-09-06 08:14:15.3420|INFO|145|New beacon pivot: 18076068 (0xa7784ace97a902977004f8a6fe4f56610a04730e97aced32762138804cd61f9d) 
2023-09-06 08:14:18.5621|INFO|135|New pivot block has been set based on ForkChoiceUpdate from CL. Pivot block number: 18076003, hash: 0x59b05ce84a097ff7a357d8c679c4ce3e77ca4e54c8776c5da4de2020d1453766
2023-09-06 08:14:18.7270|INFO|135|Changing state UpdatingPivot to FastHeaders, BeaconHeaders at processed: 0 | state: 0 | block: 0 | header: 18051000 | target block: 18076085 | peer block: 18076152 
2023-09-06 08:14:18.7270|INFO|135|Sync mode changed from UpdatingPivot to FastHeaders, BeaconHeaders 
2023-09-06 08:14:30.6972|ERROR|98|Peer sent orphaned blocks/headers inside the batch 

Changes

  • Dont disable UpdatingPivot on dbload.

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

  • Need to test premerge network also.

@asdacap asdacap marked this pull request as ready for review September 7, 2023 03:50
@asdacap asdacap merged commit 4cdd176 into master Sep 7, 2023
61 checks passed
@asdacap asdacap deleted the fix/pivot-not-updating-on-dbload branch September 7, 2023 16:40
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.

None yet

3 participants