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

[RFC] limiter: smarter perPeerLimit #81

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

magik6k
Copy link
Contributor

@magik6k magik6k commented Jun 26, 2018

This makes the limiter.perPeerLimit depend on fdConsuming.

graph of perPeerLimit vs fdConsuming:
graph

@ghost ghost assigned magik6k Jun 26, 2018
@ghost ghost added the status/in-progress In progress label Jun 26, 2018
@magik6k magik6k changed the title limiter: smarter perPeerLimit [RFC] limiter: smarter perPeerLimit Jun 26, 2018
@magik6k magik6k requested a review from Stebalien June 26, 2018 16:07
@magik6k
Copy link
Contributor Author

magik6k commented Jun 26, 2018

(Will work on fixing tests once the idea is approved)

@@ -91,27 +97,41 @@ func (dl *dialLimiter) freeFDToken() {
}
}

func (dl *dialLimiter) updateWaitingPerPeer() {
for p := range dl.activePerPeer {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two loops are going to be very CPU heavy.

@Stebalien
Copy link
Member

So, this looks like it decreases per-peer dials as we run out of file descriptors? We should make sure this doesn't lead to a situation where everything is slow and therefore none of the dials really finish.

@bigs bigs added need/community-input Needs input from the wider community optimization labels Sep 4, 2018
@bigs
Copy link
Contributor

bigs commented Sep 4, 2018

i think this change should hold off until we get a better idea of where we're getting bogged down when dialing DHT nodes

@bigs bigs added the status/blocked Unable to be worked further until needs are met label Sep 4, 2018
@raulk
Copy link
Member

raulk commented Jan 31, 2019

This idea is interesting. Choking the peer limit as the FD allowance runs out. However, I worry that this will bring unfairness to the system. Also won't work well with the address pollution problem of the DHT (e.g. we have peers with 200 addresses), unless we implement an address scoring system. Right now we're pretty dumb and dial addresses in the order returned by the peerstore, but if we can prioritise certain addresses, even in a choked scenario we could end up dialling successfully.

FYI @magik6k – we traced down the source of many woes to the DHT tripping over the FD limit in the swarm, and we are now pacing dials dynamically: libp2p/go-libp2p-kad-dht#237.

Are you still interested in discussing ideas around this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
need/community-input Needs input from the wider community optimization status/blocked Unable to be worked further until needs are met status/in-progress In progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants