Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 29 additions & 7 deletions listenarr.application/Downloads/Common/DownloadClientGateway.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,17 @@ public async Task<List<QueueItem>> GetQueueAsync(DownloadClientConfiguration cli
{
var adapter = ResolveAdapter(client);
var items = await adapter.GetQueueAsync(client, ct);
var tasks = items.Select(item => TranslateQueueItemPathsAsync(client, item));
// Resolved once for the batch. Translating each item used to query for the client's
// mappings itself, inside this fan-out, against a scoped repository shared by everything
// else in the scope. An empty queue is the common case on an idle client and needs no
// lookup at all, which is what it cost before the batch lookup was introduced.
IReadOnlyList<RemotePathMapping> mappings = [];
if (items.Count > 0)
{
mappings = await remotePathMappingService.GetPathMappingByClientAsync(client);
}

var tasks = items.Select(item => TranslateQueueItemPathsAsync(mappings, client, item));
return [.. await Task.WhenAll(tasks)];
}

Expand Down Expand Up @@ -137,7 +147,9 @@ public async Task<QueueItem> GetQueueItemAsync(
var adapter = ResolveAdapter(client);
var item = await adapter.GetImportItemAsync(client, download, queueItem, null, ct);

return await TranslateQueueItemPathsAsync(client, item);
// Single item, so the lookup here is one query either way.
var mappings = await remotePathMappingService.GetPathMappingByClientAsync(client);
return await TranslateQueueItemPathsAsync(mappings, client, item);
}

public async Task<List<Download>> FetchDownloadsAsync(DownloadClientConfiguration client, List<Download> downloads, CancellationToken ct = default)
Expand Down Expand Up @@ -165,7 +177,13 @@ public async Task<List<Download>> FetchDownloadsAsync(DownloadClientConfiguratio
ex);
}

var tasks = items.Select(item => TranslateQueueItemPathsAsync(client, item));
IReadOnlyList<RemotePathMapping> mappings = [];
if (items.Count > 0)
{
mappings = await remotePathMappingService.GetPathMappingByClientAsync(client);
}

var tasks = items.Select(item => TranslateQueueItemPathsAsync(mappings, client, item));
items = [.. await Task.WhenAll(tasks)];

