Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#5102 closed Bug (fixed)

Don't invalidate OS' files cache when closing files

Reported by: Bogdar Owned by: jordan
Priority: Normal Milestone: 2.77
Component: libtransmission Version: 2.73
Severity: Normal Keywords:
Cc:

Description

Hello.

OS: Debian 6, 64bit, linux kernel based on 2.6.32.59 Trasmission 2.71 Build options are: --enable-cli --enable-daemon --enable-external-natpmp

I have fileserver serving our software and other files (over 200Gb data on disk). Server have 40GB RAM and run both nginx web-server and rtorrent serving same set of files. Usually I have ~38Gb in disk cache on this box and 1-2% of iowait with 0 free memory. Yesterday I start transmission as prospected replace for rtorrent with same data. After one hour with transmission outgoing traffic drop from ~120Mb/sec to 50Mb/sec, iowait grow from 2% to 10-15%, disk cache reduced from 38Gb to ~18-20Gb. Today I stop transmission daemon and in 30 minutes disk cache grow from 17Gb to 29Gb and keep growing now, iowait fall to 5% and network traffic increased from 45Mb/sec to 130Mb/sec I test this few times and each time I start transmission disk cache decrease and free memory increase. When I stop transmission, disk cache grows and free memory decrease. Outgoing traffic and iowait also changes as mentioned below.

All the time both nginx and rtorrent was running.

Value of "prefetch-enabled" and "cache-size-mb" (I try with 10240) does not impact

settings are blow:

{

"alt-speed-down": 50, "alt-speed-enabled": false, "alt-speed-time-begin": 540, "alt-speed-time-day": 127, "alt-speed-time-enabled": false, "alt-speed-time-end": 1020, "alt-speed-up": 50, "bind-address-ipv4": "0.0.0.0", "bind-address-ipv6": "::", "blocklist-enabled": false, "blocklist-url": "http://www.example.com/blocklist", "cache-size-mb": 10240, "dht-enabled": true, "download-dir": "/torrents/files/", "download-limit": 100, "download-limit-enabled": 0, "download-queue-enabled": true, "download-queue-size": 5, "encryption": 0, "idle-seeding-limit": 30, "idle-seeding-limit-enabled": false, "incomplete-dir": "/torrents/files/", "incomplete-dir-enabled": false, "lazy-bitfield-enabled": true, "lpd-enabled": false, "max-peers-global": 1024, "message-level": 2, "open-file-limit": 32768, "peer-congestion-algorithm": "", "peer-limit-global": 2048, "peer-limit-per-torrent": 1024, "peer-port": 51413, "peer-port-random-high": 65535, "peer-port-random-low": 49152, "peer-port-random-on-start": false, "peer-socket-tos": "default", "pex-enabled": true, "port-forwarding-enabled": false, "preallocation": 1, "prefetch-enabled": 0, "proxy": "", "proxy-auth-enabled": false, "proxy-auth-password": "", "proxy-auth-username": "", "proxy-enabled": false, "proxy-port": 80, "proxy-type": 0, "queue-stalled-enabled": true, "queue-stalled-minutes": 30, "ratio-limit": 2, "ratio-limit-enabled": false, "rename-partial-files": true, "rpc-authentication-required": true, "rpc-bind-address": "0.0.0.0", "rpc-enabled": true, "rpc-password": ".........", "rpc-port": 9091, "rpc-url": "/transmission/", "rpc-username": "transmission", "rpc-whitelist": "127.0.0.1", "rpc-whitelist-enabled": true, "scrape-paused-torrents-enabled": true, "script-torrent-done-enabled": false, "script-torrent-done-filename": "", "seed-queue-enabled": false, "seed-queue-size": 10, "speed-limit-down": 100, "speed-limit-down-enabled": false, "speed-limit-up": 100, "speed-limit-up-enabled": false, "start-added-torrents": true, "trash-original-torrent-files": false, "umask": 18, "upload-limit": 100, "upload-limit-enabled": 0, "upload-slots-per-torrent": 1024, "utp-enabled": false, "watch-dir": "/torrents/watch", "watch-dir-enabled": true

}

Currently transmission daemon 2.71 looks unusable for production usage.

Change History (19)

comment:1 Changed 8 years ago by jch

You might want to try the patch attached to #3371. (I have no idea whether it still applies cleanly.)

--jch

comment:2 Changed 8 years ago by jordan

