From d5c230858757b48d2e7f9a54810c1386c064d51b Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Tue, 19 Nov 2019 17:34:52 +0100 Subject: [PATCH] Fixed: File imports on cloud drives slow due to transaction logic --- .../DiskTests/DiskTransferServiceFixture.cs | 43 +++++++++++++++++++ .../DiskTests/MockMount.cs | 30 +++++++++++++ .../Disk/DiskTransferService.cs | 24 +++++++++-- 3 files changed, 93 insertions(+), 4 deletions(-) create mode 100644 src/NzbDrone.Common.Test/DiskTests/MockMount.cs diff --git a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs index ae804b01e..1e4974bc8 100644 --- a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs +++ b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs @@ -20,15 +20,37 @@ public class DiskTransferServiceFixture : TestBase private readonly string _tempTargetPath = @"C:\target\my.video.mkv.partial~".AsOsAgnostic(); private readonly string _nfsFile = ".nfs01231232"; + private MockMount _sourceMount; + private MockMount _targetMount; + [SetUp] public void SetUp() { Mocker.GetMock(MockBehavior.Strict); + _sourceMount = new MockMount() + { + Name = "source", + RootDirectory = @"C:\source".AsOsAgnostic() + }; + _targetMount = new MockMount() + { + Name = "target", + RootDirectory = @"C:\target".AsOsAgnostic() + }; + Mocker.GetMock() .Setup(v => v.GetMount(It.IsAny())) .Returns((IMount)null); + Mocker.GetMock() + .Setup(v => v.GetMount(It.Is(p => p.StartsWith(_sourceMount.RootDirectory)))) + .Returns(_sourceMount); + + Mocker.GetMock() + .Setup(v => v.GetMount(It.Is(p => p.StartsWith(_targetMount.RootDirectory)))) + .Returns(_targetMount); + WithEmulatedDiskProvider(); WithExistingFile(_sourcePath); @@ -58,6 +80,27 @@ public void should_not_use_verified_transfer_on_windows() .Verify(v => v.MoveFile(_sourcePath, _targetPath, false), Times.Once()); } + [TestCase("fuse.mergerfs")] + [TestCase("fuse.rclone")] + [TestCase("mergerfs")] + [TestCase("rclone")] + public void should_not_use_verified_transfer_on_specific_filesystems(string fs) + { + MonoOnly(); + + _targetMount.DriveFormat = fs; + + Subject.VerificationMode.Should().Be(DiskTransferVerificationMode.VerifyOnly); + + var result = Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move); + + Mocker.GetMock() + .Verify(v => v.TryCreateHardLink(_sourcePath, _backupPath), Times.Never()); + + Mocker.GetMock() + .Verify(v => v.MoveFile(_sourcePath, _targetPath, false), Times.Once()); + } + [Test] public void should_throw_if_path_is_the_same() { diff --git a/src/NzbDrone.Common.Test/DiskTests/MockMount.cs b/src/NzbDrone.Common.Test/DiskTests/MockMount.cs new file mode 100644 index 000000000..a13679ccb --- /dev/null +++ b/src/NzbDrone.Common.Test/DiskTests/MockMount.cs @@ -0,0 +1,30 @@ +using System.IO; +using NzbDrone.Common.Disk; + +namespace NzbDrone.Common.Test.DiskTests +{ + public class MockMount : IMount + { + public long AvailableFreeSpace { get; set; } + + public string DriveFormat { get; set; } + + public DriveType DriveType { get; set; } = DriveType.Fixed; + + public bool IsReady { get; set; } = true; + + public MountOptions MountOptions { get; set; } + + public string Name { get; set; } + + public string RootDirectory { get; set; } + + public long TotalFreeSpace { get; set; } + + public long TotalSize { get; set; } + + public string VolumeLabel { get; set; } + + public string VolumeName { get; set; } + } +} diff --git a/src/NzbDrone.Common/Disk/DiskTransferService.cs b/src/NzbDrone.Common/Disk/DiskTransferService.cs index a2d54ec04..ee4c89ad6 100644 --- a/src/NzbDrone.Common/Disk/DiskTransferService.cs +++ b/src/NzbDrone.Common/Disk/DiskTransferService.cs @@ -286,15 +286,31 @@ public TransferMode TransferFile(string sourcePath, string targetPath, TransferM } } - // We force a transactional transfer if the transfer occurs between mounts and one of the mounts is cifs, it would be a copy anyway. - if (verificationMode == DiskTransferVerificationMode.TryTransactional && OsInfo.IsNotWindows) + // Adjust the transfer mode depending on the filesystems + if (verificationMode == DiskTransferVerificationMode.TryTransactional) { var sourceMount = _diskProvider.GetMount(sourcePath); var targetMount = _diskProvider.GetMount(targetPath); - if (sourceMount != null && targetMount != null && sourceMount.RootDirectory != targetMount.RootDirectory && - (sourceMount.DriveFormat == "cifs" || targetMount.DriveFormat == "cifs")) + var isSameMount = (sourceMount != null && targetMount != null && sourceMount.RootDirectory == targetMount.RootDirectory); + + var sourceDriveFormat = sourceMount?.DriveFormat ?? string.Empty; + var targetDriveFormat = targetMount?.DriveFormat ?? string.Empty; + + if (isSameMount) { + // No transaction needed for operations on same mount, force VerifyOnly + verificationMode = DiskTransferVerificationMode.VerifyOnly; + } + else if (sourceDriveFormat.Contains("mergerfs") || sourceDriveFormat.Contains("rclone") || + targetDriveFormat.Contains("mergerfs") || targetDriveFormat.Contains("rclone")) + { + // Cloud storage filesystems don't need any Transactional stuff and it hurts performance, force VerifyOnly + verificationMode = DiskTransferVerificationMode.VerifyOnly; + } + else if ((sourceDriveFormat == "cifs" || targetDriveFormat == "cifs") && OsInfo.IsNotWindows) + { + // Force Transactional on a cifs mount due to the likeliness of move failures on certain scenario's on mono verificationMode = DiskTransferVerificationMode.Transactional; } }