-
Notifications
You must be signed in to change notification settings - Fork 918
sync/fix: Clear gap sync on known imported blocks #8445
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
Signed-off-by: Alexandru Vasile <[email protected]>
All GitHub workflows were cancelled due to failure one of the required jobs. |
Unrelated test failing
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good. Just one more sanity check:
- When we warp sync to block x, the target of the gap sync will be block x - 1.
- There is not reasonable way that we import a known block x - 1 without completing the gap sync.
I am just wondering why it was originally done like this.
@@ -992,6 +987,18 @@ where | |||
Ok(sync) | |||
} | |||
|
|||
/// Complete the gap sync if the target number is reached and there is a gap. | |||
fn maybe_complete_gap_sync(&mut self, number: NumberFor<B>) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Maybe complete_gap_if_target
?
This PR ensures that warp sync gaps are properly cleared when known blocks are imported. Previously, gaps were only removed in response to
ImportedUnknown
events.This limitation caused issues for asset-hub and bridge-hub collators, which remained stuck in the "Block history" state without progressing.
The root cause lies in the client.info() reporting a gap during node startup or restart (ie block verification fails). In some cases, a peer may respond with the missing blocks after we’ve already imported them locally, leaving the gap open.
Grafana link: https://grafana.teleport.parity.io/goto/jCcsBLxNg?orgId=1
Traces from production:
Testing Done
Added two tests to verify that warp sync gaps are correctly cleared under both block import scenarios. The first test closely follows the operations performed by the node, while the second one emulates the imports.
Next Steps
Added extra debug logs to monitor if the issue persists (pointing towards a corupt database -- ie client.info() always has the gap present).
Closes: #8416
cc @paritytech/networking