Skip to content

Commit

Permalink
Cherry pick new dial scheduler from upstream (ethereum#1192)
Browse files Browse the repository at this point in the history
* celo-blockchain cherry-pick from upstream: p2p: new dial scheduler (ethereum#20592)

Conflicts:
  p2p/dial.go
  p2p/server.go
  p2p/server_test.go

* p2p: new dial scheduler

This change replaces the peer-to-peer dial scheduler with a new and
improved implementation. The new code is better than the previous
implementation in two key aspects:

- The time between discovery of a node and dialing that node is
  significantly lower in the new version. The old dialState kept
  a buffer of nodes and launched a task to refill it whenever the buffer
  became empty. This worked well with the discovery interface we used to
  have, but doesn't really work with the new iterator-based discovery
  API.

- Selection of static dial candidates (created by Server.AddPeer or
  through static-nodes.json) performs much better for large amounts of
  static peers. Connections to static nodes are now limited like dynanic
  dials and can no longer overstep MaxPeers or the dial ratio.

* p2p/simulations/adapters: adapt to new NodeDialer interface

* p2p: re-add check for self in checkDial

* p2p: remove peersetCh

* p2p: allow static dials when discovery is disabled

* p2p: add test for dialScheduler.removeStatic

* p2p: remove blank line

* p2p: fix documentation of maxDialPeers

* p2p: change "ok" to "added" in static node log

* p2p: improve dialTask docs

Also increase log level for "Can't resolve node"

* p2p: ensure dial resolver is truly nil without discovery

* p2p: add "looking for peers" log message

* p2p: clean up Server.run comments

* p2p: fix maxDialedConns for maxpeers < dialRatio

Always allocate at least one dial slot unless dialing is disabled using
NoDial or MaxPeers == 0. Most importantly, this fixes MaxPeers == 1 to
dedicate the sole slot to dialing instead of listening.

* p2p: fix RemovePeer to disconnect the peer again

Also make RemovePeer synchronous and add a test.

* p2p: remove "Connection set up" log message

* p2p: clean up connection logging

We previously logged outgoing connection failures up to three times.

- in SetupConn() as "Setting up connection failed addr=..."
- in setupConn() with an error-specific message and "id=... addr=..."
- in dial() as "Dial error task=..."

This commit ensures a single log message is emitted per failure and adds
"id=... addr=... conn=..." everywhere (id= omitted when the ID isn't
known yet).

Also avoid printing a log message when a static dial fails but can't be
resolved because discv4 is disabled. The light client hit this case all
the time, increasing the message count to four lines per failed
connection.

* p2p: document that RemovePeer blocks

* les: add bootstrap nodes as initial discoveries (ethereum#20688)

* celo-blockchain addition: make static dials exempt from the maxDialPeers limit

Static peers are ones we explicitly want to connect to, and so should not be subject to this limit. This is especially important since connections between validators/proxies and other validators/proxies rely on this being so, and because that's how it was prior to the new dial scheduler from upstream.

* celo-blockchain addition: Add tests for two untested RemovePeer() scenarios

Co-authored-by: Felix Lange <[email protected]>
  • Loading branch information
Or Neeman and fjl authored Oct 27, 2020
1 parent c8dcba7 commit 903d1b0
Show file tree
Hide file tree
Showing 9 changed files with 1,272 additions and 1,027 deletions.
13 changes: 13 additions & 0 deletions les/serverpool.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,19 @@ func (pool *serverPool) start(server *p2p.Server, topic discv5.Topic) {
pool.checkDial()
pool.wg.Add(1)
go pool.eventLoop()

// Inject the bootstrap nodes as initial dial candiates.
pool.wg.Add(1)
go func() {
defer pool.wg.Done()
for _, n := range server.BootstrapNodes {
select {
case pool.discNodes <- n:
case <-pool.closeCh:
return
}
}
}()
}

func (pool *serverPool) stop() {
Expand Down
Loading

0 comments on commit 903d1b0

Please sign in to comment.