multi+refactor: persistent peer manager

[ellemouton]

This PR adds a PersistentPeerManager to the server and refactors all persistent peer logic to be handled by the PersistentPeerManager. The end result is that persistentPeers, persistentPeersBackoff, persistentConnReqs and persistentRetryCancels are all removed from the server struct and replaced by a PersistentPeerManager.

[orijbot]

Visit https://dashboard.github.orijtech.com?back=0&pr=5700&remote=true&repo=ellemouton%2Flnd to see benchmark details.

[ellemouton]

def involves more work tho

Mind elaborating? Do you mean in general or specifically in this PR?

My question is, since we are already here, can we move all connection management into peer?

Very happy to do so! Do you think it should all be in 1 PR?

[yyforyongyu]

Mind elaborating? Do you mean in general or specifically in this PR?

Meant in this PR. Since we are moving the code outside of server.go, it's finally a good chance to patch the missing unit tests and refactor some of them if needed. But yeah, it would involve more work.

Very happy to do so! Do you think it should all be in 1 PR?

It depends. I'd think about how emergent issue #5377 is. If it's a pressing issue, I'd suggest we move to #5538 and land it first, since it's almost done and has been reviewed. If not, we might as well take the chance to refactor the connection manager here, moving it into its own package. This will def take more time as the scale is large, but we should do it imo. Another option is to continue what you have here, that we move out the persistent manager as the first step. Guess it's a judgment call from @Roasbeef .

[viaj3ro]

If it's a pressing issue, I'd suggest we move to #5538 and land it first, since it's almost done and has been reviewed

I'd argue it is pressing. I still have a handful of TOR peers that refuse to connect back to me even after almost 4 months. Since many nodes have IP address changes once in a while, I assume they have the same issue without ever noticing it.

[ellemouton]

holding off on this PR until 0.15. Working on the solution in #5538 again instead 👍

[hsjoberg]

I'd also argue it's quite pressing, I have several users of my wallet which have encountered this issue.

[Roasbeef]

@hsjoberg a simpler version of this was merged in https://github.com/lightningnetwork/lnd/pull/5538

[hsjoberg]

@Roasbeef Thanks! I'm aware that the other PR got into master. 💯

[ellemouton]

!lightninglabs-deploy mute 2022-Jan-01

[yyforyongyu]

Is this ready for another round?

[ellemouton]

@yyforyongyu, yes it is ready :)

[ellemouton]

Hi @joostjager ! Thanks so much for the review on this! Im gonna mute the bot on this PR for a bit just while 0.15 is top prio & will address your review as soon as i can 👍

[ellemouton]

I will push an update addressing all the comments after the review club tomorrow (I decided to change the structure quite a bit so I don't want to push it before the review club in case people are already familiar with the old structure)

[ellemouton]

Ok, this is finally ready for re-review :) I have restructured the commits significantly which will hopefully make review easier

[lightninglabs-deploy]

@bhandras: review reminder @roasbeef: review reminder @yyforyongyu: review reminder @ellemouton, remember to re-request review from reviewers when ready

[lightninglabs-deploy]

@bhandras: review reminder @roasbeef: review reminder @yyforyongyu: review reminder @ellemouton, remember to re-request review from reviewers when ready

[lightninglabs-deploy]

@bhandras: review reminder @roasbeef: review reminder @yyforyongyu: review reminder @ellemouton, remember to re-request review from reviewers when ready

[ellemouton]

!lightninglabs-deploy mute

[ellemouton]

muting for a bit. Will pick this up again when my plate clears up a bit

[ellemouton]

closing for now. Can re-open once re-prio'd