Invalidate the singleton DirectoryService cache on filesystem changes
This commit is contained in:
@@ -368,8 +368,8 @@ namespace Emby.Server.Implementations.IO
|
||||
return;
|
||||
}
|
||||
|
||||
// Invalidate before the checks below: a change we deliberately do not refresh for still
|
||||
// has to be read correctly the next time somebody looks at that folder.
|
||||
// The injected service is a singleton, so drop the path before the checks below:
|
||||
// a change we deliberately do not refresh for still has to read correctly later.
|
||||
_directoryService.Invalidate(path);
|
||||
|
||||
// Ignore certain files, If the parent of an ignored path has a change event, ignore that too
|
||||
|
||||
@@ -3723,8 +3723,7 @@ namespace Emby.Server.Implementations.Library
|
||||
}
|
||||
}
|
||||
|
||||
// The validation below would otherwise resolve the libraries root from a listing
|
||||
// taken before this folder was created.
|
||||
// The injected service is a singleton, so its listing predates this folder.
|
||||
_directoryService.Invalidate(virtualFolderPath);
|
||||
}
|
||||
finally
|
||||
|
||||
@@ -189,8 +189,7 @@ public class LibraryStructureController : BaseJellyfinApiController
|
||||
|
||||
Directory.Move(currentPath, newPath);
|
||||
|
||||
// The validation below would otherwise resolve the libraries root from a listing taken
|
||||
// before the folder was moved.
|
||||
// The injected service is a singleton, so its listings of both paths are now stale.
|
||||
_directoryService.Invalidate(currentPath);
|
||||
_directoryService.Invalidate(newPath);
|
||||
}
|
||||
|
||||
@@ -18,7 +18,6 @@
|
||||
</PropertyGroup>
|
||||
|
||||
<ItemGroup>
|
||||
<PackageReference Include="BitFaster.Caching" />
|
||||
<PackageReference Include="Microsoft.Extensions.Configuration.Binder" />
|
||||
</ItemGroup>
|
||||
|
||||
|
||||
@@ -1,31 +1,33 @@
|
||||
#pragma warning disable CS1591
|
||||
|
||||
using System;
|
||||
using System.Collections.Concurrent;
|
||||
using System.Collections.Generic;
|
||||
using System.IO;
|
||||
using System.Linq;
|
||||
using System.Runtime.CompilerServices;
|
||||
using BitFaster.Caching.Lru;
|
||||
using MediaBrowser.Model.IO;
|
||||
|
||||
namespace MediaBrowser.Controller.Providers
|
||||
{
|
||||
public class DirectoryService : IDirectoryService
|
||||
{
|
||||
private static readonly ConditionalWeakTable<IFileSystem, DirectoryCache> _caches = [];
|
||||
// TODO make static and switch to FastConcurrentLru.
|
||||
private readonly ConcurrentDictionary<string, FileSystemMetadata[]> _cache = new(StringComparer.Ordinal);
|
||||
|
||||
private readonly ConcurrentDictionary<string, FileSystemMetadata> _fileCache = new(StringComparer.Ordinal);
|
||||
|
||||
private readonly ConcurrentDictionary<string, List<string>> _filePathCache = new(StringComparer.Ordinal);
|
||||
|
||||
private readonly IFileSystem _fileSystem;
|
||||
private readonly DirectoryCache _cache;
|
||||
|
||||
public DirectoryService(IFileSystem fileSystem)
|
||||
{
|
||||
_fileSystem = fileSystem;
|
||||
_cache = _caches.GetValue(fileSystem, static _ => new DirectoryCache());
|
||||
}
|
||||
|
||||
public FileSystemMetadata[] GetFileSystemEntries(string path)
|
||||
{
|
||||
return _cache.Entries.GetOrAdd(
|
||||
return _cache.GetOrAdd(
|
||||
path,
|
||||
static (p, fileSystem) =>
|
||||
{
|
||||
@@ -87,15 +89,13 @@ namespace MediaBrowser.Controller.Providers
|
||||
|
||||
public FileSystemMetadata? GetFileSystemEntry(string path)
|
||||
{
|
||||
if (!_cache.Files.TryGet(path, out var result))
|
||||
if (!_fileCache.TryGetValue(path, out var result))
|
||||
{
|
||||
var file = _fileSystem.GetFileSystemInfo(path);
|
||||
|
||||
// Only cache hits: a missing file can turn up later.
|
||||
if (file?.Exists ?? false)
|
||||
{
|
||||
result = file;
|
||||
_cache.Files.AddOrUpdate(path, result);
|
||||
_fileCache.TryAdd(path, result);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -109,11 +109,10 @@ namespace MediaBrowser.Controller.Providers
|
||||
{
|
||||
if (clearCache)
|
||||
{
|
||||
// Not Invalidate(), which would also drop the parent listing for no reason here.
|
||||
Forget(path);
|
||||
_filePathCache.TryRemove(path, out _);
|
||||
}
|
||||
|
||||
return _cache.FilePaths.GetOrAdd(
|
||||
var filePaths = _filePathCache.GetOrAdd(
|
||||
path,
|
||||
static (p, fileSystem) =>
|
||||
{
|
||||
@@ -127,6 +126,8 @@ namespace MediaBrowser.Controller.Providers
|
||||
}
|
||||
},
|
||||
_fileSystem);
|
||||
|
||||
return filePaths;
|
||||
}
|
||||
|
||||
public void Invalidate(string path)
|
||||
@@ -147,28 +148,9 @@ namespace MediaBrowser.Controller.Providers
|
||||
|
||||
private void Forget(string path)
|
||||
{
|
||||
_cache.Entries.TryRemove(path, out _);
|
||||
_cache.Files.TryRemove(path, out _);
|
||||
_cache.FilePaths.TryRemove(path, out _);
|
||||
}
|
||||
|
||||
private sealed class DirectoryCache
|
||||
{
|
||||
private const int DirectoryCacheSize = 2048;
|
||||
private const int FileCacheSize = 8192;
|
||||
|
||||
// The cache outlives the DirectoryService instances reading it, so entries need their
|
||||
// own staleness bound. A long refresh can outlive it and re-read a directory partway.
|
||||
private static readonly TimeSpan _entryLifetime = TimeSpan.FromMinutes(1);
|
||||
|
||||
public ConcurrentTLru<string, FileSystemMetadata[]> Entries { get; }
|
||||
= new(Environment.ProcessorCount, DirectoryCacheSize, StringComparer.Ordinal, _entryLifetime);
|
||||
|
||||
public ConcurrentTLru<string, FileSystemMetadata> Files { get; }
|
||||
= new(Environment.ProcessorCount, FileCacheSize, StringComparer.Ordinal, _entryLifetime);
|
||||
|
||||
public ConcurrentTLru<string, List<string>> FilePaths { get; }
|
||||
= new(Environment.ProcessorCount, DirectoryCacheSize, StringComparer.Ordinal, _entryLifetime);
|
||||
_cache.TryRemove(path, out _);
|
||||
_fileCache.TryRemove(path, out _);
|
||||
_filePathCache.TryRemove(path, out _);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -255,7 +255,7 @@ public class LyricManager : ILyricManager
|
||||
_libraryMonitor.ReportFileSystemChangeComplete(path, false);
|
||||
}
|
||||
|
||||
// The refresh below would otherwise find the deleted file in a cached listing.
|
||||
// The injected service is a singleton, so its listing would keep the deleted file.
|
||||
_directoryService.Invalidate(path);
|
||||
}
|
||||
|
||||
@@ -453,7 +453,7 @@ public class LyricManager : ILyricManager
|
||||
await stream.CopyToAsync(fs).ConfigureAwait(false);
|
||||
}
|
||||
|
||||
// The refresh that follows would otherwise not see the new file.
|
||||
// The injected service is a singleton, so its listing of the folder is now stale.
|
||||
_directoryService.Invalidate(savePath);
|
||||
|
||||
return;
|
||||
|
||||
@@ -284,7 +284,7 @@ namespace MediaBrowser.Providers.Subtitles
|
||||
await stream.CopyToAsync(fs).ConfigureAwait(false);
|
||||
}
|
||||
|
||||
// The refresh that follows would otherwise not see the new file.
|
||||
// The injected service is a singleton, so its listing of the folder is now stale.
|
||||
_directoryService.Invalidate(path);
|
||||
|
||||
return;
|
||||
@@ -401,7 +401,7 @@ namespace MediaBrowser.Providers.Subtitles
|
||||
_monitor.ReportFileSystemChangeComplete(path, false);
|
||||
}
|
||||
|
||||
// The refresh below would otherwise find the deleted file in a cached listing.
|
||||
// The injected service is a singleton, so its listing would keep the deleted file.
|
||||
_directoryService.Invalidate(path);
|
||||
|
||||
return item.RefreshMetadata(CancellationToken.None);
|
||||
|
||||
@@ -1,4 +1,3 @@
|
||||
using System.Globalization;
|
||||
using System.IO;
|
||||
using System.Linq;
|
||||
using MediaBrowser.Controller.Providers;
|
||||
@@ -268,62 +267,6 @@ namespace Jellyfin.Controller.Tests
|
||||
fileSystemMock.Verify(f => f.GetFileSystemEntries(_lowerCasePath), Times.Once);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetFileSystemEntries_FarMorePathsThanTheCacheHolds_EvictsInsteadOfGrowing()
|
||||
{
|
||||
// Eviction of the first path shows up as the file system being read for it twice.
|
||||
const int PathCount = 8192;
|
||||
|
||||
var fileSystemMock = new Mock<IFileSystem>();
|
||||
fileSystemMock.Setup(f => f.GetFileSystemEntries(It.IsAny<string>()))
|
||||
.Returns(_lowerCaseFileSystemMetadata);
|
||||
|
||||
var directoryService = new DirectoryService(fileSystemMock.Object);
|
||||
|
||||
const string FirstPath = "/music/artist0";
|
||||
directoryService.GetFileSystemEntries(FirstPath);
|
||||
|
||||
for (var i = 1; i < PathCount; i++)
|
||||
{
|
||||
directoryService.GetFileSystemEntries("/music/artist" + i.ToString(CultureInfo.InvariantCulture));
|
||||
}
|
||||
|
||||
directoryService.GetFileSystemEntries(FirstPath);
|
||||
|
||||
fileSystemMock.Verify(f => f.GetFileSystemEntries(FirstPath), Times.Exactly(2));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetFileSystemEntries_SecondServiceOverSameFileSystem_ReusesTheFirstAnswer()
|
||||
{
|
||||
var fileSystemMock = new Mock<IFileSystem>();
|
||||
fileSystemMock.Setup(f => f.GetFileSystemEntries(_lowerCasePath))
|
||||
.Returns(_lowerCaseFileSystemMetadata);
|
||||
|
||||
new DirectoryService(fileSystemMock.Object).GetFileSystemEntries(_lowerCasePath);
|
||||
var result = new DirectoryService(fileSystemMock.Object).GetFileSystemEntries(_lowerCasePath);
|
||||
|
||||
Assert.Equal(_lowerCaseFileSystemMetadata, result);
|
||||
fileSystemMock.Verify(f => f.GetFileSystemEntries(_lowerCasePath), Times.Once);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetFileSystemEntries_SeparateFileSystems_DoNotShareAnswers()
|
||||
{
|
||||
var firstFileSystem = new Mock<IFileSystem>();
|
||||
firstFileSystem.Setup(f => f.GetFileSystemEntries(_lowerCasePath))
|
||||
.Returns(_lowerCaseFileSystemMetadata);
|
||||
var secondFileSystem = new Mock<IFileSystem>();
|
||||
secondFileSystem.Setup(f => f.GetFileSystemEntries(_lowerCasePath))
|
||||
.Returns(_upperCaseFileSystemMetadata);
|
||||
|
||||
var firstResult = new DirectoryService(firstFileSystem.Object).GetFileSystemEntries(_lowerCasePath);
|
||||
var secondResult = new DirectoryService(secondFileSystem.Object).GetFileSystemEntries(_lowerCasePath);
|
||||
|
||||
Assert.Equal(_lowerCaseFileSystemMetadata, firstResult);
|
||||
Assert.Equal(_upperCaseFileSystemMetadata, secondResult);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Invalidate_GivenADirectory_DropsBothTheListingAndTheFilePaths()
|
||||
{
|
||||
@@ -363,22 +306,6 @@ namespace Jellyfin.Controller.Tests
|
||||
Assert.Equal(_upperCaseFileSystemMetadata, directoryService.GetFileSystemEntries(_lowerCasePath));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Invalidate_OnOneService_IsSeenByAnotherOverTheSameFileSystem()
|
||||
{
|
||||
var fileSystemMock = new Mock<IFileSystem>();
|
||||
fileSystemMock.SetupSequence(f => f.GetFileSystemEntries(_lowerCasePath))
|
||||
.Returns(_lowerCaseFileSystemMetadata)
|
||||
.Returns(_upperCaseFileSystemMetadata);
|
||||
|
||||
new DirectoryService(fileSystemMock.Object).GetFileSystemEntries(_lowerCasePath);
|
||||
new DirectoryService(fileSystemMock.Object).Invalidate(Path.Combine(_lowerCasePath, "Song 2.srt"));
|
||||
|
||||
var result = new DirectoryService(fileSystemMock.Object).GetFileSystemEntries(_lowerCasePath);
|
||||
|
||||
Assert.Equal(_upperCaseFileSystemMetadata, result);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetFilePaths_ClearingTheCache_KeepsTheParentDirectory()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user