Skip to content
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

fix(comms): only reap when number of connections exceeds threshold #4607

Merged

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Sep 2, 2022

Description

  • only release connection handles of non-neighbouring peers after successful connect
  • adds min threshold for connection reaping with default 50
  • only reap connections that have less than 3 substreams
  • only reap "excess" (num_connections - 50) connections
  • adds RpcServer query that returns number of sessions for a peer
  • updates list-connections command to display number of peer connection handles and rpc sessions
  • updates list-connections to display two tables, one with wallets and one with base nodes

Motivation and Context

Previously, connection handles would be dropped (making them reapable) when refreshing the neighbour peer pool. The neighbour pool only starts the attempt to connect, but the non-neighbouring connection handles should only be dropped if a replacement neighbour was actually able to connect.
Reaping should only apply when we have many connections, otherwise many connections are acceptable.

Fixes #4608

How Has This Been Tested?

Manually, checking that connections to other base nodes/wallets running this PR stay connected

@sdbondi sdbondi force-pushed the comms-improve-reaping branch 5 times, most recently from f4648dd to d1b5917 Compare September 2, 2022 09:08
@stringhandler stringhandler merged commit 415f339 into tari-project:development Sep 2, 2022
@sdbondi sdbondi deleted the comms-improve-reaping branch September 2, 2022 13:35
@SWvheerden SWvheerden mentioned this pull request Nov 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BN loses connections over time
2 participants