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

[bug] BTRFS subvolumes are treated as different filesystems, hurting the performance of moving a torrent #1060

Open
leijurv opened this issue Nov 22, 2019 · 8 comments
Labels
pr welcome type:feat A new feature type:perf A code change that improves performance

Comments

@leijurv
Copy link

leijurv commented Nov 22, 2019

Even though I have just one filesystem, when I try and move a completed torrent from one folder to another, transmission thinks they are different filesystems since they're in different btrfs subvolumes, and therefore it reads the files in one by one and writes them one by one.

I've timed it, and moving the same folder is more than 10x faster just using mv and removing and readding the torrent, than letting transmission do it.

Can there be an option to use the system mv instead of, uh, whatever it's currently doing to move the files?

@kilobyte
Copy link

This is actually pretty nice, and I abuse it by having the incomplete-vs-finished dirs on different subvolumes. This greatly reduces space waste due to fragmented misaligned extents.

Transmission's write patterns play with btrfs so badly that a torrent may take ~4 times the space. Such files are also damn slow to read. Rewriting the file turns this pathological layout to optimal, fully linear, big extented layout. On the other hand, mv merely relinks the inode to another directory, without improving the file.

You can test if my theory of your problem is right by running compsize on a file that's nearly finished. If the "Disk Usage" field is higher than "Referenced", that's it.

@leijurv
Copy link
Author

leijurv commented Nov 26, 2019

https://xkcd.com/1172/

@kilobyte
Copy link

What I mean is that Transmission should do such a rewrite on btrfs in all cases, even when the finished dir is on the same subvolume, to fix the problem I mentioned.

There's BTRFS_IOC_DEFRAG_RANGE but that's nowhere as good as a full copy+delete.

@RobCrowston
Copy link
Contributor

RobCrowston commented Dec 28, 2019

Even though I have just one filesystem, when I try and move a completed torrent from one folder to another, transmission thinks they are different filesystems since they're in different btrfs subvolumes, and therefore it reads the files in one by one and writes them one by one.

Transmission doesn’t think that. Your operating system does. The POSIX implementation of tr_sys_path_rename is just a thin wrapper around rename(2). If rename(2) failed to create a hard link at the new location, Transmission falls back to a read-write loop.

I am working on some improvements to that. See master...RobCrowston:kernelcopy-wip

I don’t use Linux very often so I am happy to take feedback.

This is actually pretty nice, and I abuse it by having the incomplete-vs-finished dirs on different subvolumes. This greatly reduces space waste due to fragmented misaligned extents.

I use the same defragmentation trick (copy to a different volume after completion) on my machine, since I use ZFS. My ultimate goal is to make the file copy asynchronous, but that requires more work.

@pyrovski
Copy link
Contributor

pyrovski commented Jan 5, 2020

Making file copy asynchronous seems rather unlikely in the existing codebase without substantial improvements in locking discipline. Currently, moving torrent data locks the entire session, which prevents basically anything else from happening. setLocation() calls tr_torrentLock(), which simply calls tr_sessionLock(). There's a similar situation going on with the peer manager lock in peer-mgr.c; locking the peer manager locks the entire session.

In order to make moves asynchronous, we need the ability to lock a single torrent without locking the session. It's unclear how much the existing code assumes the session is locked while holding a torrent lock.

@pyrovski
Copy link
Contributor

pyrovski commented Jan 6, 2020

Looking at the code a bit more, there is a sticking point on the design of the per-session pool of open files. setLocation() is mostly torrent-specific, but it cleans up open files in the session, so it needs a session lock. I think this could be solved with the addition of a new isMoving state bit in the torrent struct.

@ckerr
Copy link
Member

ckerr commented May 17, 2020

If someone using BTRFS want to take this on as an improvement project, I'd love to see a patch and would be happy to review it

@Sepero
Copy link

Sepero commented Jun 10, 2023

This is actually pretty nice, and I abuse it by having the incomplete-vs-finished dirs on different subvolumes. This greatly reduces space waste due to fragmented misaligned extents.

Transmission's write patterns play with btrfs so badly that a torrent may take ~4 times the space. Such files are also damn slow to read. Rewriting the file turns this pathological layout to optimal, fully linear, big extented layout. On the other hand, mv merely relinks the inode to another directory, without improving the file.

You can test if my theory of your problem is right by running compsize on a file that's nearly finished. If the "Disk Usage" field is higher than "Referenced", that's it.

Wow, that's neat. I'm having a problem with finished torrents being fragmented and this may just solve it without needing to create an extra unnecessary filesystem

To @leijurv I'm sorry about your problem. Perhaps you can do a work around with the configuration option script-torrent-done-enabled
https://github.com/transmission/transmission/blob/main/docs/Editing-Configuration-Files.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr welcome type:feat A new feature type:perf A code change that improves performance
Development

No branches or pull requests

6 participants