From 9c096aae10d4cdff8978198769f9d847d0623f9f Mon Sep 17 00:00:00 2001 From: Qstick Date: Sun, 26 Jul 2020 21:23:10 +0100 Subject: [PATCH] Fixed: Performance issue when scanning large root folder --- src/NzbDrone.Common/Disk/DiskProviderBase.cs | 7 + src/NzbDrone.Common/Disk/IDiskProvider.cs | 1 + .../DiskScanServiceTests/ScanFixture.cs | 158 +++++++++--------- .../MediaFiles/DiskScanService.cs | 35 ++-- 4 files changed, 106 insertions(+), 95 deletions(-) diff --git a/src/NzbDrone.Common/Disk/DiskProviderBase.cs b/src/NzbDrone.Common/Disk/DiskProviderBase.cs index 46f277723..7656a6ff3 100644 --- a/src/NzbDrone.Common/Disk/DiskProviderBase.cs +++ b/src/NzbDrone.Common/Disk/DiskProviderBase.cs @@ -149,6 +149,13 @@ public bool FolderWritable(string path) } } + public bool FolderEmpty(string path) + { + Ensure.That(path, () => path).IsValidPath(); + + return _fileSystem.Directory.EnumerateFileSystemEntries(path).Empty(); + } + public string[] GetDirectories(string path) { Ensure.That(path, () => path).IsValidPath(); diff --git a/src/NzbDrone.Common/Disk/IDiskProvider.cs b/src/NzbDrone.Common/Disk/IDiskProvider.cs index e3bbc23f7..2fe2f6359 100644 --- a/src/NzbDrone.Common/Disk/IDiskProvider.cs +++ b/src/NzbDrone.Common/Disk/IDiskProvider.cs @@ -22,6 +22,7 @@ public interface IDiskProvider bool FileExists(string path); bool FileExists(string path, StringComparison stringComparison); bool FolderWritable(string path); + bool FolderEmpty(string path); string[] GetDirectories(string path); string[] GetFiles(string path, SearchOption searchOption); long GetFolderSize(string path); diff --git a/src/NzbDrone.Core.Test/MediaFiles/DiskScanServiceTests/ScanFixture.cs b/src/NzbDrone.Core.Test/MediaFiles/DiskScanServiceTests/ScanFixture.cs index 1aee7c976..1c9d10f8a 100644 --- a/src/NzbDrone.Core.Test/MediaFiles/DiskScanServiceTests/ScanFixture.cs +++ b/src/NzbDrone.Core.Test/MediaFiles/DiskScanServiceTests/ScanFixture.cs @@ -24,18 +24,18 @@ namespace NzbDrone.Core.Test.MediaFiles.DiskScanServiceTests [TestFixture] public class ScanFixture : FileSystemTest { - private Author _artist; + private Author _author; private string _rootFolder; - private string _otherArtistFolder; + private string _otherAuthorFolder; [SetUp] public void Setup() { _rootFolder = @"C:\Test\Music".AsOsAgnostic(); - _otherArtistFolder = @"C:\Test\Music\OtherArtist".AsOsAgnostic(); + _otherAuthorFolder = @"C:\Test\Music\OtherArtist".AsOsAgnostic(); var artistFolder = @"C:\Test\Music\Artist".AsOsAgnostic(); - _artist = Builder.CreateNew() + _author = Builder.CreateNew() .With(s => s.Path = artistFolder) .Build(); @@ -76,7 +76,7 @@ private void GivenRootFolder(params string[] subfolders) private void GivenArtistFolder() { - GivenRootFolder(_artist.Path); + GivenRootFolder(_author.Path); } private List GivenFiles(IEnumerable files, DateTimeOffset? lastWrite = null) @@ -104,7 +104,7 @@ private void GivenKnownFiles(IEnumerable files, DateTimeOffset? lastWrit } Mocker.GetMock() - .Setup(x => x.GetFilesWithBasePath(_artist.Path)) + .Setup(x => x.GetFilesWithBasePath(_author.Path)) .Returns(files.Select(x => new BookFile { Path = x, @@ -115,12 +115,12 @@ private void GivenKnownFiles(IEnumerable files, DateTimeOffset? lastWrit [Test] public void should_not_scan_if_root_folder_does_not_exist() { - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); ExceptionVerification.ExpectedWarns(1); Mocker.GetMock() - .Verify(v => v.FolderExists(_artist.Path), Times.Never()); + .Verify(v => v.FolderExists(_author.Path), Times.Never()); Mocker.GetMock() .Verify(v => v.Clean(It.IsAny(), It.IsAny>()), Times.Never()); @@ -134,12 +134,12 @@ public void should_not_scan_if_root_folder_is_empty() { GivenRootFolder(); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); ExceptionVerification.ExpectedWarns(1); Mocker.GetMock() - .Verify(v => v.FolderExists(_artist.Path), Times.Never()); + .Verify(v => v.GetFiles(_author.Path, SearchOption.AllDirectories), Times.Never()); Mocker.GetMock() .Verify(v => v.Clean(It.IsAny(), It.IsAny>()), Times.Never()); @@ -151,11 +151,11 @@ public void should_not_scan_if_root_folder_is_empty() [Test] public void should_clean_if_folder_does_not_exist() { - GivenRootFolder(_otherArtistFolder); + GivenRootFolder(_otherAuthorFolder); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); - DiskProvider.FolderExists(_artist.Path).Should().BeFalse(); + DiskProvider.FolderExists(_author.Path).Should().BeFalse(); Mocker.GetMock() .Verify(v => v.Clean(It.IsAny(), It.IsAny>()), Times.Once()); @@ -168,11 +168,11 @@ public void should_find_files_at_root_of_artist_folder() GivenFiles(new List { - Path.Combine(_artist.Path, "file1.mobi"), - Path.Combine(_artist.Path, "s01e01.mobi") + Path.Combine(_author.Path, "file1.mobi"), + Path.Combine(_author.Path, "s01e01.mobi") }); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(v => v.GetImportDecisions(It.Is>(l => l.Count == 2), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); @@ -185,14 +185,14 @@ public void should_not_scan_extras_subfolder() GivenFiles(new List { - Path.Combine(_artist.Path, "EXTRAS", "file1.mobi"), - Path.Combine(_artist.Path, "Extras", "file2.mobi"), - Path.Combine(_artist.Path, "EXTRAs", "file3.mobi"), - Path.Combine(_artist.Path, "ExTrAs", "file4.mobi"), - Path.Combine(_artist.Path, "Season 1", "s01e01.mobi") + Path.Combine(_author.Path, "EXTRAS", "file1.mobi"), + Path.Combine(_author.Path, "Extras", "file2.mobi"), + Path.Combine(_author.Path, "EXTRAs", "file3.mobi"), + Path.Combine(_author.Path, "ExTrAs", "file4.mobi"), + Path.Combine(_author.Path, "Season 1", "s01e01.mobi") }); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(v => v.GetImportDecisions(It.Is>(l => l.Count == 1), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); @@ -205,12 +205,12 @@ public void should_not_scan_AppleDouble_subfolder() GivenFiles(new List { - Path.Combine(_artist.Path, ".AppleDouble", "file1.mobi"), - Path.Combine(_artist.Path, ".appledouble", "file2.mobi"), - Path.Combine(_artist.Path, "Season 1", "s01e01.mobi") + Path.Combine(_author.Path, ".AppleDouble", "file1.mobi"), + Path.Combine(_author.Path, ".appledouble", "file2.mobi"), + Path.Combine(_author.Path, "Season 1", "s01e01.mobi") }); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(v => v.GetImportDecisions(It.Is>(l => l.Count == 1), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); @@ -219,21 +219,21 @@ public void should_not_scan_AppleDouble_subfolder() [Test] public void should_scan_extras_artist_and_subfolders() { - _artist.Path = @"C:\Test\Music\Extras".AsOsAgnostic(); + _author.Path = @"C:\Test\Music\Extras".AsOsAgnostic(); GivenArtistFolder(); GivenFiles(new List { - Path.Combine(_artist.Path, "Extras", "file1.mobi"), - Path.Combine(_artist.Path, ".AppleDouble", "file2.mobi"), - Path.Combine(_artist.Path, "Season 1", "s01e01.mobi"), - Path.Combine(_artist.Path, "Season 1", "s01e02.mobi"), - Path.Combine(_artist.Path, "Season 2", "s02e01.mobi"), - Path.Combine(_artist.Path, "Season 2", "s02e02.mobi"), + Path.Combine(_author.Path, "Extras", "file1.mobi"), + Path.Combine(_author.Path, ".AppleDouble", "file2.mobi"), + Path.Combine(_author.Path, "Season 1", "s01e01.mobi"), + Path.Combine(_author.Path, "Season 1", "s01e02.mobi"), + Path.Combine(_author.Path, "Season 2", "s02e01.mobi"), + Path.Combine(_author.Path, "Season 2", "s02e02.mobi"), }); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(v => v.GetImportDecisions(It.Is>(l => l.Count == 4), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); @@ -246,10 +246,10 @@ public void should_scan_files_that_start_with_period() GivenFiles(new List { - Path.Combine(_artist.Path, "Album 1", ".t01.mobi") + Path.Combine(_author.Path, "Album 1", ".t01.mobi") }); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(v => v.GetImportDecisions(It.Is>(l => l.Count == 1), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); @@ -262,13 +262,13 @@ public void should_not_scan_subfolders_that_start_with_period() GivenFiles(new List { - Path.Combine(_artist.Path, ".@__thumb", "file1.mobi"), - Path.Combine(_artist.Path, ".@__THUMB", "file2.mobi"), - Path.Combine(_artist.Path, ".hidden", "file2.mobi"), - Path.Combine(_artist.Path, "Season 1", "s01e01.mobi") + Path.Combine(_author.Path, ".@__thumb", "file1.mobi"), + Path.Combine(_author.Path, ".@__THUMB", "file2.mobi"), + Path.Combine(_author.Path, ".hidden", "file2.mobi"), + Path.Combine(_author.Path, "Season 1", "s01e01.mobi") }); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(v => v.GetImportDecisions(It.Is>(l => l.Count == 1), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); @@ -281,14 +281,14 @@ public void should_not_scan_subfolder_of_season_folder_that_starts_with_a_period GivenFiles(new List { - Path.Combine(_artist.Path, "Season 1", ".@__thumb", "file1.mobi"), - Path.Combine(_artist.Path, "Season 1", ".@__THUMB", "file2.mobi"), - Path.Combine(_artist.Path, "Season 1", ".hidden", "file2.mobi"), - Path.Combine(_artist.Path, "Season 1", ".AppleDouble", "s01e01.mobi"), - Path.Combine(_artist.Path, "Season 1", "s01e01.mobi") + Path.Combine(_author.Path, "Season 1", ".@__thumb", "file1.mobi"), + Path.Combine(_author.Path, "Season 1", ".@__THUMB", "file2.mobi"), + Path.Combine(_author.Path, "Season 1", ".hidden", "file2.mobi"), + Path.Combine(_author.Path, "Season 1", ".AppleDouble", "s01e01.mobi"), + Path.Combine(_author.Path, "Season 1", "s01e01.mobi") }); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(v => v.GetImportDecisions(It.Is>(l => l.Count == 1), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); @@ -301,11 +301,11 @@ public void should_not_scan_Synology_eaDir() GivenFiles(new List { - Path.Combine(_artist.Path, "@eaDir", "file1.mobi"), - Path.Combine(_artist.Path, "Season 1", "s01e01.mobi") + Path.Combine(_author.Path, "@eaDir", "file1.mobi"), + Path.Combine(_author.Path, "Season 1", "s01e01.mobi") }); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(v => v.GetImportDecisions(It.Is>(l => l.Count == 1), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); @@ -318,11 +318,11 @@ public void should_not_scan_thumb_folder() GivenFiles(new List { - Path.Combine(_artist.Path, ".@__thumb", "file1.mobi"), - Path.Combine(_artist.Path, "Season 1", "s01e01.mobi") + Path.Combine(_author.Path, ".@__thumb", "file1.mobi"), + Path.Combine(_author.Path, "Season 1", "s01e01.mobi") }); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(v => v.GetImportDecisions(It.Is>(l => l.Count == 1), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); @@ -331,17 +331,17 @@ public void should_not_scan_thumb_folder() [Test] public void should_scan_dotHack_folder() { - _artist.Path = @"C:\Test\Music\.hack".AsOsAgnostic(); + _author.Path = @"C:\Test\Music\.hack".AsOsAgnostic(); GivenArtistFolder(); GivenFiles(new List { - Path.Combine(_artist.Path, "Season 1", "file1.mobi"), - Path.Combine(_artist.Path, "Season 1", "s01e01.mobi") + Path.Combine(_author.Path, "Season 1", "file1.mobi"), + Path.Combine(_author.Path, "Season 1", "s01e01.mobi") }); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(v => v.GetImportDecisions(It.Is>(l => l.Count == 2), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); @@ -354,12 +354,12 @@ public void should_exclude_osx_metadata_files() GivenFiles(new List { - Path.Combine(_artist.Path, ".DS_STORE"), - Path.Combine(_artist.Path, "._24 The Status Quo Combustion.mobi"), - Path.Combine(_artist.Path, "24 The Status Quo Combustion.mobi") + Path.Combine(_author.Path, ".DS_STORE"), + Path.Combine(_author.Path, "._24 The Status Quo Combustion.mobi"), + Path.Combine(_author.Path, "24 The Status Quo Combustion.mobi") }); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(v => v.GetImportDecisions(It.Is>(l => l.Count == 1), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); @@ -372,7 +372,7 @@ private void GivenRejections() .Returns((List fileList, IdentificationOverrides idOverrides, ImportDecisionMakerInfo idInfo, ImportDecisionMakerConfig idConfig) => fileList.Select(x => new LocalBook { - Author = _artist, + Author = _author, Path = x.FullName, Modified = x.LastWriteTimeUtc, FileTrackInfo = new ParsedTrackInfo() @@ -386,15 +386,15 @@ public void should_insert_new_unmatched_files_when_all_new() { var files = new List { - Path.Combine(_artist.Path, "Season 1", "file1.mobi"), - Path.Combine(_artist.Path, "Season 1", "s01e01.mobi") + Path.Combine(_author.Path, "Season 1", "file1.mobi"), + Path.Combine(_author.Path, "Season 1", "s01e01.mobi") }; GivenFiles(files); GivenKnownFiles(new List()); GivenRejections(); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(x => x.AddMany(It.Is>(l => l.Select(t => t.Path).SequenceEqual(files))), @@ -406,15 +406,15 @@ public void should_insert_new_unmatched_files_when_some_known() { var files = new List { - Path.Combine(_artist.Path, "Season 1", "file1.mobi"), - Path.Combine(_artist.Path, "Season 1", "s01e01.mobi") + Path.Combine(_author.Path, "Season 1", "file1.mobi"), + Path.Combine(_author.Path, "Season 1", "s01e01.mobi") }; GivenFiles(files); GivenKnownFiles(files.GetRange(1, 1)); GivenRejections(); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(x => x.AddMany(It.Is>(l => l.Select(t => t.Path).SequenceEqual(files.GetRange(0, 1)))), @@ -426,15 +426,15 @@ public void should_not_insert_files_when_all_known() { var files = new List { - Path.Combine(_artist.Path, "Season 1", "file1.mobi"), - Path.Combine(_artist.Path, "Season 1", "s01e01.mobi") + Path.Combine(_author.Path, "Season 1", "file1.mobi"), + Path.Combine(_author.Path, "Season 1", "s01e01.mobi") }; GivenFiles(files); GivenKnownFiles(files); GivenRejections(); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(x => x.AddMany(It.Is>(l => l.Count == 0)), @@ -450,15 +450,15 @@ public void should_not_update_info_for_unchanged_known_files() { var files = new List { - Path.Combine(_artist.Path, "Season 1", "file1.mobi"), - Path.Combine(_artist.Path, "Season 1", "s01e01.mobi") + Path.Combine(_author.Path, "Season 1", "file1.mobi"), + Path.Combine(_author.Path, "Season 1", "s01e01.mobi") }; GivenFiles(files); GivenKnownFiles(files); GivenRejections(); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(x => x.Update(It.Is>(l => l.Count == 0)), @@ -474,15 +474,15 @@ public void should_update_info_for_changed_known_files() { var files = new List { - Path.Combine(_artist.Path, "Season 1", "file1.mobi"), - Path.Combine(_artist.Path, "Season 1", "s01e01.mobi") + Path.Combine(_author.Path, "Season 1", "file1.mobi"), + Path.Combine(_author.Path, "Season 1", "s01e01.mobi") }; GivenFiles(files, new DateTime(2019, 2, 1)); GivenKnownFiles(files); GivenRejections(); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(x => x.Update(It.Is>(l => l.Count == 2)), @@ -494,7 +494,7 @@ public void should_update_fields_for_updated_files() { var files = new List { - Path.Combine(_artist.Path, "Season 1", "file1.mobi"), + Path.Combine(_author.Path, "Season 1", "file1.mobi"), }; GivenKnownFiles(files); @@ -516,7 +516,7 @@ public void should_update_fields_for_updated_files() .Setup(x => x.GetImportDecisions(It.IsAny>(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns(new List> { new ImportDecision(localTrack, new Rejection("Reject")) }); - Subject.Scan(new List { _artist.Path }); + Subject.Scan(new List { _author.Path }); Mocker.GetMock() .Verify(x => x.Update(It.Is>( diff --git a/src/NzbDrone.Core/MediaFiles/DiskScanService.cs b/src/NzbDrone.Core/MediaFiles/DiskScanService.cs index dd51bdb22..29ab0b780 100644 --- a/src/NzbDrone.Core/MediaFiles/DiskScanService.cs +++ b/src/NzbDrone.Core/MediaFiles/DiskScanService.cs @@ -102,25 +102,28 @@ public void Scan(List folders = null, FilterFilesType filter = FilterFil return; } - if (!_diskProvider.FolderExists(rootFolder.Path)) - { - _logger.Warn("Root folder {0} doesn't exist.", rootFolder.Path); + var folderExists = _diskProvider.FolderExists(folder); - var skippedArtists = _authorService.GetAuthors(authorIds); - skippedArtists.ForEach(x => _eventAggregator.PublishEvent(new AuthorScanSkippedEvent(x, AuthorScanSkippedReason.RootFolderDoesNotExist))); - return; + if (!folderExists) + { + if (!_diskProvider.FolderExists(rootFolder.Path)) + { + _logger.Warn("Authors' root folder ({0}) doesn't exist.", rootFolder); + var skippedAuthors = _authorService.GetAuthors(authorIds); + skippedAuthors.ForEach(x => _eventAggregator.PublishEvent(new AuthorScanSkippedEvent(x, AuthorScanSkippedReason.RootFolderDoesNotExist))); + return; + } + + if (_diskProvider.FolderEmpty(rootFolder.Path)) + { + _logger.Warn("Authors' root folder ({0}) is empty.", rootFolder); + var skippedAuthors = _authorService.GetAuthors(authorIds); + skippedAuthors.ForEach(x => _eventAggregator.PublishEvent(new AuthorScanSkippedEvent(x, AuthorScanSkippedReason.RootFolderIsEmpty))); + return; + } } - if (_diskProvider.GetDirectories(rootFolder.Path).Empty()) - { - _logger.Warn("Root folder {0} is empty.", rootFolder.Path); - - var skippedArtists = _authorService.GetAuthors(authorIds); - skippedArtists.ForEach(x => _eventAggregator.PublishEvent(new AuthorScanSkippedEvent(x, AuthorScanSkippedReason.RootFolderIsEmpty))); - return; - } - - if (!_diskProvider.FolderExists(folder)) + if (!folderExists) { _logger.Debug("Specified scan folder ({0}) doesn't exist.", folder);