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: prevent race condition between block propagation and sync #3536

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Nov 4, 2021

Description

  • prevents base node from switching to sync mode before the block has a chance to arrive via propagation

Motivation and Context

Base node may switch to sync mode many times as the block arrives at peers, syncing a single block and returning to
listening state. It is more efficient to stay in listening state for a while to give the block chance to arrive via propagation.

How Has This Been Tested?

Manually tested:

  • more than 1 behind tip - sync as normal
  • startup 1 behind missed propagation, enters sync mode
  • 1 behind, prevented from entering sync mode, block arrives shortly after

image
Prevented 3 times in a few minutes on my base node.

@sdbondi sdbondi force-pushed the core-propagation-vs-sync-race-condition branch from 9ad7276 to 5fcafa9 Compare November 4, 2021 14:28
@sdbondi
Copy link
Member Author

sdbondi commented Nov 8, 2021

I've just noticed the blocks_behind_before_considered_lagging setting - think we should default this to 1 because this causes unnecessary network overhead

@stringhandler
Copy link
Collaborator

I've just noticed the blocks_behind_before_considered_lagging setting - think we should default this to 1 because this causes unnecessary network overhead

This setting is only intended to be used in debugging and tests. It should be set to 0 by default

@sdbondi
Copy link
Member Author

sdbondi commented Nov 8, 2021

There is a race condition as described in the PR which puts the base node in header sync before the block arrives via propagation.

Copy link
Collaborator

@stringhandler stringhandler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool

@sdbondi
Copy link
Member Author

sdbondi commented Nov 8, 2021

Yeah so maybe that setting has a slightly different use case. than this which would never need to wait > 1 - ok cool makes sense

@aviator-app aviator-app bot merged commit 6bbb654 into tari-project:development Nov 8, 2021
@sdbondi sdbondi deleted the core-propagation-vs-sync-race-condition branch November 9, 2021 04:51
@sdbondi sdbondi restored the core-propagation-vs-sync-race-condition branch February 3, 2022 05:29
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

2 participants