Fixes Reconnecting to Wires #116
Conversation
As mentioned on mafintosh/peerflix#203
|
There may still be a better way to restart peer search then restarting |
|
It probably it would be better to move this logic into 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. |
|
While you're at it, consider switching to use torrent-discovery so we can share the effort of maintaining this with webtorrent :-) |
|
@feross, |
|
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 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 As for a reason for Would be awesome to use torrent-discovery, but it would still need |
Some people might use |
|
Agreed, but doesn't that conflict with the fact that As mentioned on mafintosh/peerflix#203 , torrent-stream not only disconnects all peers but also blocks them, so after killing them they can't be reconnected anymore without using 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. |
|
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. |
|
If you are correct then But then what about the new peers that are found by restarting |
|
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. |
|
I'm all for getting peerflix to seed better after downloading so feel free
|
|
I can't figure out why, but I am 100% sure now that something is terribly wrong with From the code in this pull request, I commented out the line that uses The torrent got up to 12 seeds on the first video, video finished downloading, peers went down to 0. Used 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 This is not the case for all poor health torrents, as I've checked the code in Best I can do is change it to |
|
@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 |
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 |
Required for Reconnecting to Wires Fix
| ontorrent(link); | ||
| } else { | ||
| fs.readFile(torrentPath, function(_, buf) { | ||
| if (destroyed) return; | ||
| swarm.resume(); | ||
| pauseSwarm = false; |
There was a problem hiding this comment.
This happens asynchronously, so the pause() below has already happened.
There was a problem hiding this comment.
Agreed, don't know how I missed that.
Do you think that moving pauseSwarm = false; above fs.readFile() could create any issues? Can torrentPath get to this point and not be a valid path?
But then pauseSwarm = false; would be in both if and else statements, is it even necessary to pause the swarm? Personally I don't see any possible reason for it.
There was a problem hiding this comment.
Why are you trying to change the order of events in the first place?
There was a problem hiding this comment.
Fixing wire reconnection checks for a very particular case in which someone resumes a paused swarm. By using swarm.pause() right before swarm.resume() at the very start of torrent-stream this behavior is reproduced for no reason, and it will trigger wire reconnection.
See: https://github.com/jaruba/peer-wire-swarm/blob/master/index.js#L177
There was a problem hiding this comment.
But again, I think this line:
swarm.pause()
in torrent-stream doesn't do anything, because in both if and else that come right after it, swarm.resume() is called.
I tested torrent-stream with that line removed and everything works as it should (as swarm.paused is false by default), it looks like it is there as a fallback if swarm.resume() isn't triggered, but I cannot see how it could ever not be triggered.
Unless torrentPath is wrong and then fs.readFile() errors out, but if that happens torrent-stream in it's entirety won't work anyway, so why even use swarm.pause() in this case?
|
@jaruba, I've created a branch based on your suggestions: https://github.com/mafintosh/torrent-stream/tree/discovery @mafintosh, please take a look. |
|
created a new PR: #117 |
|
closing this one |
As mentioned on mafintosh/peerflix#203
Reinitializes peer search on
engine.files[i].select()if the swarm was previously paused and addsengine.discover()that also restarts peer search.