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

Reintroduce network starter #7007

Closed
wants to merge 5 commits into from

Conversation

dmitry-markin
Copy link
Contributor

@dmitry-markin dmitry-markin commented Dec 27, 2024

This reverts #6400 as we are not ready yet to remove the network starter. Network starting faster than other tasks leads to race conditions, namely #6573 (comment).

The comment about why we need network starter is updated to reflect the current state of affairs.

@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 Dec 27, 2024
@dmitry-markin dmitry-markin requested a review from bkchr December 27, 2024 12:36
@bkchr
Copy link
Member

bkchr commented Dec 27, 2024

Can we not solve this issue differently? I would propose we introduce a NetworkState (or better name) variant to SyncEvent. This returns all the connected peers and is pushed to the event stream as the first event. This way grandpa et all will not miss any connected peers.

@dmitry-markin
Copy link
Contributor Author

Yes, I like the idea of proactively pushing the state as the first message. This should do the trick. Thanks for the suggestion!

@dmitry-markin dmitry-markin marked this pull request as draft December 27, 2024 15:01
@bkchr bkchr deleted the dm-reintroduce-network-starter branch December 28, 2024 19:57
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
Status: Blocked ⛔️
Development

Successfully merging this pull request may close these issues.

2 participants