-
Notifications
You must be signed in to change notification settings - Fork 166
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
[AccessNode] Disable connection pruning for staked AN that supports unstaked AN #1254
Conversation
vishalchangrani
commented
Sep 4, 2021
- Adds back the peer manager to the Staked AN which supports the unstaked AN.
- Allows the peer manager to skip connection pruning optionally.
…aked AN; adding option to disable connection pruning
@@ -572,29 +572,6 @@ func (builder *FlowAccessNodeBuilder) extraFlags() { | |||
}) | |||
} | |||
|
|||
// initMiddleware creates the network.Middleware implementation with the libp2p factory function, metrics, peer update | |||
// interval, and validators. The network.Middleware is then passed into the initNetwork function. | |||
func (builder *FlowAccessNodeBuilder) initMiddleware(nodeID flow.Identifier, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Stake AN and the UAN no longer share the common middleware that was being generated by this function. Instead they each have their own version of this method - one with peer manager and one without.
validators ...network.MessageValidator) network.Middleware { | ||
|
||
// disable connection pruning for the staked AN which supports the unstaked AN | ||
peerManagerFactory := p2p.PeerManagerFactory([]p2p.Option{p2p.WithInterval(builder.PeerUpdateInterval)}, p2p.DisableConnectionPruning()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use the stock PeerManager but don't prune connections
backoffConnector: connector, | ||
host: host, | ||
log: log, | ||
}, nil | ||
pruneConnections: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
default is to always prune
// connect to each of the peer.AddrInfo in pInfos | ||
l.connectToPeers(ctx, peerIDs) | ||
|
||
// disconnect from any other peers not in pInfos | ||
l.trimAllConnectionsExcept(peerIDs) | ||
if l.pruneConnections { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
THE check
Codecov Report
@@ Coverage Diff @@
## master #1254 +/- ##
=======================================
Coverage 56.18% 56.18%
=======================================
Files 496 496
Lines 30179 30190 +11
=======================================
+ Hits 16956 16963 +7
- Misses 10921 10924 +3
- Partials 2302 2303 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a bunch! LGTM
network/p2p/libp2pConnector.go
Outdated
func newLibp2pConnector(host host.Host, log zerolog.Logger) (*libp2pConnector, error) { | ||
type ConnectorOption func(connector *Libp2pConnector) | ||
|
||
func DisableConnectionPruning() ConnectorOption { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: WithConnectionPruning(b bool)
?
closes 5831 (https://github.com/dapperlabs/flow-go/issues/5831) |
} | ||
|
||
var _ Connector = &libp2pConnector{} | ||
var _ Connector = &Libp2pConnector{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vishalchangrani what is the purpose of this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a compile time check to make sure that Libp2pConnector implements the Connector interface
network/p2p/middleware.go
Outdated
@@ -159,7 +161,7 @@ func DefaultValidators(log zerolog.Logger, flowID flow.Identifier) []network.Mes | |||
} | |||
} | |||
|
|||
func (m *Middleware) topologyPeers() (peer.IDSlice, error) { | |||
func (m *Middleware) TopologyPeers() (peer.IDSlice, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need to make this public?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't ..good catch. switched it back.
bors merge |