Skip to content
This repository has been archived by the owner on Apr 21, 2022. It is now read-only.

Don't count connections in the grace period #46

Closed
Stebalien opened this issue May 26, 2019 · 4 comments · Fixed by #50
Closed

Don't count connections in the grace period #46

Stebalien opened this issue May 26, 2019 · 4 comments · Fixed by #50

Comments

@Stebalien
Copy link
Member

The connection manager currently counts connections in the grace period towards the connection limit. Unfortunately, this means we can end up killing every (or almost every) single connection not in the grace period if we get enough grace-period connections.

We should:

  1. Check to see if this is happening (e.g., on the relays/gateways).
  2. Consider not counting these peers.
@raulk
Copy link
Member

raulk commented May 27, 2019

Sounds fair. Let's only count the connections we can effectively close.

@raulk
Copy link
Member

raulk commented May 27, 2019

The danger is that we could end up closing high-value connections in place of lower-graded connections that happen to be within their grace period. In some protocols this could lead to killing high quality long-lived peers while keeping peers we only connected to for a single RPC or so. I think burst modelling (as I’ve proposed for v2) will help here.

@vyzo
Copy link
Contributor

vyzo commented May 27, 2019

This is going to be a little tricky to implement.

@Stebalien
Copy link
Member Author

We don't have to implement the optimal solution. We can:

  1. Check our total number of connections, abort if too few.
  2. Walk through these and count, discarding connections in the grace-period.

Although, the optimal solution shouldn't be that hard:

  1. Keep a queue of new peers associated with their first-seen time.
  2. Keep two maps: peers in the grace period and peers not in the grace period.
  3. Have a background goroutine moving connections from one to the other.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants