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 edge case where state sync is not triggered #5635

Merged
merged 6 commits into from
Sep 10, 2024

Conversation

liuchengxu
Copy link
Contributor

This PR addresses an issue where state sync may fail to start if the conditions required for its initiation are not met when a finalized block notification is received. pending_state_sync_attempt is introduced to trigger the state sync later when the conditions are satisfied.

This issue was spotted when I worked on #5406, specifically, queue_blocks was not empty when the finalized block notification was received, and then the state sync was stalled. cc @dmitry-markin

Pure refactoring, zero logical changes.
There is an edge case where the finalized block notification
is received, but the conditions required to initiate the state sync are
not fully met. In such cases, state sync would fail to start as expected
and remain stalled.

This fixes it by storing the pending attempt and trying to start it
later.
Copy link
Contributor

@dmitry-markin dmitry-markin left a comment

Choose a reason for hiding this comment

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

Nicely done, thank you!

@dmitry-markin dmitry-markin requested a review from bkchr September 9, 2024 08:40
@dmitry-markin dmitry-markin added T0-node This PR/Issue is related to the topic “node”. I2-bug The node fails to follow expected behavior. labels Sep 9, 2024
@bkchr bkchr enabled auto-merge September 10, 2024 07:29
auto-merge was automatically disabled September 10, 2024 08:11

Head branch was pushed to by a user without write access

@liuchengxu
Copy link
Contributor Author

@bkchr Please click the button again, fmt wasn't good in the applied suggestion :(

@dmitry-markin dmitry-markin added this pull request to the merge queue Sep 10, 2024
Merged via the queue into paritytech:master with commit 8236718 Sep 10, 2024
201 of 203 checks passed
nazar-pc pushed a commit to autonomys/polkadot-sdk that referenced this pull request Sep 27, 2024
This PR addresses an issue where state sync may fail to start if the
conditions required for its initiation are not met when a finalized
block notification is received. `pending_state_sync_attempt` is
introduced to trigger the state sync later when the conditions are
satisfied.

This issue was spotted when I worked on paritytech#5406, specifically,
`queue_blocks` was not empty when the finalized block notification was
received, and then the state sync was stalled. cc @dmitry-markin

---------

Co-authored-by: Dmitry Markin <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
(cherry picked from commit 8236718)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I2-bug The node fails to follow expected behavior. T0-node This PR/Issue is related to the topic “node”.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants