server: cleanup persistent connection retries #982
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…fallback to default port In this commit, we fix an existing bug within the codebase: if a peer connected to us inbound, then we'd attempt to use the assigned port when re-establishing a connection to them. We fix this issue in this commit by adding a new method to look up any advertisements for the peer, and use the specified port that matches our connection attempt. If we can't find a proper advertisement, then we'll simply use the default peer port.
aakselrod
previously approved these changes
Mar 31, 2018
In this commit, we fix a minor bug in the prior versions of lnd. Before this commit, if we received a new inbound connection for channel creation, the channel was created, and then the peer disconnected, we wouldn't automatically reconnect. In this commit we fix this issue by overloading the WatchNewChannel method to also accept the peer's ID so we can add it to the set of persistent connections.
cfromknecht
force-pushed
the
server-conns
branch
from
bc13190 to
ffa2a9e
Compare
rodasmith
reviewed
Mar 31, 2018
lnd.go
Outdated
| addr *lnwire.NetAddress) error { | ||
|
|
||
| // First, we'll mark this new peer as a persistent | ||
| // re-connection purposes. |
There was a problem hiding this comment.
(nit) comment typo "... as a persistent [peer for] re-connection purposes."
cfromknecht
force-pushed
the
server-conns
branch
2 times, most recently
from
dba8c5a to
d0a4231
Compare
This commits changes the behavior of our connection reestablishment, and resolves some minor issues that could lead to uncancelled requests or an infinite connection loop. - Will not attempt to Remove connection requests with an ID of 0. This can happen for reconnect attempts that get scheduled, but have not started at the time the server cancels the connection requests. - Adds a per-peer cancellation channel, that is closed upon a successful inbound or outbound connection. The goroutine spwaned to handle the reconnect by the peerTerminationWatch now selects on this channel, and skips reconnecting if it is closed before the backoff matures. - Properly computes the backoff when no entry in persistentPeersBackoff is found. Previously, a value of 0 would be returned, cause all subsequent backoff attempts to use a backoff of 0. - Cancels a peers retries and remove connections immediately after receiving an inbound connection, to mimic the structure of OutboundPeerConnected. - Cancels all persistent connection requests after calling DisconnectPeers. - Allow additional connection attempts to peers, even if there already exists a pending connection attempt.
Alters the behavior of ConnectEnsure to initiate a connection attempt in both directions. Additionally, the wait predicate only returns true after cross checking both peer lists.
cfromknecht
force-pushed
the
server-conns
branch
2 times, most recently
from
6a5f46b to
c8323a4
Compare
Uses EnsureConnected when reconnecting nodes in: - switch offline delivery persistence - graph topology notifications - channel funding persistence
cfromknecht
force-pushed
the
server-conns
branch
from
c8323a4 to
7db7c9c
Compare
This was referenced Mar 31, 2018
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Complementary to #981.
This commits changes the behavior of our connection
reestablishment, and resolves some minor issues that
could lead to uncancelled requests or an infinite
connection loop.
Will not attempt to Remove connection requests with
an ID of 0. This can happen for reconnect attempts
that get scheduled, but have not started at the
time the server cancels the connection requests.
Adds a per-peer cancellation channel, that is
closed upon a successful inbound or outbound
connection. The goroutine spwaned to handle the
reconnect by the peerTerminationWatch now
selects on this channel, and skips reconnecting
if it is closed before the backoff matures.
Properly computes the backoff when no entry in
persistentPeersBackoff is found. Previously, a
value of 0 would be returned, cause all subsequent
backoff attempts to use a backoff of 0.
Cancels a peer's retries and remove connections
immediately after receiving an inbound connection,
to mimic the structure of OutboundPeerConnected.
Fixes #935, #941, #979, #982.