enable fast resume of previous torrents #27

[bsuh]

Write the bitfield of downloaded pieces to disk and the hash of the bitfield as well to verify the file isn't corrupted

[mafintosh]

What if the actual data files gets corrupted? This wont catch that right? On Sun 5 Jul 2015 at 12:42 Brian Suh notifications@github.com wrote:

Write the bitfield of downloaded pieces to disk and the hash of the

bitfield as well to verify the file isn't corrupted

You can view, comment on, or merge this pull request online at:

https://github.com/mafintosh/torrent-stream/pull/126 Commit Summary

• enable fast resume of previous torrents #27

File Changes

• M index.js https://github.com/mafintosh/torrent-stream/pull/126/files#diff-0 (95)

Patch Links:

• https://github.com/mafintosh/torrent-stream/pull/126.patch
• https://github.com/mafintosh/torrent-stream/pull/126.diff

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

[bsuh]

No it won't. From what I could tell (from documentation not source code FWIW), libtorrent-rasterbar doesn't check for data corruption either. qBittorent does seem to keep store the last modified date of files and recheck hashes if there's a discrepancy.

AFAIK there's no way to check for data corruption without actually hashing the files, which is what I'm trying to avoid in the first place.

[bsuh]

What could be done is to send 'have' messages to peers based on the fast resume data, but check a piece hash when a peer requests it or it is being read by a file stream, if there's a discrepancy, then choke the peer and refetch the piece.

[asapach]

You don't want to check the hash every time you access a piece, because it will kill performance. This means you'll have to maintain some kind of bitfield of verified pieces, which increases complexity.

[bsuh]

Right, but only if we want to make sure there's no filesystem corruption. My opinion is that if a user's filesystem is trashing files, then it's the user's problem.

[asapach]

It's the swarm's problem if you start distributing corrupt pieces.

[bsuh]

Don't clients just block you?

[asapach]

They should, but you never know. https://en.wikipedia.org/wiki/Torrent_poisoning

[bsuh]

So is there something I can do to get this merged?

[mafintosh]

@bsuh could we make this opt-in for now? then after we test it for a bit in the wild we can make it default behaiviour

[bsuh]

Yea, I'll make it a parameter for opts tomorrow. I'm feeling hyperactive and it's hard to focus today.

[mafintosh]

we all know that feeling :)

[bsuh]

Added it as an option.

Hmm. When fast resume is enabled, this seems to break the blocklist test, because 'blocked-peer' gets emitted before 'ready' event. The test attaches the 'blocked-peer' only after the engine's 'ready' event.

I don't know if the expectation of 'ready' event always coming before 'blocked-peer' event is a valid expectation. If that's how it's supposed to work, it won't always be true, since engine initialization is asynchronous. It seems that with a fresh torrent with no written data and just 1 file, the one fs.exists call during verification will be fast enough that the 'ready' event will be emitted before 'blocked-peer' event, but one fs.readFile for the non-existent fast resume data file isn't fast enough. Also if previously written data exists, then you need multiple fs calls to verify. So I think the test should be changed?

test('peer should block the seed via blocklist', function(t) {
    t.plan(3);
    var peer = torrents(torrent, {
        dht: false,
        blocklist: [
            { start: '127.0.0.1', end: '127.0.0.1' }
        ]
    });
    peer.once('ready', function() {
        t.ok(true, 'peer should be ready');
        peer.once('blocked-peer', function(addr) {
            t.equal(addr, '127.0.0.1:6882');
            peer.destroy(t.ok.bind(t, true, 'peer should be destroyed'));
        });
    });
});

to

test('peer should block the seed via blocklist', function(t) {
    t.plan(3);
    var peer = torrents(torrent, {
        dht: false,
        blocklist: [
            { start: '127.0.0.1', end: '127.0.0.1' }
        ]
    });
    peer.once('ready', function() {
        t.ok(true, 'peer should be ready');
    });
    peer.once('blocked-peer', function(addr) {
        t.equal(addr, '127.0.0.1:6882');
        peer.destroy(t.ok.bind(t, true, 'peer should be destroyed'));
    });
});

[muchweb]

I have stopped a download halfway through and then re-started a program. It seems that it just starts the whole download all over again.

Please implement a fast check and resume feature the way other clients do it.