foreach (QueueItem item in items)
Expand Down Expand Up @@ -228,20 +246,24 @@ private List<string> GetExternalIds(List<Download> downloads)
/// Make sure all paths are locally accessible after processing and
/// that a proper list of sanitized source files is produced
/// </summary>
/// <param name="mappings">Remote path mappings already resolved for this client</param>
/// <param name="client">Download client configuration to use for path mapping</param>
/// <param name="item">Queue item to translate/sanitize</param>
/// <returns></returns>
private async Task<QueueItem> TranslateQueueItemPathsAsync(DownloadClientConfiguration client, QueueItem item)
private async Task<QueueItem> TranslateQueueItemPathsAsync(
IReadOnlyList<RemotePathMapping> mappings,
DownloadClientConfiguration client,
QueueItem item)
{
if (!string.IsNullOrEmpty(item.RemotePath))
{
item.LocalPath = await remotePathMappingService.TranslatePathAsync(client, item.RemotePath);
item.LocalPath = remotePathMappingService.TranslatePath(mappings, client, item.RemotePath);
EnsureNativePath(item.LocalPath, client.Name);
}

if (!string.IsNullOrEmpty(item.ContentPath))
{
item.ContentPath = await remotePathMappingService.TranslatePathAsync(client, item.ContentPath);
item.ContentPath = remotePathMappingService.TranslatePath(mappings, client, item.ContentPath);
EnsureNativePath(item.ContentPath, client.Name);
}

Expand All @@ -256,7 +278,7 @@ private async Task<QueueItem> TranslateQueueItemPathsAsync(DownloadClientConfigu
List<string> sourceFiles = [];
foreach (string file in item.SourceFiles)
{
var sourceFile = await remotePathMappingService.TranslatePathAsync(client, file);
var sourceFile = remotePathMappingService.TranslatePath(mappings, client, file);
EnsureNativePath(sourceFile, client.Name);
sourceFiles.Add(sourceFile);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,25 @@ public interface IRemotePathMappingService
/// A matching mapping exists but its local side is unavailable or unsafe on this host.
/// </exception>
Task<string> TranslatePathAsync(DownloadClientConfiguration client, string remotePath);

/// <summary>
/// Translates a remote path using mappings the caller has already resolved.
/// </summary>
/// <remarks>
/// For callers translating many paths for one client. Resolving the mappings once and
/// translating from them keeps a parallel batch off the scoped repository, and so off the
/// scoped DbContext behind it, which permits one operation at a time.
/// </remarks>
/// <param name="mappings">Mappings already resolved for this client, most specific first</param>
/// <param name="client">The download client reporting the path</param>
/// <param name="remotePath">The path as reported by the download client</param>
/// <returns>The translated local path, or the original path if no mapping matches.</returns>
/// <exception cref="ArgumentNullException"><paramref name="mappings"/> is null.</exception>
/// <exception cref="InvalidOperationException">
/// A matching mapping exists but its local side is unavailable or unsafe on this host.
/// </exception>
string TranslatePath(
IReadOnlyList<RemotePathMapping> mappings,
DownloadClientConfiguration client,
string remotePath);
}
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,39 @@ public async Task<List<RemotePathMapping>> GetAllAsync()
return await remotePathMappingRepository.GetByIdAsync(id);
}

// A client's mappings change only when the user edits them, and every queue poll needs the
// whole set for that client. Serving them from the shared memory cache keeps the
// steady-state poll off the repository entirely, which matters because the repository and
// the DbContext behind it are scoped while several clients are polled concurrently from a
// single scope. Readarr does the same thing in
// src/NzbDrone.Core/RemotePathMappings/RemotePathMappingService.cs, where All() is served
// from a cache with a ten second lifetime that add, update and remove clear.
//
// The three writers below already removed this key. Only the population was missing, so
// nothing was ever in the cache and every caller went to the database.
private static readonly TimeSpan ClientCacheLifetime = TimeSpan.FromSeconds(10);

public async Task<List<RemotePathMapping>> GetPathMappingByClientAsync(DownloadClientConfiguration client)
{
return await remotePathMappingRepository.GetByClientIdAsync(client.Id);
var cacheKey = ClientCacheKey(client.Id);

if (cache.TryGetValue(cacheKey, out RemotePathMapping[]? cached) && cached is not null)
{
return [.. cached];
}

var mappings = await remotePathMappingRepository.GetByClientIdAsync(client.Id);

// Cache a private copy and hand every caller its own list. The repository reads these
// untracked, so sharing the instances across scopes is safe, but sharing the list
// itself would let one caller's edit reach the next one.
cache.Set(cacheKey, mappings.ToArray(), ClientCacheLifetime);

return mappings;
}

private static string ClientCacheKey(string downloadClientId) => $"rpm_client_{downloadClientId}";

public async Task<RemotePathMapping> CreateAsync(RemotePathMapping mapping)
{
mapping.NormalizePaths();
Expand All @@ -55,7 +83,7 @@ public async Task<RemotePathMapping> CreateAsync(RemotePathMapping mapping)

try
{
cache.Remove($"rpm_client_{saved.DownloadClientId}");
cache.Remove(ClientCacheKey(saved.DownloadClientId));
}
catch (Exception exception) when (exception is not (OperationCanceledException or OutOfMemoryException or StackOverflowException))
{
Expand Down Expand Up @@ -84,7 +112,7 @@ public async Task<RemotePathMapping> UpdateAsync(RemotePathMapping mapping)
"Updated remote path mapping {MappingId} for client {ClientId}: {RemotePath} -> {LocalPath}",
saved.Id, saved.DownloadClientId, saved.RemotePath, saved.LocalPath);

try { cache.Remove($"rpm_client_{saved.DownloadClientId}"); }
try { cache.Remove(ClientCacheKey(saved.DownloadClientId)); }
catch (Exception caughtEx_2) when (caughtEx_2 is not OperationCanceledException && caughtEx_2 is not OutOfMemoryException && caughtEx_2 is not StackOverflowException)
{
System.Diagnostics.Debug.WriteLine("Suppressed non-fatal exception in catch block.");
Expand All @@ -106,7 +134,7 @@ public async Task<bool> DeleteAsync(int id)
"Deleted remote path mapping {MappingId} for client {ClientId}",
id, existing.DownloadClientId);

try { cache.Remove($"rpm_client_{existing.DownloadClientId}"); }
try { cache.Remove(ClientCacheKey(existing.DownloadClientId)); }
catch (Exception caughtEx_3) when (caughtEx_3 is not OperationCanceledException && caughtEx_3 is not OutOfMemoryException && caughtEx_3 is not StackOverflowException)
{
System.Diagnostics.Debug.WriteLine("Suppressed non-fatal exception in catch block.");
Expand All @@ -123,7 +151,29 @@ public async Task<string> TranslatePathAsync(DownloadClientConfiguration client,
return remotePath;
}

var mappings = await GetPathMappingByClientAsync(client);
return TranslatePath(await GetPathMappingByClientAsync(client), client, remotePath);
}

// The mapping lookup and the translation are separated so a caller translating many paths
// for one client can resolve the mappings once. The repository is scoped and so is the
// DbContext behind it, so translating a batch in parallel while each call did its own
// lookup meant concurrent queries on a context that permits one at a time.
public string TranslatePath(
IReadOnlyList<RemotePathMapping> mappings,
DownloadClientConfiguration client,
string remotePath)
{
// TranslatePathAsync always had the mappings in hand. This overload takes them from a
// caller, so the one contract it adds is that they are actually there. Failing here
// says which argument was wrong; without it an empty-looking translation just returns
// the remote path and the caller never learns that nothing was consulted.
ArgumentNullException.ThrowIfNull(mappings);

if (string.IsNullOrEmpty(remotePath))
{
return remotePath;
}

foreach (var mapping in mappings)
{
if (!TryGetRemoteSemantics(
Expand Down
Loading