I benchmarked peppering POSIX_FADV_DONTNEED hints into fdlimit.c, but the result was to slow down the reads by over half.

But, Bogdar's talking about a fairly large time window -- half an hour. Maybe fdlimit.c could periodically close the cached files that haven't been accessed in the last N minutes.

comment:3 Changed 8 years ago by Bogdar

Here is a dirty hack which works well for me.

--- transmission-2.71.orig/libtransmission/fdlimit.c
+++ transmission-2.71/libtransmission/fdlimit.c
@@ -237,7 +237,7 @@ int
 tr_prefetch( int fd UNUSED, off_t offset UNUSED, size_t count UNUSED )
 {
 #ifdef HAVE_POSIX_FADVISE
-    return posix_fadvise( fd, offset, count, POSIX_FADV_WILLNEED );
+//    return posix_fadvise( fd, offset, count, POSIX_FADV_WILLNEED );
 #elif defined(SYS_DARWIN)
     struct radvisory radv;
     radv.ra_offset = offset;
@@ -257,7 +257,7 @@ tr_set_file_for_single_pass( int fd )
            for these to fail silently, so don't let them affect errno */
         const int err = errno;
 #ifdef HAVE_POSIX_FADVISE
-        posix_fadvise( fd, 0, 0, POSIX_FADV_SEQUENTIAL );
+//        posix_fadvise( fd, 0, 0, POSIX_FADV_SEQUENTIAL );
 #endif
 #ifdef SYS_DARWIN
         fcntl( fd, F_RDAHEAD, 1 );
@@ -291,9 +291,9 @@ tr_close_file( int fd )
 #if defined(HAVE_POSIX_FADVISE)
     /* Set hint about not caching this file.
        It's okay for this to fail silently, so don't let it affect errno */
-    const int err = errno;
-    posix_fadvise( fd, 0, 0, POSIX_FADV_DONTNEED );
-    errno = err;
+//    const int err = errno;
+//    posix_fadvise( fd, 0, 0, POSIX_FADV_DONTNEED );
+//    errno = err;
 #endif
 #ifdef SYS_DARWIN
     /* it's unclear to me from the man pages if this actually flushes out the cache,
--- transmission-2.71.orig/libtransmission/verify.c
+++ transmission-2.71/libtransmission/verify.c
@@ -93,7 +93,7 @@ verifyTorrent( tr_torrent * tor, bool *
                 bytesThisPass = (uint32_t)numRead;
                 SHA1_Update( &sha, buffer, bytesThisPass );
 #if defined HAVE_POSIX_FADVISE && defined POSIX_FADV_DONTNEED
-                posix_fadvise( fd, filePos, bytesThisPass, POSIX_FADV_DONTNEED );
+//                posix_fadvise( fd, filePos, bytesThisPass, POSIX_FADV_DONTNEED );
 #endif
             }
         }

To reproduce a bug, build and run unmodified transmission on Linux, add big torrent (for example, 3Gb of files for host with 4Gb RAM), server also that files also via HTTP server.

Try #1: 1st, download torrent from other host 2nd, run iostat for device which host data, 'iostat -k 15 /dev/sda' for example 3rd, run HTTP download from other host, compare download speed and 'iostat' output

Result: iostat show disk read activity at the same speed as download, because transmission prevent file caching in RAM Expected result: files were cached in RAM after 1st step, download via HTTP did not cause any disk reads.

Try #2: 1st, download torrent files via HTTP, check 'Cached' value in 'free' or 'top' output, it should be no less then torrent files size. 2nd, download same files via trasmission, check 'iostat' - there will be read activity, also check 'Cached' - it will sacrificially decrease. 3rd, download again via HTTP - there will be disk reads and filesystem cache grow as expected.

My hack listed above solved this issue for me.

comment:4 Changed 8 years ago by jordan

Those changes seem to slow down disk reads:

Test 1: trunk

After 44400 actions, 402.1 MiB of disk IO in 5723 msec (score: 73675.037568)
After 44500 actions, 403.1 MiB of disk IO in 5766 msec (score: 73304.618800)
After 44600 actions, 404.2 MiB of disk IO in 5770 msec (score: 73458.246101)
After 44700 actions, 405.0 MiB of disk IO in 5772 msec (score: 73574.719335)
After 44800 actions, 405.8 MiB of disk IO in 5779 msec (score: 73627.354214)
After 44900 actions, 406.5 MiB of disk IO in 5821 msec (score: 73228.401993)
After 45000 actions, 407.9 MiB of disk IO in 5824 msec (score: 73438.241758)
After 45100 actions, 408.7 MiB of disk IO in 5848 msec (score: 73279.737346)
After 45200 actions, 409.5 MiB of disk IO in 5848 msec (score: 73419.819425)
After 45300 actions, 410.7 MiB of disk IO in 5894 msec (score: 73063.633526)
After 45400 actions, 411.5 MiB of disk IO in 5897 msec (score: 73173.716805)
After 45500 actions, 412.2 MiB of disk IO in 5899 msec (score: 73276.669266)

Test 2: trunk minus POSIX_FADV_WILLNEED

After 36000 actions, 326.3 MiB of disk IO in 10437 msec (score: 32786.837597)
After 36100 actions, 327.2 MiB of disk IO in 10455 msec (score: 32818.147107)
After 36200 actions, 328.0 MiB of disk IO in 10501 msec (score: 32752.397676)
After 36300 actions, 329.2 MiB of disk IO in 10534 msec (score: 32766.444655)
After 36400 actions, 330.2 MiB of disk IO in 10572 msec (score: 32750.952705)
After 36500 actions, 331.3 MiB of disk IO in 10579 msec (score: 32840.790245)
After 36600 actions, 332.1 MiB of disk IO in 10600 msec (score: 32854.556981)
After 36700 actions, 332.8 MiB of disk IO in 10644 msec (score: 32784.931980)

Test 3: trunk minus POSIX_FADV_SEQUENTIAL

After 32900 actions, 296.4 MiB of disk IO in 4507 msec (score: 68949.484801)
After 33000 actions, 297.1 MiB of disk IO in 4509 msec (score: 69089.681969)
After 33100 actions, 297.9 MiB of disk IO in 4509 msec (score: 69285.897538)
After 33200 actions, 299.2 MiB of disk IO in 4510 msec (score: 69553.894457)
After 33300 actions, 299.9 MiB of disk IO in 4510 msec (score: 69735.535255)
After 33400 actions, 300.7 MiB of disk IO in 4510 msec (score: 69917.176053)
After 33500 actions, 302.0 MiB of disk IO in 4649 msec (score: 68122.761884)
After 33600 actions, 302.7 MiB of disk IO in 4652 msec (score: 68223.229579)

comment:5 Changed 8 years ago by cfpp2p

Here is a forum post that I don't necessarily agree with. However it does provide a quick way to test without re-compiling the disabling of POSIX_FADVISE. For those interested see the post's two 'code' sections.

https://forum.transmissionbt.com/viewtopic.php?f=1&t=14221&sid=90d2dea8cbac0c1568356611eace8605

comment:6 Changed 8 years ago by jordan

Bogdar, could you repeat your test with a patch that only removes POSIX_FADV_DONTNEED? I'm suspecting that's the culprit for the http + BitTorrent? scenario you described in comment:3.

comment:7 Changed 8 years ago by jordan

Bogdar, ping

comment:8 Changed 8 years ago by x190

https://forum.transmissionbt.com/viewtopic.php?f=1&t=14221&sid=90d2dea8cbac0c1568356611eace8605#p63029

fdlimit.c:
==========
void
tr_close_file (int fd)
{
#if defined (HAVE_POSIX_FADVISE)
  /* Set hint about not caching this file.
     It's okay for this to fail silently, so don't let it affect errno */
  const int err = errno;
  posix_fadvise (fd, 0, 0, POSIX_FADV_DONTNEED);
  errno = err;
#endif
#ifdef SYS_DARWIN
  /* it's unclear to me from the man pages if this actually flushes out the cache,
   * but it couldn't hurt... */
  fcntl (fd, F_NOCACHE, 1);
#endif
  close (fd);
}

