server: improve initial peer bootstrapping #1205
Conversation
meshcollider
added
the
discovery
server.go
Outdated
| // new connection via our public endpoint, which will require the lock | ||
| // an add the peer to the server's internal state. | ||
| if done != nil { | ||
| done <- struct{}{} |
There was a problem hiding this comment.
you can close the channel instead, which will never block :)
server.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| // ignore is a set used to keep track of peers already retrieved | ||
| // from our bootstrappers in order to avoid duplicates. | ||
| ignore := make(map[autopilot.NodeID]struct{}) |
There was a problem hiding this comment.
this can be initialized inside initialPeerBootstrap ?
There was a problem hiding this comment.
Yep, forgot to move this. Fixed.
server.go
Outdated
| // quickly by discarding peers that are slowing | ||
| // us down. | ||
| select { | ||
| case <-done: |
There was a problem hiding this comment.
If the connection fails, then this won't be sent on, and we will wait the 3 seconds before trying the next peers. Should we isntead make this channel chan error, where we can send an error in case it fails, and close in case it succeeds?
wpaulino
force-pushed
the
initial-peer-bootstrap
branch
from
6b23ef7
to
8cffbf5
Compare
|
Needs a rebase to fix conflicts as well, also we can manually implement the shuffle using |
wpaulino
force-pushed
the
initial-peer-bootstrap
branch
2 times, most recently
from
c216ba5
to
8826b8d
Compare
|
Rebased. |
server.go
Outdated
| // quickly by discarding peers that are slowing | ||
| // us down. | ||
| select { | ||
| case <-errChan: |
server.go
Outdated
| case <-errChan: | ||
| // TODO: tune timeout? 3 seconds might be *too* | ||
| // aggressive but works well. | ||
| case <-time.After(3 * time.Second): |
There was a problem hiding this comment.
handle the timeout somehow? At least log.
There was a problem hiding this comment.
The timeout doesn't need to be handled since we'll just continue to the next peer. Added a trace log though.
server.go
Outdated
| errChan := make(chan error, 1) | ||
| s.connectToPeer(a, errChan) | ||
| select { | ||
| case <-errChan: |
| select { | ||
| case err := <-errChan: | ||
| return err | ||
| case <-s.quit: |
There was a problem hiding this comment.
can also return ErrServerShuttingDown in this case
In this commit, we address an existing issue with regards to the inital peer bootstrapping stage. At times, the bootstrappers can be unreliable by providing addresses for peers that no longer exist/are currently offline. This would lead to nodes quickly entering an exponential backoff method used to maintain a minimum target of peers without first achieving said target. We address this by separating the peer bootstrapper into two stages: the initial peer bootstrapping and maintaining a target set of nodes to maintain an up-to-date view of the network. The initial peer bootstrapping stage has been made aggressive in order to provide such view of the network as quickly as possible. Once done, we continue on with the existing exponential backoff method responsible for maintaining a target set of nodes.
In this commit, we randomize the order of the different bootstrappers in order to prevent from always querying potentially unreliable bootstrappers first.
wpaulino
force-pushed
the
initial-peer-bootstrap
branch
from
8826b8d
to
800b136
Compare
In this commit, we address an existing issue with regards to the inital
peer bootstrapping stage. At times, the bootstrappers can be unreliable
by providing addresses for peers that no longer exist/are currently
offline. This would lead to nodes quickly entering an exponential
backoff method used to maintain a minimum target of peers without first
achieving said target.
We address this by separating the peer bootstrapper into two stages: the
initial peer bootstrapping and maintaining a target set of nodes to
maintain an up-to-date view of the network. The initial peer
bootstrapping stage has been made aggressive in order to provide such
view of the network as quickly as possible. Once done, we continue on
with the existing exponential backoff method responsible for maintaining
a target set of nodes.
We also randomize the order of the different bootstrappers to prevent
from always querying potentially unreliable bootstrappers first.