From bfa629f4315fc06b65b657db2b8b5a5be7b8a639 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Thu, 16 Jul 2015 17:10:39 +0200 Subject: [PATCH] Refactored VerifiedFile transfer to have a verified and transactional mode. --- .../DiskTests/DiskTransferServiceFixture.cs | 457 +++++++++++++++--- .../Disk/DiskTransferService.cs | 224 +++++++-- 2 files changed, 572 insertions(+), 109 deletions(-) diff --git a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs index 36300a2ee..669449e44 100644 --- a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs +++ b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs @@ -29,6 +29,131 @@ namespace NzbDrone.Common.Test.DiskTests WithExistingFile(_sourcePath); } + [Test] + public void should_use_verified_transfer_on_mono() + { + MonoOnly(); + + Subject.VerificationMode.Should().Be(DiskTransferVerificationMode.Transactional); + } + + [Test] + public void should_not_use_verified_transfer_on_windows() + { + WindowsOnly(); + + 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() + { + Assert.Throws(() => Subject.TransferFile(_sourcePath, _sourcePath, TransferMode.HardLink)); + + Mocker.GetMock() + .Verify(v => v.TryCreateHardLink(_sourcePath, _sourcePath), Times.Never()); + } + + [Test] + public void should_throw_if_different_casing_unless_moving() + { + var targetPath = Path.Combine(Path.GetDirectoryName(_sourcePath), Path.GetFileName(_sourcePath).ToUpper()); + + Assert.Throws(() => Subject.TransferFile(_sourcePath, targetPath, TransferMode.HardLink)); + } + + [Test] + public void should_rename_via_temp_if_different_casing() + { + var targetPath = Path.Combine(Path.GetDirectoryName(_sourcePath), Path.GetFileName(_sourcePath).ToUpper()); + + Mocker.GetMock() + .Setup(v => v.MoveFile(_sourcePath, _backupPath, true)) + .Callback(() => + { + WithExistingFile(_backupPath, true); + WithExistingFile(_sourcePath, false); + }); + + Mocker.GetMock() + .Setup(v => v.MoveFile(_backupPath, targetPath, false)) + .Callback(() => + { + WithExistingFile(targetPath, true); + WithExistingFile(_backupPath, false); + }); + + var result = Subject.TransferFile(_sourcePath, targetPath, TransferMode.Move); + + Mocker.GetMock() + .Verify(v => v.MoveFile(_backupPath, targetPath, false), Times.Once()); + } + + [Test] + public void should_rollback_rename_via_temp_on_exception() + { + var targetPath = Path.Combine(Path.GetDirectoryName(_sourcePath), Path.GetFileName(_sourcePath).ToUpper()); + + Mocker.GetMock() + .Setup(v => v.MoveFile(_sourcePath, _backupPath, true)) + .Callback(() => + { + WithExistingFile(_backupPath, true); + WithExistingFile(_sourcePath, false); + }); + + Mocker.GetMock() + .Setup(v => v.MoveFile(_backupPath, targetPath, false)) + .Throws(new IOException("Access Violation")); + + Assert.Throws(() => Subject.TransferFile(_sourcePath, targetPath, TransferMode.Move)); + + Mocker.GetMock() + .Verify(v => v.MoveFile(_backupPath, _sourcePath, false), Times.Once()); + } + + [Test] + public void should_log_error_if_rollback_move_fails() + { + var targetPath = Path.Combine(Path.GetDirectoryName(_sourcePath), Path.GetFileName(_sourcePath).ToUpper()); + + Mocker.GetMock() + .Setup(v => v.MoveFile(_sourcePath, _backupPath, true)) + .Callback(() => + { + WithExistingFile(_backupPath, true); + WithExistingFile(_sourcePath, false); + }); + + Mocker.GetMock() + .Setup(v => v.MoveFile(_backupPath, targetPath, false)) + .Throws(new IOException("Access Violation")); + + Mocker.GetMock() + .Setup(v => v.MoveFile(_backupPath, _sourcePath, false)) + .Throws(new IOException("Access Violation")); + + Assert.Throws(() => Subject.TransferFile(_sourcePath, targetPath, TransferMode.Move)); + + ExceptionVerification.ExpectedErrors(1); + } + + [Test] + public void should_throw_if_destination_is_child_of_source() + { + var childPath = Path.Combine(_sourcePath, "child"); + + Assert.Throws(() => Subject.TransferFile(_sourcePath, childPath, TransferMode.Move)); + } + [Test] public void should_hardlink_only() { @@ -48,23 +173,257 @@ namespace NzbDrone.Common.Test.DiskTests } [Test] - public void should_not_use_verified_transfer_on_windows() + public void should_fallback_to_copy_if_hardlink_failed() { - WindowsOnly(); + Subject.VerificationMode = DiskTransferVerificationMode.Transactional; + + WithFailedHardlink(); var result = Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move); Mocker.GetMock() - .Verify(v => v.TryCreateHardLink(_sourcePath, _backupPath), Times.Never()); + .Verify(v => v.CopyFile(_sourcePath, _tempTargetPath, false), Times.Once()); + + Mocker.GetMock() + .Verify(v => v.MoveFile(_tempTargetPath, _targetPath, false), Times.Once()); + + VerifyDeletedFile(_sourcePath); + } + + [Test] + public void mode_none_should_not_verify_copy() + { + Subject.VerificationMode = DiskTransferVerificationMode.None; + + Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Copy); + + Mocker.GetMock() + .Verify(v => v.CopyFile(_sourcePath, _targetPath, false), Times.Once()); + + Mocker.GetMock() + .Verify(v => v.GetFileSize(It.IsAny()), Times.Never()); + } + + [Test] + public void mode_none_should_not_verify_move() + { + Subject.VerificationMode = DiskTransferVerificationMode.None; + + Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move); + + Mocker.GetMock() + .Verify(v => v.MoveFile(_sourcePath, _targetPath, false), Times.Once()); + + Mocker.GetMock() + .Verify(v => v.GetFileSize(It.IsAny()), Times.Never()); + } + + [Test] + public void mode_none_should_delete_existing_target_when_overwriting() + { + Subject.VerificationMode = DiskTransferVerificationMode.None; + + WithExistingFile(_targetPath); + + Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move, true); + + Mocker.GetMock() + .Verify(v => v.DeleteFile(_targetPath), Times.Once()); Mocker.GetMock() .Verify(v => v.MoveFile(_sourcePath, _targetPath, false), Times.Once()); } [Test] - public void should_retry_if_partial_copy() + public void mode_none_should_throw_if_existing_target_when_not_overwriting() { - MonoOnly(); + Subject.VerificationMode = DiskTransferVerificationMode.None; + + WithExistingFile(_targetPath); + + Assert.Throws(() => Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move, false)); + + Mocker.GetMock() + .Verify(v => v.DeleteFile(_targetPath), Times.Never()); + + Mocker.GetMock() + .Verify(v => v.MoveFile(_sourcePath, _targetPath, false), Times.Never()); + } + + [Test] + public void mode_verifyonly_should_verify_copy() + { + Subject.VerificationMode = DiskTransferVerificationMode.VerifyOnly; + + Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Copy); + + Mocker.GetMock() + .Verify(v => v.GetFileSize(_sourcePath), Times.Once()); + + Mocker.GetMock() + .Verify(v => v.GetFileSize(_targetPath), Times.Once()); + } + + [Test] + public void mode_verifyonly_should_rollback_copy_on_partial_and_throw() + { + Subject.VerificationMode = DiskTransferVerificationMode.VerifyOnly; + + Mocker.GetMock() + .Setup(v => v.CopyFile(_sourcePath, _targetPath, false)) + .Callback(() => + { + WithExistingFile(_targetPath, true, 900); + }); + + Assert.Throws(() => Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Copy)); + + Mocker.GetMock() + .Verify(v => v.DeleteFile(_targetPath), Times.Once()); + } + + [Test] + public void should_log_error_if_rollback_copy_fails() + { + Subject.VerificationMode = DiskTransferVerificationMode.VerifyOnly; + + Mocker.GetMock() + .Setup(v => v.CopyFile(_sourcePath, _targetPath, false)) + .Callback(() => + { + WithExistingFile(_targetPath, true, 900); + }); + + Mocker.GetMock() + .Setup(v => v.DeleteFile(_targetPath)) + .Throws(new IOException("Access Violation")); + + Assert.Throws(() => Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Copy)); + + ExceptionVerification.ExpectedErrors(1); + } + + [Test] + public void mode_verifyonly_should_verify_move() + { + Subject.VerificationMode = DiskTransferVerificationMode.VerifyOnly; + + Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move); + + Mocker.GetMock() + .Verify(v => v.GetFileSize(_sourcePath), Times.Once()); + + Mocker.GetMock() + .Verify(v => v.GetFileSize(_targetPath), Times.Once()); + } + + [Test] + public void mode_verifyonly_should_not_rollback_move_on_partial_and_throw() + { + Subject.VerificationMode = DiskTransferVerificationMode.VerifyOnly; + + Mocker.GetMock() + .Setup(v => v.MoveFile(_sourcePath, _targetPath, false)) + .Callback(() => + { + WithExistingFile(_sourcePath, false); + WithExistingFile(_targetPath, true, 900); + }); + + Assert.Throws(() => Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move)); + + Mocker.GetMock() + .Verify(v => v.DeleteFile(_targetPath), Times.Never()); + + ExceptionVerification.ExpectedErrors(1); + } + + [Test] + public void mode_verifyonly_should_rollback_move_on_partial_if_source_remains() + { + Subject.VerificationMode = DiskTransferVerificationMode.VerifyOnly; + + Mocker.GetMock() + .Setup(v => v.MoveFile(_sourcePath, _targetPath, false)) + .Callback(() => + { + WithExistingFile(_targetPath, true, 900); + }); + + Assert.Throws(() => Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move)); + + Mocker.GetMock() + .Verify(v => v.DeleteFile(_targetPath), Times.Once()); + } + + [Test] + public void should_log_error_if_rollback_partialmove_fails() + { + Subject.VerificationMode = DiskTransferVerificationMode.VerifyOnly; + + Mocker.GetMock() + .Setup(v => v.MoveFile(_sourcePath, _targetPath, false)) + .Callback(() => + { + WithExistingFile(_targetPath, true, 900); + }); + + Mocker.GetMock() + .Setup(v => v.DeleteFile(_targetPath)) + .Throws(new IOException("Access Violation")); + + Assert.Throws(() => Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move)); + + ExceptionVerification.ExpectedErrors(1); + } + + [Test] + public void mode_transactional_should_delete_old_backup() + { + Subject.VerificationMode = DiskTransferVerificationMode.Transactional; + + WithExistingFile(_backupPath); + + WithSuccessfulHardlink(_sourcePath, _backupPath); + + Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move); + + Mocker.GetMock() + .Verify(v => v.DeleteFile(_backupPath), Times.Once()); + } + + [Test] + public void mode_transactional_should_delete_old_partial() + { + Subject.VerificationMode = DiskTransferVerificationMode.Transactional; + + WithExistingFile(_tempTargetPath); + + WithSuccessfulHardlink(_sourcePath, _backupPath); + + Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move); + + Mocker.GetMock() + .Verify(v => v.DeleteFile(_tempTargetPath), Times.Once()); + } + + [Test] + public void mode_transactional_should_hardlink_before_move() + { + Subject.VerificationMode = DiskTransferVerificationMode.Transactional; + + WithSuccessfulHardlink(_sourcePath, _backupPath); + + var result = Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move); + + Mocker.GetMock() + .Verify(v => v.TryCreateHardLink(_sourcePath, _backupPath), Times.Once()); + } + + [Test] + public void mode_transactional_should_retry_if_partial_copy() + { + Subject.VerificationMode = DiskTransferVerificationMode.Transactional; var retry = 0; Mocker.GetMock() @@ -81,9 +440,9 @@ namespace NzbDrone.Common.Test.DiskTests } [Test] - public void should_retry_twice_if_partial_copy() + public void mode_transactional_should_retry_twice_if_partial_copy() { - MonoOnly(); + Subject.VerificationMode = DiskTransferVerificationMode.Transactional; var retry = 0; Mocker.GetMock() @@ -101,22 +460,9 @@ namespace NzbDrone.Common.Test.DiskTests } [Test] - public void should_hardlink_before_move() + public void mode_transactional_should_remove_source_after_move() { - MonoOnly(); - - WithSuccessfulHardlink(_sourcePath, _backupPath); - - var result = Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move); - - Mocker.GetMock() - .Verify(v => v.TryCreateHardLink(_sourcePath, _backupPath), Times.Once()); - } - - [Test] - public void should_remove_source_after_move() - { - MonoOnly(); + Subject.VerificationMode = DiskTransferVerificationMode.Transactional; WithSuccessfulHardlink(_sourcePath, _backupPath); @@ -130,9 +476,9 @@ namespace NzbDrone.Common.Test.DiskTests } [Test] - public void should_not_remove_source_if_partial_still_exists() + public void mode_transactional_should_not_remove_source_if_partial_still_exists() { - MonoOnly(); + Subject.VerificationMode = DiskTransferVerificationMode.Transactional; var targetPath = Path.Combine(Path.GetDirectoryName(_targetPath), Path.GetFileName(_targetPath).ToUpper()); var tempTargetPath = targetPath + ".partial~"; @@ -156,36 +502,29 @@ namespace NzbDrone.Common.Test.DiskTests } [Test] - public void should_rename_via_temp() + public void mode_transactional_should_remove_partial_if_copy_fails() { - var targetPath = Path.Combine(Path.GetDirectoryName(_sourcePath), Path.GetFileName(_sourcePath).ToUpper()); + Subject.VerificationMode = DiskTransferVerificationMode.Transactional; + + WithSuccessfulHardlink(_sourcePath, _backupPath); Mocker.GetMock() - .Setup(v => v.MoveFile(_sourcePath, _backupPath, false)) - .Callback(() => - { - WithExistingFile(_backupPath, true); - WithExistingFile(_sourcePath, false); - }); - - Mocker.GetMock() - .Setup(v => v.MoveFile(_backupPath, targetPath, false)) + .Setup(v => v.CopyFile(_sourcePath, _tempTargetPath, false)) .Callback(() => { - WithExistingFile(targetPath, true); - WithExistingFile(_backupPath, false); - }); + WithExistingFile(_tempTargetPath, true, 900); + }) + .Throws(new IOException("Blackbox IO error")); - var result = Subject.TransferFile(_sourcePath, targetPath, TransferMode.Move); - - Mocker.GetMock() - .Verify(v => v.MoveFile(_backupPath, targetPath, false), Times.Once()); + Assert.Throws(() => Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Copy)); + + VerifyDeletedFile(_tempTargetPath); } [Test] - public void should_remove_backup_if_move_throws() + public void mode_transactional_should_remove_backup_if_move_throws() { - MonoOnly(); + Subject.VerificationMode = DiskTransferVerificationMode.Transactional; WithSuccessfulHardlink(_sourcePath, _backupPath); @@ -199,9 +538,9 @@ namespace NzbDrone.Common.Test.DiskTests } [Test] - public void should_remove_partial_if_move_fails() + public void mode_transactional_should_remove_partial_if_move_fails() { - MonoOnly(); + Subject.VerificationMode = DiskTransferVerificationMode.Transactional; WithSuccessfulHardlink(_sourcePath, _backupPath); @@ -218,24 +557,6 @@ namespace NzbDrone.Common.Test.DiskTests VerifyDeletedFile(_tempTargetPath); } - [Test] - public void should_fallback_to_copy_if_hardlink_failed() - { - MonoOnly(); - - WithFailedHardlink(); - - var result = Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move); - - Mocker.GetMock() - .Verify(v => v.CopyFile(_sourcePath, _tempTargetPath, false), Times.Once()); - - Mocker.GetMock() - .Verify(v => v.MoveFile(_tempTargetPath, _targetPath, false), Times.Once()); - - VerifyDeletedFile(_sourcePath); - } - [Test] public void CopyFolder_should_copy_folder() { @@ -310,14 +631,6 @@ namespace NzbDrone.Common.Test.DiskTests Assert.Throws(() => Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Copy)); } - [Test] - public void should_throw_if_destination_is_child_of_source() - { - var childPath = Path.Combine(_sourcePath, "child"); - - Assert.Throws(() => Subject.TransferFile(_sourcePath, childPath, TransferMode.Move)); - } - public DirectoryInfo GetFilledTempFolder() { var tempFolder = GetTempFilePath(); diff --git a/src/NzbDrone.Common/Disk/DiskTransferService.cs b/src/NzbDrone.Common/Disk/DiskTransferService.cs index 3feca3800..80647d61f 100644 --- a/src/NzbDrone.Common/Disk/DiskTransferService.cs +++ b/src/NzbDrone.Common/Disk/DiskTransferService.cs @@ -17,6 +17,13 @@ namespace NzbDrone.Common.Disk TransferMode TransferFile(String sourcePath, String targetPath, TransferMode mode, bool overwrite = false, bool verified = true); } + public enum DiskTransferVerificationMode + { + None, + VerifyOnly, + Transactional + } + public class DiskTransferService : IDiskTransferService { private const Int32 RetryCount = 2; @@ -24,10 +31,16 @@ namespace NzbDrone.Common.Disk private readonly IDiskProvider _diskProvider; private readonly Logger _logger; + public DiskTransferVerificationMode VerificationMode { get; set; } + public DiskTransferService(IDiskProvider diskProvider, Logger logger) { _diskProvider = diskProvider; _logger = logger; + + // TODO: Atm we haven't seen partial transfers on windows so we disable verified transfer. + // (If enabled in the future, be sure to check specifically for ReFS, which doesn't support hardlinks.) + VerificationMode = OsInfo.IsWindows ? DiskTransferVerificationMode.VerifyOnly : DiskTransferVerificationMode.Transactional; } public TransferMode TransferFolder(String sourcePath, String targetPath, TransferMode mode, bool verified = true) @@ -35,10 +48,8 @@ namespace NzbDrone.Common.Disk Ensure.That(sourcePath, () => sourcePath).IsValidPath(); Ensure.That(targetPath, () => targetPath).IsValidPath(); - if (OsInfo.IsWindows) + if (VerificationMode != DiskTransferVerificationMode.Transactional) { - // TODO: Atm we haven't seen partial transfers on windows so we disable verified transfer. - // (If enabled in the future, be sure to check specifically for ReFS, which doesn't support hardlinks.) verified = false; } @@ -74,10 +85,8 @@ namespace NzbDrone.Common.Disk Ensure.That(sourcePath, () => sourcePath).IsValidPath(); Ensure.That(targetPath, () => targetPath).IsValidPath(); - if (OsInfo.IsWindows) + if (VerificationMode != DiskTransferVerificationMode.Transactional) { - // TODO: Atm we haven't seen partial transfers on windows so we disable verified transfer. - // (If enabled in the future, be sure to check specifically for ReFS, which doesn't support hardlinks.) verified = false; } @@ -98,15 +107,21 @@ namespace NzbDrone.Common.Disk if (mode.HasFlag(TransferMode.Move)) { var tempPath = sourcePath + ".backup~"; - _diskProvider.MoveFile(sourcePath, tempPath); - if (_diskProvider.FileExists(targetPath)) + _diskProvider.MoveFile(sourcePath, tempPath, true); + try { - _diskProvider.MoveFile(tempPath, sourcePath); - } + ClearTargetPath(targetPath, overwrite); - _diskProvider.MoveFile(tempPath, targetPath); - return TransferMode.Move; + _diskProvider.MoveFile(tempPath, targetPath); + + return TransferMode.Move; + } + catch + { + RollbackMove(sourcePath, tempPath); + throw; + } } return TransferMode.None; @@ -117,10 +132,7 @@ namespace NzbDrone.Common.Disk throw new IOException(string.Format("Destination cannot be a child of the source [{0}] => [{1}]", sourcePath, targetPath)); } - if (_diskProvider.FileExists(targetPath) && overwrite) - { - _diskProvider.DeleteFile(targetPath); - } + ClearTargetPath(targetPath, overwrite); if (mode.HasFlag(TransferMode.HardLink)) { @@ -155,6 +167,52 @@ namespace NzbDrone.Common.Disk throw new IOException(String.Format("Failed to completely transfer [{0}] to [{1}], aborting.", sourcePath, targetPath)); } + else if (VerificationMode == DiskTransferVerificationMode.VerifyOnly) + { + var originalSize = _diskProvider.GetFileSize(sourcePath); + + if (mode.HasFlag(TransferMode.Copy)) + { + try + { + _diskProvider.CopyFile(sourcePath, targetPath); + + var targetSize = _diskProvider.GetFileSize(targetPath); + if (targetSize != originalSize) + { + throw new IOException(string.Format("File copy incomplete. [{0}] was {1} bytes long instead of {2} bytes.", targetPath, targetSize, originalSize)); + } + + return TransferMode.Copy; + } + catch + { + RollbackCopy(sourcePath, targetPath); + throw; + } + } + + if (mode.HasFlag(TransferMode.Move)) + { + try + { + _diskProvider.MoveFile(sourcePath, targetPath); + + var targetSize = _diskProvider.GetFileSize(targetPath); + if (targetSize != originalSize) + { + throw new IOException(string.Format("File copy incomplete, data loss may have occured. [{0}] was {1} bytes long instead of the expected {2}.", targetPath, targetSize, originalSize)); + } + + return TransferMode.Move; + } + catch + { + RollbackPartialMove(sourcePath, targetPath); + throw; + } + } + } else { if (mode.HasFlag(TransferMode.Copy)) @@ -173,39 +231,131 @@ namespace NzbDrone.Common.Disk return TransferMode.None; } + private void ClearTargetPath(String targetPath, bool overwrite) + { + if (_diskProvider.FileExists(targetPath)) + { + if (overwrite) + { + _diskProvider.DeleteFile(targetPath); + } + else + { + throw new IOException(string.Format("Destination already exists [{0}]", targetPath)); + } + } + } + + private void RollbackPartialMove(string sourcePath, string targetPath) + { + try + { + _logger.Debug("Rolling back incomplete file move [{0}] to [{1}].", sourcePath, targetPath); + + WaitForIO(); + + if (_diskProvider.FileExists(sourcePath)) + { + _diskProvider.DeleteFile(targetPath); + } + else + { + _logger.Error("Failed to properly rollback the file move [{0}] to [{1}], incomplete file may be left in target path.", sourcePath, targetPath); + } + } + catch (Exception ex) + { + _logger.ErrorException(string.Format("Failed to properly rollback the file move [{0}] to [{1}], incomplete file may be left in target path.", sourcePath, targetPath), ex); + } + } + + private void RollbackMove(string sourcePath, string targetPath) + { + try + { + _logger.Debug("Rolling back file move [{0}] to [{1}].", sourcePath, targetPath); + + WaitForIO(); + + _diskProvider.MoveFile(targetPath, sourcePath); + } + catch (Exception ex) + { + _logger.ErrorException(string.Format("Failed to properly rollback the file move [{0}] to [{1}], file may be left in target path.", sourcePath, targetPath), ex); + } + } + + private void RollbackCopy(string sourcePath, string targetPath) + { + try + { + _logger.Debug("Rolling back file copy [{0}] to [{1}].", sourcePath, targetPath); + + WaitForIO(); + + if (_diskProvider.FileExists(targetPath)) + { + _diskProvider.DeleteFile(targetPath); + } + } + catch (Exception ex) + { + _logger.ErrorException(string.Format("Failed to properly rollback the file copy [{0}] to [{1}], file may be left in target path.", sourcePath, targetPath), ex); + } + } + + private void WaitForIO() + { + // This delay is intended to give the IO stack a bit of time to recover, this is especially required if remote NAS devices are involved. + Thread.Sleep(3000); + } + private Boolean TryCopyFile(String sourcePath, String targetPath) { var originalSize = _diskProvider.GetFileSize(sourcePath); var tempTargetPath = targetPath + ".partial~"; - for (var i = 0; i <= RetryCount; i++) + try { - _diskProvider.CopyFile(sourcePath, tempTargetPath); + for (var i = 0; i <= RetryCount; i++) + { + _diskProvider.CopyFile(sourcePath, tempTargetPath); + + if (_diskProvider.FileExists(tempTargetPath)) + { + var targetSize = _diskProvider.GetFileSize(tempTargetPath); + if (targetSize == originalSize) + { + _diskProvider.MoveFile(tempTargetPath, targetPath); + return true; + } + } + + WaitForIO(); + + _diskProvider.DeleteFile(tempTargetPath); + + if (i == RetryCount) + { + _logger.Error("Failed to completely transfer [{0}] to [{1}], aborting.", sourcePath, targetPath, i + 1, RetryCount); + } + else + { + _logger.Warn("Failed to completely transfer [{0}] to [{1}], retrying [{2}/{3}].", sourcePath, targetPath, i + 1, RetryCount); + } + } + } + catch + { + WaitForIO(); if (_diskProvider.FileExists(tempTargetPath)) { - var targetSize = _diskProvider.GetFileSize(tempTargetPath); - - if (targetSize == originalSize) - { - _diskProvider.MoveFile(tempTargetPath, targetPath); - return true; - } + _diskProvider.DeleteFile(tempTargetPath); } - Thread.Sleep(5000); - - _diskProvider.DeleteFile(tempTargetPath); - - if (i == RetryCount) - { - _logger.Error("Failed to completely transfer [{0}] to [{1}], aborting.", sourcePath, targetPath, i + 1, RetryCount); - } - else - { - _logger.Warn("Failed to completely transfer [{0}] to [{1}], retrying [{2}/{3}].", sourcePath, targetPath, i + 1, RetryCount); - } + throw; } return false;