Fixes Reconnecting to Wires

[jaruba]

As mentioned on https://github.com/mafintosh/peerflix/issues/203

Reinitializes peer search on engine.files[i].select() if the swarm was previously paused and adds engine.discover() that also restarts peer search.

[jaruba]

There may still be a better way to restart peer search then restarting peerDiscovery() completely. This is just the way I got it working in my tests.

[asapach]

It probably it would be better to move this logic into lib/peer-discovery.js and encapsulate it as peerDiscovery.restart() by recreating DHT and tracker.

Also I'm not sure it's a good idea to restart peer discovery when the swarm is paused, because I assume it was paused for a reason. Or alternatively, the swarm should be resumed as well.

[feross]

While you're at it, consider switching to use torrent-discovery so we can share the effort of maintaining this with webtorrent :-)

[asapach]

@feross, torrent-discovery is missing updatePort() method. Otherwise we should be able to switch.

[jaruba]

@asapach

I personally can't be sure that peer discovery even stops, but I really doubt it does not stop, as I've tested this rigorously in the past, if I didn't launch peer discovery with engine.discover(), after the swarm has paused and lost all peers, no new peers wore ever found.

With the above function I used to get a few more new peers (for old, poor health torrents, I would get 1-2 new peers that I would of never gotten without using it). But this might not mean that peer discovery was paused/stopped necessarily as there are the same chances that a different issue could create this behavior. So I can not guarantee this happens until I am sure that no new peers are ever found after reinitializing the swarm. I guess I should test this more with a more popular torrent with multiple videos. (as with popular torrents there are more chances of finding new peers, not finding new peers for poor health torrents does not really prove anything)

All I know for sure is that there was a clear improvement after using engine.discover() in this scenario.

As for a reason for peerDiscovery to pause/stop after the swarm has paused, it makes sense for it to pause when the swarm has began a timeout event that will disconnect all peers (as no files are selected to download anymore). TBH, if it doesn't do this, it should, but it should also start peerDiscovery again after the swarm is resumed.

@feross

Would be awesome to use torrent-discovery, but it would still need .pause() and .resume() for it to be integrated and solve this issue best, then as much as I would love to integrate it I fear I may not be the best person to do this, not without both your help as I don't believe I know enough about torrent-stream and webtorrent to be sure that I won't create more issues then I try to solve.

[asapach]

... it makes sense for it to pause when the swarm has began a timeout event that will disconnect all peers (as no files are selected to download anymore). TBH, if it doesn't do this, it should, ...

Some people might use torrent-stream in a seed box, wherein after all the files have been downloaded, the peer continues to seed indefinitely. Stopping peer discovery prohibits this scenario.

[jaruba]

Agreed, but doesn't that conflict with the fact that torrent-stream kills all peers anyway after all files have been downloaded?

As mentioned on https://github.com/mafintosh/peerflix/issues/203 , torrent-stream not only disconnects all peers but also blocks them, so after killing them they can't be reconnected anymore without using swarm.reconnectAll(), so in the current situation, I am sure it can't work as a seed box, not without some serious changes to the code.

If you use it in a seed box you should have the choice of keeping the peers alive, otherwise it should also pause peer discovery.

[asapach]

My understanding is that it doesn't kill the peers, but rather they choose to disconnect, since there's no need to maintain a seeder-to-seeder connection. Leechers should still be able to connect.

[jaruba]

If you are correct then peerDiscovery always works, and engine.discover() is not only useless but also complicates things for no reason. And you may very well be right as I haven't yet found anything in the code that either kills peers or ever pauses/stops peerDiscovery, and I've searched for this...

But then what about the new peers that are found by restarting peerDiscovery and what about this issue: https://github.com/mafintosh/peerflix/issues/159 which contradicts the possibility of using torrent-stream as a seed box?

[asapach]

Peerflix does actually pause the swarm when it's not downloading: https://github.com/mafintosh/peerflix/blob/master/index.js#L193

As for the "new" peers, those could be the peers that either previously timed out or disconnected.

[mafintosh]

I'm all for getting peerflix to seed better after downloading so feel free to change that current behaivior On Fri 8 May 2015 at 23:50 Aliaksei Sapach notifications@github.com wrote:

Peerflix does actually pause the swarm when it's not downloading: https://github.com/mafintosh/peerflix/blob/master/index.js#L193

As for the "new" peers, those could be the peers that either previously timed out or disconnected.

— Reply to this email directly or view it on GitHub https://github.com/mafintosh/torrent-stream/pull/116#issuecomment-100377245 .

[jaruba]

I can't figure out why, but I am 100% sure now that something is terribly wrong with peerDiscovery() after the swarm was paused (at least)

From the code in this pull request, I commented out the line that uses engine.discover() and started doing more tests with a poor health torrent that has multiple video files.

The torrent got up to 12 seeds on the first video, video finished downloading, peers went down to 0. Used engine.files[i].select() on a different video file and peer-wire-swarm used .reconnectAll(), all the peers successfully passed this line:

https://github.com/mafintosh/peer-wire-swarm/blob/master/index.js#L199

But only 2 peers wore actually reconnected, and the file was not downloading, so I manually executed engine.discover() and in a split second 14 more peers wore connected and the video started downloading.

This is not the case for all poor health torrents, as swarm.reconnectAll() reconnects to a lot more peers in other cases, but using engine.discover() does increase the number of peers significantly. It is my belief that both these methods are necessary for this bug to be fully fixed.

I've checked the code in torrent-stream and lib/peer-discovery.js thoroughly but I can't find anything that can cause peerDiscovery() to die out or go in a paused state.

Best I can do is change it to peerDiscovery.restart() as @asapach suggested. Any thoughts?

[jaruba]

@mafintosh About peerflix not seeding after the torrent or all selected files have finished downloading, do you think this is because the swarm is paused when peerflix looses interest?

If so it should be easy enough to add a parameter that stops it from pausing the swarm after interest is lost.

I guess if torrent-stream continues to seed after download has finished and peerflix doesn't, it does sound like this would be the reason for it. But I would need confirmation that this is the case as testing this can be troublesome.

[yipperr]

But only 2 peers wore actually reconnected, and the file was not downloading, so I manually executed engine.discover() and in a split second 14 more peers wore connected and the video started downloading. But then what about the new peers that are found by restarting peerDiscovery and what about this issue:

i observed the exact same behavior as @jaruba said and running engine.discover() again in my instance i got connected to 19 peers and download resumed prior to this it was just staying still without any progress it tend to happen with with torrent with a low amount of seeds and peers

[asapach]

@jaruba, I've created a branch based on your suggestions: https://github.com/mafintosh/torrent-stream/tree/discovery Could you please try it out?

@mafintosh, please take a look.

[asapach]

created a new PR: #117

[jaruba]

closing this one