fix issue with read_piece() stopping torrent #7359
Conversation
|
Should you also update the web page reference to indicate the torrent will not be paused on error? http://libtorrent.org/reference-Alerts.html#read_piece_alert |
|
oh, interesting, stopping the torrent is documented. I think this patch may not be right. |
Huh. Well if you're already keeping track of the pieces downloaded (but not yet written to disk) through this internal call to user_have_piece() then why not expose it to user applications so that we don't have to maintain our own data structure duplicating this exact same bookkeeping logic? You already expose the internal data structure for downloaded and saved to disk pieces through the have_piece() call so why not do it also for this structure for merely downloaded pieces? It would save us some headaches. |
|
I think there's some misunderstanding. You can call have_piece() if you'd like to know whether a piece has been downloaded or not. You're probably not going to like it thought, as it's a blocking call. I don't know what you're asking for be exposed in addition to this. |
|
Oh so the internal user_have_piece() call and the have_piece() API call are querying the same data structure? If so then your change isn't really cancelling the issuing of "a read for a piece we know we don't have" because it's being pessimistic and will also no longer issue a read request for a piece we do have in libtorrent's disk cache (but hasn't yet been saved to disk/OS) as you said in the related thread ("If you read the data via libtorrent, with read_piece() it will be read through the same path data is read when requested by peers. i.e. it can read from libtorrent's disk cache.") This current change here will make waiting to call read_piece() take longer than necessary (and effectively dependent on the blocking have_piece() call as you said) which would not be a good outcome, as we'd have to wait until the piece is saved to the OS instead of merely waiting until it is downloaded. In light of this the original change to no longer pause or stop the torrent when the piece isn't there is favorable. Though probably the best change from the API perspective is as you just suggested in the related thread to add a flag to an overload of the read_piece() API call to notify with read_piece_alert of when the piece is eventually downloaded. |
|
What does this latest commit do now? |
|
it checks if the piece has passed hash check, which does not require that it has been written to disk (i.e. handed off to the OS) |
|
Ah ok. Yes that is optimal! Otherwise it's a noop (not a failure) right? I don't know if you checked the msg in the related thread, though what mechanism does read_piece() use to keep track of multiple in-flight read requests to individual pieces? Could you use this same mechanism as is - or maybe improve it some more - to support the addition of an overloaded call with a flag to keep track of read requests to yet to be downloaded pieces? |
|
Hmm this change has broken how Deluge handles setting an error state for torrents that were seeding but the files were moved on OS therefore should not try to re-download them without user intervention. I will need to revisit this to understand what would work instead, any suggestions @arvidn? Deluge ticket: https://dev.deluge-torrent.org/ticket/1032 |
|
are calls to |
|
It's been a while since I touched that code but the tests were using read_piece to force the error which seemed to simulate what was occurring with actual torrents correctly. I think there are two scenarios, one where re-adding a torrent to the session and the files on disk are missing and the other where a running session with seeding torrent have their file on disk moved or removed (e.g. unmounted disk) I created a ticket with sample code to demo the difference in lt versions: https://dev.deluge-torrent.org/ticket/3603 |
on pieces not yet downloaded