Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

`torrent.rescanFiles()` should detect missing pieces that were initially verified #1695

Open
guanzo opened this issue Aug 8, 2019 · 4 comments
Open

Comments

@guanzo
Copy link
Contributor

@guanzo guanzo commented Aug 8, 2019

Related: #1650

What version of WebTorrent?
v0.107.0 (latest)

What did you expect to happen?
torrent.rescanFiles() (and by association _verifyPieces) should detect pieces that were previously verified but now missing, and rerequest them.

What actually happened?
torrent.rescanFiles() doesn't do that.

If a piece was verified on torrent initialization, then deleted, then rescanFiles() is called, webtorrent should detect the missing piece and fetch it. After all, thats the point of verifying pieces, right?

Currently, once a piece is verified, it stays verified even if store.get(index) inside _verifyPieces() returns a missing chunk error. This is because state that tracks pieces (torrent.bitfield, torrent.pieces, torrent._hashes, torrent._reservations) doesn't react to changes in the underlying storage. I believe Webtorrent should be robust enough to handle unreliable storage, such as indexedDB, where a user or script can delete pieces at will.

At a high level, once _verifyPieces() is called, the torrent should be aware of what pieces are actually verified and react accordingly.

@guanzo guanzo changed the title `torrent.rescanFiles()` should detect missing pieces that were already verified `torrent.rescanFiles()` should detect missing pieces that were initially verified Aug 8, 2019
@feross

This comment has been minimized.

Copy link
Member

@feross feross commented Aug 8, 2019

@jhiesey Do you have thoughts about this?

@jhiesey

This comment has been minimized.

Copy link
Contributor

@jhiesey jhiesey commented Aug 8, 2019

The original reason for adding torrent.rescanFiles() was to allow adding data into webtorrent after a torrent was already started, specifically for a use case the Internet Archive has.

I agree that the naming is somewhat confusing, and it would better match expectations and be more useful if it also handled detecting data that went missing from the datastore. Should I see about implementing that?

@feross

This comment has been minimized.

Copy link
Member

@feross feross commented Aug 8, 2019

Should I see about implementing that?

Only if you're interested. I still don't fully understand the root cause of the issue and haven't had time to dig in, and I want to understand more about what's involved to fix this before we commit to fixing it.

@guanzo

This comment has been minimized.

Copy link
Contributor Author

@guanzo guanzo commented Aug 8, 2019

For some context, I am proactively deleting chunks from indexedDB for reasons (business logic, avoid QuotaExceededError, w/e). Here's how I'm doing that now.

async function deletePieces(torrentId, pieceIndexes = []) {
    // Stop active torrent
    await new Promise(resolve => torrentClient.remove(torrentId, resolve))
   // Deletes specified pieces of this torrent from indexedDB
    await deletePiecesFromStore(torrentId, pieceIndexes)
    // Re-add torrent to verify pieces
    torrentClient.add(torrentId)
}

This approach is wasteful, it destroys all state and connections. What I'd like to be able to do is this:

async function deletePieces(torrentId, pieceIndexes = []) {
    // Deletes specified pieces of this torrent from indexedDB
    await deletePiecesFromStore(torrentId, pieceIndexes)
    const torrent = torrentClient.get(torrentId)
    torrent.rescanFiles() 
}

A detected missing piece should only be refetched if it was selected with torrent.select(), If it was deselected with torrent.deselect(), then it shouldn't be refetched, but the torrent should update itself and inform any peers that it no longer has this piece.

OPTIONAL: Taking this further, we could add a new torrent method that deletes pieces and handles updating internal state (this.bitfield, etc). This method could simply call torrent.store.delete(index), but that'd require updating the abstract chunk store API. I get that it's unusual for a torrent client to only delete pieces as opposed to entire files, so I don't expect this API to be implemented.

async function deletePieces(torrentId, pieceIndexes = []) {
    const torrent = torrentClient.get(torrentId)
    for (const index of pieceIndexes) {
        torrent.deletePiece(index)
    }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.