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

partsfile is not deleted upon finishing downloading #1967

Closed
zeule opened this issue May 3, 2017 · 8 comments
Closed

partsfile is not deleted upon finishing downloading #1967

zeule opened this issue May 3, 2017 · 8 comments
Labels

Comments

@zeule
Copy link
Contributor

zeule commented May 3, 2017

libtorrent 1.1.3
Linux x64
GCC 6.3.0

Using qBittorrent master:

  1. Start downloading a torrent with many small files aligned in torrent randonly.
  2. Unselect most part of the files from dowloading.
  3. Wait for finishing. Libtorrent created a big .parts file for this torrent so du -s <torrent dir> <parts file> is almost equal to the torrent full size.
  4. Select all other files and wait until downloading is finished.

Now final dir size is equal to the torrent size, but .parts file size did not change, so du -s <torrent dir> <parts file> is significantly greater than the torrent size.

@arvidn
Copy link
Owner

arvidn commented May 3, 2017

The part file doesn't shrink when data from it is "evicted", nor is it removed if every part is removed from it.

This is what you're referring to, right?

I suppose the last part would be fairly simple to implement, just deleting the file whenever there's nothing it it.

@zeule
Copy link
Contributor Author

zeule commented May 4, 2017

Yes, exactly. It also would be nice to shrink it when pieces get moved from it to the final files. On LInux a naïve implementation would simply call fallocate(fb, FALLOC_FL_PUNCH_HOLE, ...) when a piece is evicted. Is it possible to implement?

@arvidn
Copy link
Owner

arvidn commented May 4, 2017

That would probably be a pretty simple patch. do you want to give it a stab?
It would be somewhere in this block here: https://github.com/arvidn/libtorrent/blob/RC_1_1/src/part_file.cpp#L355

it conditioned on FALLOC_FL_PUNCH_HOLE being defined probably.

@zeule
Copy link
Contributor Author

zeule commented May 4, 2017

I can try. But why inside of the loop but not at the end of the function (via a RAI object, declared here, at the beginning of the function) in order to free all the blocks at once?

@arvidn
Copy link
Owner

arvidn commented May 4, 2017

because the mutex is released in the middle of that function. when the mutex is acquired again at the end, we do the lookup again. If we can't find the piece, another thread may already have punched a hold in the file and then re-allocated that space for some other piece.

In fact, it also opens up the question of whether it's really OK to punch a hole while another thread is still reading from that portion (probably not) and that perhaps that need to be dealt with. perhaps a per-piece lock bit or something. not sure.

@zeule
Copy link
Contributor Author

zeule commented May 4, 2017

I see. It's more complicated than trivial, then :) "Another reading thread" is a thread which uploads this peace to a remote peer?

@arvidn
Copy link
Owner

arvidn commented May 5, 2017

yeah, disk I/O is done in multiple threads, and holding a mutex during the actual disk access would defeat the purpose of multi-threaded I/O.

The race condition I point out exists today though. punching a hole will just increase the chance a bit of getting into problems a bit. I'm not sure what the simplest way forward would be. perhaps to ignore the race for now and deal with it separately.

@stale
Copy link

stale bot commented Feb 29, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 29, 2020
@stale stale bot closed this as completed Mar 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants