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): adds extra checks for sync stream termination #3927

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Mar 17, 2022

Description

  • adds extra checks in block sync RPC service for premature stream termination
  • uses rolling average for slow peer detection in header and horizon sync
  • prevent more than one UTXO sync session per peer

Motivation and Context

Peers may end a sync session and quickly initiate a new one. Because the previous session could be still sending the batch of data for the previous session, the new session will be denied until the data has been fetched. This PR adds additional checks after loading data from the database but before sending it to check if the session has ended. However, this race condition still exists as it is inherent to the rule of only allowing one sync session per peer. In my tests with 600ms max permitted sync latency I did not manage to trigger the Forbidden: Existing sync session found for this client. Only a single session is permitted error, so this may still be an issue. But these changes should lessen the chance of this happening.

How Has This Been Tested?

Manually, existing tests

@sdbondi sdbondi force-pushed the core-sync-session-ends-correctly-on-timeout branch from 4c24900 to 0658f64 Compare March 17, 2022 15:54
SWvheerden
SWvheerden previously approved these changes Mar 23, 2022
Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

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

LGTM
Tested with a sync from 0 up to the tip.

@stringhandler stringhandler merged commit dd544cb into tari-project:development Mar 25, 2022
@sdbondi sdbondi deleted the core-sync-session-ends-correctly-on-timeout branch March 25, 2022 12:36
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