Tracker server should prune old peers who did not leave the swarm gracefully

[feross]

If a peer doesn't send a stop message then we should prune them after a while.

[feross]

This is the package we should use: https://npmjs.com/package/lru

It's also used in bittorrent-dht where it works great.

[dkobia]

@feross, glad to see even a little progress on this ticket

[DiegoRBaquero]

IMO we should use a TTL based cached as LRU depends on a size. So a peer without any data exchanged in the last X minutes with the tracker gets removed.

[DiegoRBaquero]

I'm thinking of throwing a PR based on node-cache as @lorentzkim did in #87. The module doesn't create n timers/intervals, but I think all of its code is synchronous, especially the periodic TTL check that removes old keys.

I've created an issue on its repo and plan to PR to fix the issue. (node-cache/node-cache#56)

Thoughts?

[feross]

I know the tracker spec technically requires an lru based on time, but space is the actual real constraint. If a tracker gets popular, it shouldn't crash with an out-of-memory error -- it should just drop drop peers sooner than normal.

An lru that depends on size is close enough to right. The worst that happens is that on an unpopular tracker, old ip addresses will be given out. But that already happens all the time and clients don't mind. We use lru in bittorrent-dht and it works great. Also, @mafintosh is a maintainer which is another plus.

[yciabaud]

If we use lru what would be the default length of the cache? 10K like bittorrent-dht?
Do we use maxAge option too in order to fit the tracker spec?

[feross]

I think we should try to do what the spec says by default, but of course let the user set the lru options themselves if they want to.

The lru readme says: "If you pass maxAge items will be evicted if they are older than maxAge when you access them." Let's make sure that items that never get accessed don't stick around forever. If that will happen with maxAge, then I'd rather just set max (i.e. max elements to store) instead.

[yciabaud]

I was thinking of setting both params, max to ensure we are not consuming too much memory and maxAge to avoid returning peers that did not leave gracefully on announce if the max is not reached.

The downside of this is that if we use maxAge is that we need to handle when we try to access an old peer and each access will bring the peer to the top so max can drop a more active peer.

Do you think returning old peers is no big deal? Using max only would be easier.