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

WIP: Introduce PeerConnManager to manage peer connections #7283

Draft
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

yyforyongyu
Copy link
Member

This PR attempts to fix #6788 by first moving the peer connection-related code into its dedicated package. The moving process is surprisingly smooth as the peer conn manager is already hidden in server.go, we only need to peel it off from the file and make it a dedicated service.

Commits are made in a way so they're easy to follow and digest, let me know if they are too small.

TODO:

  • patch unit tests

@Crypt-iQ
Copy link
Collaborator

Crypt-iQ commented Jan 3, 2023

I think this could build off of #5700 ?

@Roasbeef
Copy link
Member

Roasbeef commented Jan 3, 2023

Yeah was going to mention #5700 here, cc @ellemouton

@yyforyongyu
Copy link
Member Author

@Crypt-iQ and @Roasbeef this PR only moves code from server.go to peerconn.go, no functionality changes. So I guess #5700 can be built upon this one, if still relevant.

Also add start and stop methods for peer conn manager.
This commit also moves the initialization of `PeerConnManager` after
channel router is created.
This commit moves the shutdown process of `connMgr` into the peer conn
manager's shutdown. It also moves the peer disconnection inside peer
conn manager's shutdown to make sure the disconnection is successful.
Also move some of the fields from `PeerConnManager` to
`PeerConnManagerConfig`.
This commit moves the peer conn manager related code to
`conn_manager.go` and updates the error used. Changes inside `server.go`
is a pure code moving.
This commit renames `PeerConnManager` and `PeerConnManagerConfig` to
`Manager` and `ManagerConfig` by the request of the linter `revive`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Handle race condition in peer connection
4 participants