fix(p2p): fix possible connectivity issue #1996
Conversation
|
You can find the image built from this PR at |
alrevuelta
changed the title
|
You can find the experimental image built from this PR at |
| # TODO: Temporally disabled. Might be causing connection issues | ||
| #if inRelayPeers.len > pm.inRelayPeersTarget: | ||
| # await pm.pruneInRelayConns(inRelayPeers.len - pm.inRelayPeersTarget) |
There was a problem hiding this comment.
I vote to remove that directly. If we need it in the future we can review this commit and easily restore it back. The TODO is likely to be there forever :)
| # TODO: Temporally disabled. Might be causing connection issues | |
| #if inRelayPeers.len > pm.inRelayPeersTarget: | |
| # await pm.pruneInRelayConns(inRelayPeers.len - pm.inRelayPeersTarget) |
There was a problem hiding this comment.
would like to leave it there, since its something I would like to revisit. Some peer prunning in inbound connections is needed, the open question is how.
| @@ -101,7 +101,7 @@ proc newWakuSwitch*( | |||
| if peerStoreCapacity.isSome(): | |||
| b = b.withPeerStore(peerStoreCapacity.get()) | |||
| else: | |||
| let defaultPeerStoreCapacity = int(round(float64(maxConnections)*1.25)) | |||
| let defaultPeerStoreCapacity = int(maxConnections)*5 | |||
There was a problem hiding this comment.
Shall we have a comment indicating why we set the max PeerStoreCapacity to that?
From the PR's comment is clear, but I'd mention that in the code as well
There was a problem hiding this comment.
will address it in the future. maybe this shouldn't even be configurable and always harcoded depending on the amount of maxConnections.
| @@ -10,6 +10,7 @@ import | |||
| chronicles, | |||
| metrics, | |||
| libp2p/multistream, | |||
| libp2p/connmanager, | |||
There was a problem hiding this comment.
Is this really needed?
There was a problem hiding this comment.
good catch, fixed
47e3a49
alrevuelta
force-pushed
the
fix-con-issues
branch
from
6f7874c
to
47e3a49
Compare
Description:
xthe peerstore size should be way bigger than that. If no, we wont benefit much from the exponential backoff, since after prunning, this state is "forgotten".Test:
waku-simulator