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

Stop mux incase termination of miniprotocols timeout #3575

Merged
merged 3 commits into from Jan 18, 2022

Conversation

karknu
Copy link
Contributor

@karknu karknu commented Jan 17, 2022

When demoting a peer to cold there is a 300s timeout for the
miniprotocols to terminate. In case of timeout the connection will be
left hanging, it will only be unregistered. The next time the
peer is promoted to warm there may still be lingering miniprotocols in
an unknown state.

By calling stopMux we ensure that the connection will be
reset.

@karknu karknu requested a review from coot as a code owner January 17, 2022 08:47
@karknu karknu requested a review from coot January 17, 2022 10:12
Copy link
Contributor

@coot coot left a comment

Choose a reason for hiding this comment

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

LGTM

When demoting a peer to cold there is a 300s timeout for the
miniprotocols to terminate. In case of timeout the connection will be
left hanging, it will only be unregistered. The next time the
peer is promoted to warm there may still be lingering miniprotocols in
an unknown state.

By calling stopMux we ensure that the connection will be
reset.
unregisterOutboundConnection isn't needed after stopMux.
If keepalive or blockfetch's unregister function is interrupted it will
cause some state to be left. This will result in a space leak and an
assertion failure the next time the node attempt to talk to that peer.

Change their unregister functions to run inside uninterruptibleMask_.
@karknu karknu force-pushed the karknu/peerstateaction_timeout branch from a3aa9bf to 51fb6f4 Compare January 18, 2022 13:49
@coot
Copy link
Contributor

coot commented Jan 18, 2022

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jan 18, 2022

Build succeeded:

@iohk-bors iohk-bors bot merged commit aca8a53 into master Jan 18, 2022
@iohk-bors iohk-bors bot deleted the karknu/peerstateaction_timeout branch January 18, 2022 15:46
@karknu
Copy link
Contributor Author

karknu commented Jan 20, 2022

Fixes #3570

karknu added a commit that referenced this pull request Jan 21, 2022
3575: Stop mux incase termination of miniprotocols timeout r=coot a=karknu

When demoting a peer to cold there is a 300s timeout for the
miniprotocols to terminate. In case of timeout the connection will be
left hanging, it will only be unregistered. The next time the
peer is promoted to warm there may still be lingering miniprotocols in
an unknown state.

By calling stopMux we ensure that the connection will be
reset.

Co-authored-by: Karl Knutsson <karl.knutsson@iohk.io>
@karknu karknu mentioned this pull request Jan 21, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants