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

Refactor Sync state logic #1614

Closed
AgeManning opened this issue Sep 11, 2020 · 2 comments
Closed

Refactor Sync state logic #1614

AgeManning opened this issue Sep 11, 2020 · 2 comments
Assignees
Labels
consensus An issue/PR that touches consensus code, such as state_processing or block verification. enhancement New feature or request

Comments

@AgeManning
Copy link
Member

Description

The logic of calculating the node's current sync is disjoint and exists partly in NetworkGlobals, partly in SyncManager and partly in RangeSync. It makes it difficult to reason about the global sync state of the node and perform upgrades such as #1613.

Instead of refactoring this sync logic, I've decided to keep it as is and make this issue, as there is a sync update PR coming from @divagant-martian and I wanted minimal conflicts.

We are considered sync'd if we are not performing any range sync, and all our peers are considered sync'd with our current head. The split logic comes due to the RangeSync managing its own view of things. There are a number of events that can cause RangeSync to update the sync state, such as RPC errors, peer disconnects, block processing etc. Because of this, the current design was to let RangeSync handle its own sync status.

Perhaps a better solution is to allow the SyncManager to be the source of truth. On all messages it passes to range sync, it could ask RangeSync what it's status is and then based on the response optionally update the global sync status.

If someone takes up this issue, we can discuss the logic of how the global sync status is calculated in this issue :).

@AgeManning AgeManning added enhancement New feature or request A1 labels Sep 11, 2020
@divagant-martian
Copy link
Collaborator

starting with this one

@divagant-martian divagant-martian self-assigned this Sep 29, 2020
bors bot pushed a commit that referenced this issue Oct 20, 2020
## Issue Addressed
#1614 and a couple of sync-stalling problems, the most important is a cyclic dependency between the sync manager and the peer manager
@divagant-martian
Copy link
Collaborator

closed by #1791

@michaelsproul michaelsproul added the consensus An issue/PR that touches consensus code, such as state_processing or block verification. label Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus An issue/PR that touches consensus code, such as state_processing or block verification. enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants