From 7c6788c7a206f1d1978e178394eb592f65b76e47 Mon Sep 17 00:00:00 2001 From: alrevuelta Date: Thu, 29 Jun 2023 16:04:24 +0200 Subject: [PATCH 1/4] chore(networking): leave room for service peers --- waku/v2/node/peer_manager/peer_manager.nim | 31 +++++++++++----------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/waku/v2/node/peer_manager/peer_manager.nim b/waku/v2/node/peer_manager/peer_manager.nim index 6289d80719..05b1b91164 100644 --- a/waku/v2/node/peer_manager/peer_manager.nim +++ b/waku/v2/node/peer_manager/peer_manager.nim @@ -56,7 +56,7 @@ const PrunePeerStoreInterval = chronos.minutes(5) # How often metrics and logs are shown/updated - LogAndMetricsInterval = chronos.seconds(60) + LogAndMetricsInterval = chronos.minutes(3) # Max peers that we allow from the same IP ColocationLimit = 5 @@ -71,7 +71,8 @@ type storage: PeerStorage serviceSlots*: Table[string, RemotePeerInfo] maxRelayPeers*: int - outPeersTarget*: int + outRelayPeersTarget: int + inRelayPeersTarget: int ipTable*: Table[string, seq[PeerId]] colocationLimit*: int started: bool @@ -376,12 +377,15 @@ proc new*(T: type PeerManager, maxBackoff=backoff raise newException(Defect, "Max backoff time can't be over 1 week") + let outRelayPeersTarget = max(maxRelayPeers div 3, 10) + let pm = PeerManager(switch: switch, peerStore: switch.peerStore, storage: storage, initialBackoffInSec: initialBackoffInSec, backoffFactor: backoffFactor, - outPeersTarget: max(maxConnections div 2, 10), + outRelayPeersTarget: outRelayPeersTarget, + inRelayPeersTarget: maxRelayPeers - outRelayPeersTarget, maxFailedAttempts: maxFailedAttempts, colocationLimit: colocationLimit, maxRelayPeers: maxRelayPeers) @@ -571,16 +575,12 @@ proc connectToRelayPeers*(pm: PeerManager) {.async.} = let (inRelayPeers, outRelayPeers) = pm.connectedPeers(WakuRelayCodec) let maxConnections = pm.switch.connManager.inSema.size let totalRelayPeers = inRelayPeers.len + outRelayPeers.len - let inPeersTarget = maxConnections - pm.outPeersTarget - - if inRelayPeers.len > inPeersTarget: - await pm.pruneInRelayConns(inRelayPeers.len-inPeersTarget) + let inPeersTarget = maxConnections - pm.outRelayPeersTarget - if outRelayPeers.len >= pm.outPeersTarget: - return + if inRelayPeers.len > pm.inRelayPeersTarget: + await pm.pruneInRelayConns(inRelayPeers.len - pm.inRelayPeersTarget) - # Leave some room for service peers - if totalRelayPeers >= (maxConnections - 5): + if outRelayPeers.len >= pm.outRelayPeersTarget: return let notConnectedPeers = pm.peerStore.getNotConnectedPeers().mapIt(RemotePeerInfo.init(it.peerId, it.addrs)) @@ -670,15 +670,14 @@ proc logAndMetrics(pm: PeerManager) {.async.} = let (inRelayPeers, outRelayPeers) = pm.connectedPeers(WakuRelayCodec) let maxConnections = pm.switch.connManager.inSema.size let totalRelayPeers = inRelayPeers.len + outRelayPeers.len - let inPeersTarget = maxConnections - pm.outPeersTarget let notConnectedPeers = pm.peerStore.getNotConnectedPeers().mapIt(RemotePeerInfo.init(it.peerId, it.addrs)) let outsideBackoffPeers = notConnectedPeers.filterIt(pm.canBeConnected(it.peerId)) + let totalConnections = pm.switch.connManager.getConnections().len info "Relay peer connections", - inRelayConns = $inRelayPeers.len & "/" & $inPeersTarget, - outRelayConns = $outRelayPeers.len & "/" & $pm.outPeersTarget, - totalRelayConns = totalRelayPeers, - maxConnections = maxConnections, + inRelayConns = $inRelayPeers.len & "/" & $pm.inRelayPeersTarget, + outRelayConns = $outRelayPeers.len & "/" & $pm.outRelayPeersTarget, + totalConnections = $totalConnections & "/" & $maxConnections, notConnectedPeers = notConnectedPeers.len, outsideBackoffPeers = outsideBackoffPeers.len From 14811865136e15f5c2f8c58682842ac2e2a2c3dd Mon Sep 17 00:00:00 2001 From: alrevuelta Date: Fri, 30 Jun 2023 09:27:05 +0200 Subject: [PATCH 2/4] infer maxRelayPeers from maxConnections if not set --- apps/wakunode2/app.nim | 2 +- apps/wakunode2/external_config.nim | 3 +- waku/v2/node/builder.nim | 9 +----- waku/v2/node/peer_manager/peer_manager.nim | 36 +++++++++++++--------- 4 files changed, 24 insertions(+), 26 deletions(-) diff --git a/apps/wakunode2/app.nim b/apps/wakunode2/app.nim index d5f8d3dcfc..fee3861ce1 100644 --- a/apps/wakunode2/app.nim +++ b/apps/wakunode2/app.nim @@ -291,7 +291,7 @@ proc initNode(conf: WakuNodeConf, sendSignedPeerRecord = conf.relayPeerExchange, # We send our own signed peer record when peer exchange enabled agentString = some(conf.agentString) ) - builder.withPeerManagerConfig(maxRelayPeers = some(conf.maxRelayPeers.int)) + builder.withPeerManagerConfig(maxRelayPeers = conf.maxRelayPeers) node = ? builder.build().mapErr(proc (err: string): string = "failed to create waku node instance: " & err) diff --git a/apps/wakunode2/external_config.nim b/apps/wakunode2/external_config.nim index c84bf39e76..74c1b5dcdb 100644 --- a/apps/wakunode2/external_config.nim +++ b/apps/wakunode2/external_config.nim @@ -95,8 +95,7 @@ type maxRelayPeers* {. desc: "Maximum allowed number of relay peers." - defaultValue: 50 - name: "max-relay-peers" }: uint16 + name: "max-relay-peers" }: Option[int] peerStoreCapacity* {. desc: "Maximum stored peers in the peerstore." diff --git a/waku/v2/node/builder.nim b/waku/v2/node/builder.nim index 94f86abc4e..2d301e7163 100644 --- a/waku/v2/node/builder.nim +++ b/waku/v2/node/builder.nim @@ -145,13 +145,6 @@ proc build*(builder: WakuNodeBuilder): Result[WakuNode, string] = if builder.record.isNone(): return err("node record is required") - # fallbck to max connections if not set - var maxRelayPeers: int - if builder.maxRelayPeers.isNone(): - maxRelayPeers = builder.switchMaxConnections.get(builders.MaxConnections) - else: - maxRelayPeers = builder.maxRelayPeers.get() - var switch: Switch try: switch = newWakuSwitch( @@ -176,7 +169,7 @@ proc build*(builder: WakuNodeBuilder): Result[WakuNode, string] = let peerManager = PeerManager.new( switch = switch, storage = builder.peerStorage.get(nil), - maxRelayPeers = maxRelayPeers, + maxRelayPeers = builder.maxRelayPeers, ) var node: WakuNode diff --git a/waku/v2/node/peer_manager/peer_manager.nim b/waku/v2/node/peer_manager/peer_manager.nim index 05b1b91164..dbfcdd5e7a 100644 --- a/waku/v2/node/peer_manager/peer_manager.nim +++ b/waku/v2/node/peer_manager/peer_manager.nim @@ -344,7 +344,7 @@ proc onPeerEvent(pm: PeerManager, peerId: PeerId, event: PeerEvent) {.async.} = proc new*(T: type PeerManager, switch: Switch, - maxRelayPeers: int = 50, + maxRelayPeers: Option[int], storage: PeerStorage = nil, initialBackoffInSec = InitialBackoffInSec, backoffFactor = BackoffFactor, @@ -359,16 +359,22 @@ proc new*(T: type PeerManager, maxConnections = maxConnections raise newException(Defect, "Max number of connections can't be greater than PeerManager capacity") - if maxRelayPeers > maxConnections: - error "Max number of relay peers can't be greater the max amount of connections", - maxConnections = maxConnections, - maxRelayPeers = maxRelayPeers - raise newException(Defect, "Max number of relay peers can't be greater the max amount of connections") - - if maxRelayPeers == maxConnections: - warn "Max number of relay peers is equal to max amount of connections, peer wont be contribute to service peers", - maxConnections = maxConnections, - maxRelayPeers = maxRelayPeers + var maxRelayPeersValue = 0 + if maxRelayPeers.isSome(): + if maxRelayPeers.get() > maxConnections: + error "Max number of relay peers can't be greater the max amount of connections", + maxConnections = maxConnections, + maxRelayPeers = maxRelayPeers.get() + raise newException(Defect, "Max number of relay peers can't be greater the max amount of connections") + + if maxRelayPeers.get() == maxConnections: + warn "Max number of relay peers is equal to max amount of connections, peer wont be contribute to service peers", + maxConnections = maxConnections, + maxRelayPeers = maxRelayPeers.get() + maxRelayPeersValue = maxRelayPeers.get() + else: + # Leave by default 20% of connections for service peers + maxRelayPeersValue = maxConnections - (maxConnections div 5) # attempt to calculate max backoff to prevent potential overflows or unreasonably high values let backoff = calculateBackoff(initialBackoffInSec, backoffFactor, maxFailedAttempts) @@ -377,7 +383,7 @@ proc new*(T: type PeerManager, maxBackoff=backoff raise newException(Defect, "Max backoff time can't be over 1 week") - let outRelayPeersTarget = max(maxRelayPeers div 3, 10) + let outRelayPeersTarget = max(maxRelayPeersValue div 3, 10) let pm = PeerManager(switch: switch, peerStore: switch.peerStore, @@ -385,10 +391,10 @@ proc new*(T: type PeerManager, initialBackoffInSec: initialBackoffInSec, backoffFactor: backoffFactor, outRelayPeersTarget: outRelayPeersTarget, - inRelayPeersTarget: maxRelayPeers - outRelayPeersTarget, + inRelayPeersTarget: maxRelayPeersValue - outRelayPeersTarget, + maxRelayPeers: maxRelayPeersValue, maxFailedAttempts: maxFailedAttempts, - colocationLimit: colocationLimit, - maxRelayPeers: maxRelayPeers) + colocationLimit: colocationLimit) proc connHook(peerId: PeerID, event: ConnEvent): Future[void] {.gcsafe.} = onConnEvent(pm, peerId, event) From c3d2d017ef47e719a10407b0108930a082136895 Mon Sep 17 00:00:00 2001 From: alrevuelta Date: Fri, 30 Jun 2023 09:50:15 +0200 Subject: [PATCH 3/4] Fix tests --- tests/v2/test_peer_manager.nim | 10 +++++----- waku/v2/node/peer_manager/peer_manager.nim | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/v2/test_peer_manager.nim b/tests/v2/test_peer_manager.nim index f6f26f5af3..eeb9e85206 100644 --- a/tests/v2/test_peer_manager.nim +++ b/tests/v2/test_peer_manager.nim @@ -607,7 +607,7 @@ procSuite "Peer Manager": .withMaxConnections(5) .build(), maxFailedAttempts = 1, - maxRelayPeers = 5, + maxRelayPeers = some(5), storage = nil) # Create 15 peers and add them to the peerstore @@ -660,7 +660,7 @@ procSuite "Peer Manager": initialBackoffInSec = 1, # with InitialBackoffInSec = 1 backoffs are: 1, 2, 4, 8secs. backoffFactor = 2, maxFailedAttempts = 10, - maxRelayPeers = 5, + maxRelayPeers = some(5), storage = nil) var p1: PeerId require p1.init("QmeuZJbXrszW2jdT7GdduSjQskPU3S7vvGWKtKgDfkDvW" & "1") @@ -709,7 +709,7 @@ procSuite "Peer Manager": .withPeerStore(10) .withMaxConnections(5) .build(), - maxRelayPeers = 5, + maxRelayPeers = some(5), maxFailedAttempts = 150, storage = nil) @@ -721,7 +721,7 @@ procSuite "Peer Manager": .withMaxConnections(5) .build(), maxFailedAttempts = 10, - maxRelayPeers = 5, + maxRelayPeers = some(5), storage = nil) let pm = PeerManager.new( @@ -730,7 +730,7 @@ procSuite "Peer Manager": .withMaxConnections(5) .build(), maxFailedAttempts = 5, - maxRelayPeers = 5, + maxRelayPeers = some(5), storage = nil) asyncTest "colocationLimit is enforced by pruneConnsByIp()": diff --git a/waku/v2/node/peer_manager/peer_manager.nim b/waku/v2/node/peer_manager/peer_manager.nim index dbfcdd5e7a..c2e430c128 100644 --- a/waku/v2/node/peer_manager/peer_manager.nim +++ b/waku/v2/node/peer_manager/peer_manager.nim @@ -344,7 +344,7 @@ proc onPeerEvent(pm: PeerManager, peerId: PeerId, event: PeerEvent) {.async.} = proc new*(T: type PeerManager, switch: Switch, - maxRelayPeers: Option[int], + maxRelayPeers: Option[int] = none(int), storage: PeerStorage = nil, initialBackoffInSec = InitialBackoffInSec, backoffFactor = BackoffFactor, From c32672fb07ba44606c7cc02f9c32596e9ba2fbcd Mon Sep 17 00:00:00 2001 From: alrevuelta Date: Tue, 4 Jul 2023 11:02:42 +0200 Subject: [PATCH 4/4] Fix comments --- waku/v2/node/peer_manager/peer_manager.nim | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/waku/v2/node/peer_manager/peer_manager.nim b/waku/v2/node/peer_manager/peer_manager.nim index c2e430c128..05ccf2834b 100644 --- a/waku/v2/node/peer_manager/peer_manager.nim +++ b/waku/v2/node/peer_manager/peer_manager.nim @@ -362,13 +362,13 @@ proc new*(T: type PeerManager, var maxRelayPeersValue = 0 if maxRelayPeers.isSome(): if maxRelayPeers.get() > maxConnections: - error "Max number of relay peers can't be greater the max amount of connections", + error "Max number of relay peers can't be greater than the max amount of connections", maxConnections = maxConnections, maxRelayPeers = maxRelayPeers.get() - raise newException(Defect, "Max number of relay peers can't be greater the max amount of connections") + raise newException(Defect, "Max number of relay peers can't be greater than the max amount of connections") if maxRelayPeers.get() == maxConnections: - warn "Max number of relay peers is equal to max amount of connections, peer wont be contribute to service peers", + warn "Max number of relay peers is equal to max amount of connections, peer won't be contributing to service peers", maxConnections = maxConnections, maxRelayPeers = maxRelayPeers.get() maxRelayPeersValue = maxRelayPeers.get()