From 070e2b2d0c566b49a6f6361f0387022979a7d12b Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Sun, 26 Apr 2026 22:30:20 +0200 Subject: [PATCH] Apply review suggestions --- .../Item/BaseItemRepository.Querying.cs | 56 ++++++------------- .../Item/LinkedChildrenService.cs | 2 +- .../FixIncorrectOwnerIdRelationships.cs | 27 +++++++++ .../Routines/MigrateLinkedChildren.cs | 7 +++ 4 files changed, 53 insertions(+), 39 deletions(-) diff --git a/Jellyfin.Server.Implementations/Item/BaseItemRepository.Querying.cs b/Jellyfin.Server.Implementations/Item/BaseItemRepository.Querying.cs index 69c8a3b811..672a69e86e 100644 --- a/Jellyfin.Server.Implementations/Item/BaseItemRepository.Querying.cs +++ b/Jellyfin.Server.Implementations/Item/BaseItemRepository.Querying.cs @@ -87,7 +87,7 @@ public sealed partial class BaseItemRepository return Array.Empty(); } - var itemsById = ApplyNavigations(context.BaseItems.AsNoTracking().Where(e => orderedIds.Contains(e.Id)), filter) + var itemsById = ApplyNavigations(context.BaseItems.AsNoTracking().WhereOneOrMany(orderedIds, e => e.Id), filter) .AsSplitQuery() .AsEnumerable() .Select(w => DeserializeBaseItem(w, filter.SkipDeserialization)) @@ -142,48 +142,27 @@ public sealed partial class BaseItemRepository groupKeySelector = e => e.Album; } - var topGroupKeys = baseQuery + // Group by GroupKey, pick the latest item per group (correlated subquery: ORDER BY DateCreated DESC, Id DESC LIMIT 1), + // order groups by group max date, take the top N — all in a single SQL statement. + // ThenByDescending(Id) is the tiebreaker for deterministic ordering when items share a DateCreated. + var topGroupItems = baseQuery .Where(groupKeyFilter) .GroupBy(groupKeySelector) - .Select(g => new { GroupKey = g.Key!, MaxDate = g.Max(e => e.DateCreated) }) + .Select(g => new + { + MaxDate = g.Max(e => e.DateCreated), + FirstId = g.OrderByDescending(e => e.DateCreated).ThenByDescending(e => e.Id).Select(e => e.Id).First() + }) .OrderByDescending(g => g.MaxDate); - if (filter.Limit.HasValue) - { - topGroupKeys = topGroupKeys.Take(filter.Limit.Value).OrderByDescending(g => g.MaxDate); - } + var firstIdsQuery = filter.Limit.HasValue + ? topGroupItems.Take(filter.Limit.Value).Select(g => g.FirstId) + : topGroupItems.Select(g => g.FirstId); - // Get only the first (most recent) item ID per group using a lightweight projection, - // then fetch full entities only for those items. This avoids loading all versions/tracks - // with expensive navigation properties just to discard duplicates. - var topGroupKeyList = topGroupKeys.Select(g => g.GroupKey).ToList(); - // ThenByDescending(Id) is a tiebreaker for deterministic ordering when multiple items - // share the same DateCreated timestamp — without it, SQL returns arbitrary order across queries. - var allItemsLite = collectionType switch - { - CollectionType.movies => baseQuery - .Where(e => e.PresentationUniqueKey != null && topGroupKeyList.Contains(e.PresentationUniqueKey)) - .OrderByDescending(e => e.DateCreated) - .ThenByDescending(e => e.Id) - .Select(e => new { e.Id, GroupKey = e.PresentationUniqueKey }) - .AsEnumerable(), - _ => baseQuery - .Where(e => e.Album != null && topGroupKeyList.Contains(e.Album)) - .OrderByDescending(e => e.DateCreated) - .ThenByDescending(e => e.Id) - .Select(e => new { e.Id, GroupKey = e.Album }) - .AsEnumerable() - }; + var firstIds = firstIdsQuery.ToList(); - // Client-side DistinctBy: EF Core/SQLite cannot reliably translate - // GroupBy(...).Select(g => g.First()) to SQL. The projection is lightweight - // (only Id + GroupKey for ~50 items), so client-side dedup is negligible. - var firstIds = allItemsLite - .DistinctBy(e => e.GroupKey) - .Select(e => e.Id) - .ToList(); - - var itemsQuery = context.BaseItems.AsNoTracking().Where(e => firstIds.Contains(e.Id)); + // Single bound JSON / array parameter via WhereOneOrMany — keeps SQL small regardless of N. + var itemsQuery = context.BaseItems.AsNoTracking().WhereOneOrMany(firstIds, e => e.Id); itemsQuery = ApplyNavigations(itemsQuery, filter); return itemsQuery @@ -250,13 +229,14 @@ public sealed partial class BaseItemRepository ? topSeriesData.Min(g => g.MaxDate)?.AddHours(-RecentAdditionWindowHours) : null; - // Fetch only the columns needed for analysis (lightweight projection). + // Restrict to episodes of the top series, optionally bounded by the global cutoff. var episodeQuery = baseQuery.Where(e => e.SeriesName != null && topSeriesNames.Contains(e.SeriesName)); if (globalCutoff is not null) { episodeQuery = episodeQuery.Where(e => e.DateCreated >= globalCutoff); } + // Lightweight projection: only the columns needed for the in-memory analysis below. var allEpisodes = episodeQuery .OrderByDescending(e => e.DateCreated) .ThenByDescending(e => e.Id) diff --git a/Jellyfin.Server.Implementations/Item/LinkedChildrenService.cs b/Jellyfin.Server.Implementations/Item/LinkedChildrenService.cs index 4d27cae218..415510b2f4 100644 --- a/Jellyfin.Server.Implementations/Item/LinkedChildrenService.cs +++ b/Jellyfin.Server.Implementations/Item/LinkedChildrenService.cs @@ -54,7 +54,7 @@ public class LinkedChildrenService : ILinkedChildrenService return query .OrderBy(lc => lc.SortOrder) .Select(lc => lc.ChildId) - .ToList(); + .ToArray(); } /// diff --git a/Jellyfin.Server/Migrations/Routines/FixIncorrectOwnerIdRelationships.cs b/Jellyfin.Server/Migrations/Routines/FixIncorrectOwnerIdRelationships.cs index c743590e43..0baf261a2e 100644 --- a/Jellyfin.Server/Migrations/Routines/FixIncorrectOwnerIdRelationships.cs +++ b/Jellyfin.Server/Migrations/Routines/FixIncorrectOwnerIdRelationships.cs @@ -87,10 +87,19 @@ public class FixIncorrectOwnerIdRelationships : IAsyncMigrationRoutine // Collect all duplicate IDs to delete in one batch var allIdsToDelete = new List(); + const int progressLogStep = 500; + var processedPaths = 0; foreach (var path in duplicatePaths) { cancellationToken.ThrowIfCancellationRequested(); + if (processedPaths > 0 && processedPaths % progressLogStep == 0) + { + _logger.LogInformation("Resolving duplicates: {Processed}/{Total} paths", processedPaths, duplicatePaths.Count); + } + + processedPaths++; + // Get all items with this path var itemsWithPath = await context.BaseItems .Where(b => b.Path == path) @@ -208,6 +217,7 @@ public class FixIncorrectOwnerIdRelationships : IAsyncMigrationRoutine } _logger.LogInformation("Found {Count} orphaned extras to reassign", orphanedExtras.Count); + const int extraProgressLogStep = 500; // Build a lookup of directory -> first video/movie item for parent resolution var extraDirectories = orphanedExtras @@ -244,8 +254,16 @@ public class FixIncorrectOwnerIdRelationships : IAsyncMigrationRoutine } var reassignedCount = 0; + var processedExtras = 0; foreach (var extra in orphanedExtras) { + if (processedExtras > 0 && processedExtras % extraProgressLogStep == 0) + { + _logger.LogInformation("Reassigning orphaned extras: {Processed}/{Total}", processedExtras, orphanedExtras.Count); + } + + processedExtras++; + if (string.IsNullOrEmpty(extra.Path)) { continue; @@ -299,8 +317,17 @@ public class FixIncorrectOwnerIdRelationships : IAsyncMigrationRoutine .ConfigureAwait(false); var updatedCount = 0; + const int linkProgressLogStep = 1000; + var processedLinks = 0; foreach (var link in alternateVersionLinks) { + if (processedLinks > 0 && processedLinks % linkProgressLogStep == 0) + { + _logger.LogInformation("Populating PrimaryVersionId: {Processed}/{Total} links", processedLinks, alternateVersionLinks.Count); + } + + processedLinks++; + if (childItems.TryGetValue(link.ChildId, out var childItem)) { childItem.PrimaryVersionId = link.ParentId; diff --git a/Jellyfin.Server/Migrations/Routines/MigrateLinkedChildren.cs b/Jellyfin.Server/Migrations/Routines/MigrateLinkedChildren.cs index 129d443ca5..14ae535531 100644 --- a/Jellyfin.Server/Migrations/Routines/MigrateLinkedChildren.cs +++ b/Jellyfin.Server/Migrations/Routines/MigrateLinkedChildren.cs @@ -75,6 +75,8 @@ internal class MigrateLinkedChildren : IDatabaseMigrationRoutine var linkedChildrenToAdd = new List(); var processedCount = 0; + const int progressLogStep = 1000; + var totalItems = itemsWithData.Count; foreach (var item in itemsWithData) { @@ -83,6 +85,11 @@ internal class MigrateLinkedChildren : IDatabaseMigrationRoutine continue; } + if (processedCount > 0 && processedCount % progressLogStep == 0) + { + _logger.LogInformation("Processing LinkedChildren: {Processed}/{Total} items", processedCount, totalItems); + } + try { using var doc = JsonDocument.Parse(item.Data);