clear out extra dial jobs after dial finishes #51
Conversation
ghost
assigned 
|
@Stebalien how does it not solve the problem? |
limiter.go
Outdated
|
|
||
| arr := dl.waitingOnFd[:0] | ||
| for _, j := range dl.waitingOnFd { | ||
| if j.peer != p { |
There was a problem hiding this comment.
These are currently considered "active". We'll need to decrement the activePerPeer counter for each one we remove.
whyrusleeping
force-pushed
the
fix/clear-cancelled-dials
branch
from
887a5d4
to
2c697ba
Compare
| delete(dl.waitingOnPeerLimit, p) | ||
| // NB: the waitingOnFd list doesnt need to be cleaned out here, we will | ||
| // remove them as we encounter them because they are 'cancelled' at this | ||
| // point |
There was a problem hiding this comment.
Same as the waitingOnPeerLimit list. The problem is that it could be a while before that happens if we're really backed up behind a bunch of slow dials. It won't be quite as bad because it's limited by dials*perPeerLimit but it will still be large and can still grow in the same way.
There was a problem hiding this comment.
@Stebalien Yeah, the tradeoff we make is the O(n) operation under a held lock after completion vs this potential extra memory buffering.
There was a problem hiding this comment.
That could be solved with a doubly linked list (wait queues like this are really one of the only uses for them).
There was a problem hiding this comment.
Ah, great point. We never really need to iterate over it, so that makes perfect sense.
|
So the final thing I need to do here is add metrics as @lgierth requested |
|
alright, so it turns out we don't have things set up yet to nicely add metrics from go-libp2p. I'm thinking that we should push to get this merged sooner, and work out the metrics later |
Agreed |
This really wouldnt be an issue if we didnt have tens of thousands of addresses hanging around for some reason