What if we need this file for seeding? Why not let the OS handle memory management at this point? Anyway, I think that is what the forum poster is getting at.

comment:9 Changed 8 years ago by jordan

x190: because https://www.google.com/search?q=inactive+memory+transmission

Actually the forum poster is talking about a more self-contained problem: when a file becomes complete, libtransmission closes the current read/write fd so that, if it's needed in the future, it can be reopened in read-only mode.

This ticket brings other apps into the mix, that libtransmission shouldn't flush the cache because the content might be needed by a web server. IMO that's a much rarer case.

comment:10 follow-up: Changed 8 years ago by jordan

x190: what I'm thinking about for this ticket is a non-GUI config option in settings.json to disable flushing the cache in tr_close_file(), which is why I was asking Bogdar to confirm that tr_close_file() is the culprit.

There doesn't seem to be a posix-friendly way of removing write permissions from an fd's open mode, so wrt the forum post, it might be worthwhile to just remove the code that closes the file when it becomes complete. I'm not sure what implications, if any, that has for other features such as removing the ".part" suffix from the end of the file.

comment:11 in reply to: ↑ 10 Changed 8 years ago by x190

Replying to jordan:

x190: what I'm thinking about for this ticket is a non-GUI config option in settings.json to disable flushing the cache in tr_close_file(), which is why I was asking Bogdar to confirm that tr_close_file() is the culprit.

