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.

Sign up for GitHub

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

Jump to bottom

sync: Remove periodic tick from SyncingEngine #5451

Open
dmitry-markin opened this issue Aug 23, 2024 · 2 comments
Open

sync: Remove periodic tick from SyncingEngine #5451

dmitry-markin opened this issue Aug 23, 2024 · 2 comments
Labels
D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue.

Comments

@dmitry-markin
Copy link
Contributor

Remove periodic tick from SyncingEngine:
https://github.com/paritytech/polkadot-sdk/blob/master/substrate/client/network/sync/src/engine.rs#L599-L604

It is not used by syncing, but without it sync tests hang forever.

See #5410 (comment) for the context.
@dmitry-markin dmitry-markin added I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue. D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. labels Aug 23, 2024
@ndkazu
Copy link
Contributor

ndkazu commented Oct 7, 2024 •
edited
Loading

Is this still a problem? I tried a cargo test in the sync folder, and also in the network folder, after removing tick_timeout.tick() from engine.rs (Lines 554~558) and tests are not hanging.
Either I'm running the wrong tests, or something in the latest merged commits fixed the issue...

@nazar-pc
Copy link
Contributor

nazar-pc commented Oct 7, 2024

There was some refactoring done that might have helped. When that comment was done a few tests were hanging, but if that is no longer the case - great!

@ndkazu ndkazu mentioned this issue Oct 8, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
D1-medium Can be fixed by a coder with good Rust knowledge but little knowledge of the codebase. I3-annoyance The node behaves within expectations, however this “expected behaviour” itself is at issue.
Projects
Networking
Status: Backlog 🗒
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nazar-pc @dmitry-markin @ndkazu