There doesn't seem to be a posix-friendly way of removing write permissions from an fd's open mode, so wrt the forum post, it might be worthwhile to just remove the code that closes the file when it becomes complete. I'm not sure what implications, if any, that has for other features such as removing the ".part" suffix from the end of the file.

Currently testing with a 1.5 GB download and I see no memory issues at all as it d/ls at 500 KB. It does not 'eat' Free Memory or drastically increase Inactive Memory, but, in fact, balances everything within about 50MB. I'll watch to see what happens when it completes.

In general, I think F_NOCACHE on Darwin is only applicable to when you open a file. Turn it off if you need to re-use the data or on if data is only needed once. Therefore, I would suggest removing it completely from tr_close_file() where, if it has any effect at all, it is giving the wrong "hint" to the OS.

Okay, download is now complete. About a minute after the transfer switched to seed mode, memory usage increased and disk reads are roughly equivalent to upload speed. VM size dropped (by 3 GB), Free Memory dropped and continues to drop considerably, and Inactive Memory increased and continues to increase, all pretty much the opposite of what you would want to achieve, I guess.

I'm wondering if F_NOCACHE is used appropriately vis-à-vis opening a file for seeding? Is its use in tr_set_file_for_single_pass() always appropriate? There does appear to be memory issues with seed mode that are not seen during downloading. I don't think complaints about 'inactive' memory are valid anymore even if they were semi-valid years ago and should not, in themselves, have anything to do with application caching management.

Last edited 8 years ago by x190 (previous) (diff)

comment:12 Changed 8 years ago by x190

Verdict is in. F_NOCACHE must go from tr_close_file(). I repeated the test above with F_NOCACHE compiled out and all seed mode problems disappeared. Free memory remained rock solid as did inactive memory, while disk reads fell to basically 0 and as an added bonus Prairie Crocuses sprung from the snow banks! Seriously, it appears putting F_NOCACHE in tr_close_file() was a colossal mistake and any memory issues were likely due to users opening a lot of memory-hogging apps that they neglected to mention. It's also quite possible Apple has improved memory management over the years. This is the first time I've done a proper test noting all values while d/ling and then seeding only the test torrent and being careful not to open additional apps during the test.

comment:13 Changed 8 years ago by jordan

  • Milestone changed from None Set to 2.80
  • Owner set to jordan
  • Severity changed from Critical to Normal
  • Status changed from new to assigned
  • Summary changed from Transission invalidate filesystem cache in Linux, this cause critical performace degradation to Don't invalidate OS' files cache when closing files

Can't say no to Prairie Crocuses...

Thanks for doing the OS X testing. Knowing that this doesn't revive the perennial "Inactive Memory" issue seals the deal.

Fixed r13803.

comment:14 Changed 8 years ago by jordan

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:15 follow-up: Changed 8 years ago by jordan

x190: note, I've only changed tr_close_file() here. I've left the other calls in as per the benchmark results in comment:4.

comment:16 in reply to: ↑ 15 Changed 8 years ago by x190

Replying to jordan:

x190: note, I've only changed tr_close_file() here. I've left the other calls in as per the benchmark results in comment:4.

Okay. That other stuff is Linux only, afaict.

Last edited 8 years ago by x190 (previous) (diff)

comment:17 Changed 8 years ago by jordan

r13920: backported bug #5102 fix from r13803 to the 2.7x branch

comment:18 Changed 8 years ago by jordan

  • Milestone changed from 2.80 to 2.77

comment:19 Changed 8 years ago by jordan

  • Component changed from Transmission to libtransmission
Note: See TracTickets for help on using tickets.