From f3aff5d417c50802dd7cb21c896e8ff04bc955e3 Mon Sep 17 00:00:00 2001 From: johoja12 <223961+johoja12@users.noreply.github.com> Date: Sun, 16 Aug 2026 21:00:18 +0000 Subject: [PATCH 01/12] Fix import list sync command serialization --- .../Messaging/Commands/CommandQueueFixture.cs | 22 +++++++++++-------- .../ImportLists/ImportListSyncCommand.cs | 2 ++ 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/NzbDrone.Core.Test/Messaging/Commands/CommandQueueFixture.cs b/src/NzbDrone.Core.Test/Messaging/Commands/CommandQueueFixture.cs index 5589eb41451..4bd1f8f0c28 100644 --- a/src/NzbDrone.Core.Test/Messaging/Commands/CommandQueueFixture.cs +++ b/src/NzbDrone.Core.Test/Messaging/Commands/CommandQueueFixture.cs @@ -29,8 +29,8 @@ private void GivenStartedTypeExclusiveCommand() { var commandModel = Builder .CreateNew() - .With(c => c.Name = "ImportListSync") - .With(c => c.Body = new ImportListSyncCommand()) + .With(c => c.Name = "TypeExclusive") + .With(c => c.Body = new TypeExclusiveCommand()) .With(c => c.Status = CommandStatus.Started) .Build(); @@ -49,6 +49,11 @@ private void GivenStartedExclusiveCommand() Subject.Add(commandModel); } + private class TypeExclusiveCommand : Command + { + public override bool IsTypeExclusive => true; + } + [Test] public void should_not_return_disk_access_command_if_another_running() { @@ -75,8 +80,8 @@ public void should_not_return_type_exclusive_command_if_another_and_disk_access_ var newCommandModel = Builder .CreateNew() - .With(c => c.Name = "ImportListSync") - .With(c => c.Body = new ImportListSyncCommand()) + .With(c => c.Name = "TypeExclusive") + .With(c => c.Body = new TypeExclusiveCommand()) .Build(); Subject.Add(newCommandModel); @@ -93,8 +98,8 @@ public void should_not_return_type_exclusive_command_if_another_running() var newCommandModel = Builder .CreateNew() - .With(c => c.Name = "ImportListSync") - .With(c => c.Body = new ImportListSyncCommand()) + .With(c => c.Name = "TypeExclusive") + .With(c => c.Body = new TypeExclusiveCommand()) .Build(); Subject.Add(newCommandModel); @@ -105,7 +110,7 @@ public void should_not_return_type_exclusive_command_if_another_running() } [Test] - public void should_return_type_exclusive_command_if_another_not_running() + public void should_not_return_import_list_sync_command_if_another_command_is_running() { GivenStartedDiskCommand(); @@ -119,8 +124,7 @@ public void should_return_type_exclusive_command_if_another_not_running() Subject.TryGet(out var command); - command.Should().NotBeNull(); - command.Status.Should().Be(CommandStatus.Started); + command.Should().BeNull(); } [Test] diff --git a/src/NzbDrone.Core/ImportLists/ImportListSyncCommand.cs b/src/NzbDrone.Core/ImportLists/ImportListSyncCommand.cs index 9051d205fba..ab0c294e3cd 100644 --- a/src/NzbDrone.Core/ImportLists/ImportListSyncCommand.cs +++ b/src/NzbDrone.Core/ImportLists/ImportListSyncCommand.cs @@ -17,6 +17,8 @@ public ImportListSyncCommand(int? definition) public override bool SendUpdatesToClient => true; + public override bool IsExclusive => true; + public override bool IsTypeExclusive => true; public override bool UpdateScheduledTask => !DefinitionId.HasValue; From 7df4bfafdcf096ccf034531b8b7cd39c8d6aad24 Mon Sep 17 00:00:00 2001 From: johoja12 <223961+johoja12@users.noreply.github.com> Date: Sun, 16 Aug 2026 21:38:42 +0000 Subject: [PATCH 02/12] fix: defer queue persistence during exclusive commands --- .../Commands/CommandQueueManagerFixture.cs | 64 +++++++++++++++++++ .../Messaging/Commands/CommandQueue.cs | 11 ++++ .../Messaging/Commands/CommandQueueManager.cs | 7 +- 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/NzbDrone.Core.Test/Messaging/Commands/CommandQueueManagerFixture.cs b/src/NzbDrone.Core.Test/Messaging/Commands/CommandQueueManagerFixture.cs index d0bf4a26ec6..87c7831f13b 100644 --- a/src/NzbDrone.Core.Test/Messaging/Commands/CommandQueueManagerFixture.cs +++ b/src/NzbDrone.Core.Test/Messaging/Commands/CommandQueueManagerFixture.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; +using System.Threading.Tasks; using FluentAssertions; using Moq; using NUnit.Framework; @@ -56,5 +58,67 @@ public void should_not_remove_commands_for_five_minutes_after_they_end() Mocker.GetMock() .Verify(v => v.Get(It.IsAny()), Times.Never()); } + + [Test] + public void should_wait_to_persist_a_command_until_an_exclusive_command_completes() + { + var exclusive = Subject.Push(new ExclusiveCommand()); + using var consumer = Subject.Queue(CancellationToken.None).GetEnumerator(); + consumer.MoveNext().Should().BeTrue(); + + using var started = new ManualResetEventSlim(); + var pendingPush = Task.Run(() => + { + started.Set(); + return Subject.Push(new RefreshMonitoredDownloadsCommand()); + }); + + try + { + started.Wait(TimeSpan.FromSeconds(1)).Should().BeTrue(); + pendingPush.Wait(TimeSpan.FromMilliseconds(100)).Should().BeFalse(); + } + finally + { + Subject.Complete(exclusive, "Done"); + } + + pendingPush.Wait(TimeSpan.FromSeconds(1)).Should().BeTrue(); + } + + [Test] + public void should_wait_to_persist_many_commands_until_an_exclusive_command_completes() + { + var exclusive = Subject.Push(new ExclusiveCommand()); + using var consumer = Subject.Queue(CancellationToken.None).GetEnumerator(); + consumer.MoveNext().Should().BeTrue(); + + using var started = new ManualResetEventSlim(); + var pendingPush = Task.Run(() => + { + started.Set(); + return Subject.PushMany(new List + { + new RefreshMonitoredDownloadsCommand() + }); + }); + + try + { + started.Wait(TimeSpan.FromSeconds(1)).Should().BeTrue(); + pendingPush.Wait(TimeSpan.FromMilliseconds(100)).Should().BeFalse(); + } + finally + { + Subject.Complete(exclusive, "Done"); + } + + pendingPush.Wait(TimeSpan.FromSeconds(1)).Should().BeTrue(); + } + + private class ExclusiveCommand : Command + { + public override bool IsExclusive => true; + } } } diff --git a/src/NzbDrone.Core/Messaging/Commands/CommandQueue.cs b/src/NzbDrone.Core/Messaging/Commands/CommandQueue.cs index 9096b418c7e..9de951a5ddb 100644 --- a/src/NzbDrone.Core/Messaging/Commands/CommandQueue.cs +++ b/src/NzbDrone.Core/Messaging/Commands/CommandQueue.cs @@ -146,6 +146,17 @@ public void PulseAllConsumers() } } + public void WaitForExclusiveCommandToComplete() + { + lock (_mutex) + { + while (_items.Any(c => c.Status == CommandStatus.Started && c.Body.IsExclusive)) + { + Monitor.Wait(_mutex); + } + } + } + public bool TryGet(out CommandModel item) { var rval = true; diff --git a/src/NzbDrone.Core/Messaging/Commands/CommandQueueManager.cs b/src/NzbDrone.Core/Messaging/Commands/CommandQueueManager.cs index 6f7f6d705e5..d848ab92fcf 100644 --- a/src/NzbDrone.Core/Messaging/Commands/CommandQueueManager.cs +++ b/src/NzbDrone.Core/Messaging/Commands/CommandQueueManager.cs @@ -87,7 +87,11 @@ public List PushMany(List commands) commandModels.Add(commandModel); } - _repo.InsertMany(commandModels); + if (commandModels.Any()) + { + _commandQueue.WaitForExclusiveCommandToComplete(); + _repo.InsertMany(commandModels); + } foreach (var commandModel in commandModels) { @@ -132,6 +136,7 @@ public CommandModel Push(TCommand command, CommandPriority priority = _logger.Trace("Inserting new command: {0}", commandModel.Name); + _commandQueue.WaitForExclusiveCommandToComplete(); _repo.Insert(commandModel); _commandQueue.Add(commandModel); From 442c8b3c8eb33499b4bbc394fa647536b3c4dc42 Mon Sep 17 00:00:00 2001 From: johoja12 <223961+johoja12@users.noreply.github.com> Date: Mon, 17 Aug 2026 10:48:23 +0000 Subject: [PATCH 03/12] fix: serialize import list workers --- ...-08-17-import-list-worker-serialization.md | 97 +++++++++++++++++++ ...import-list-worker-serialization-design.md | 27 ++++++ .../FetchAndParseImportListServiceFixture.cs | 41 ++++++++ .../FetchAndParseImportListService.cs | 55 +++++------ 4 files changed, 187 insertions(+), 33 deletions(-) create mode 100644 docs/superpowers/plans/2026-08-17-import-list-worker-serialization.md create mode 100644 docs/superpowers/specs/2026-08-17-import-list-worker-serialization-design.md diff --git a/docs/superpowers/plans/2026-08-17-import-list-worker-serialization.md b/docs/superpowers/plans/2026-08-17-import-list-worker-serialization.md new file mode 100644 index 00000000000..1ea08d89af2 --- /dev/null +++ b/docs/superpowers/plans/2026-08-17-import-list-worker-serialization.md @@ -0,0 +1,97 @@ +# Import-list Worker Serialization Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Ensure one `ImportListSync` command cannot overlap enabled import-list pipelines. + +**Architecture:** `FetchAndParseImportListService.Fetch()` will execute each accepted list directly in the existing loop rather than submitting all lists to a long-running `TaskFactory`. Existing result aggregation, failure handling, TMDb mapping, list-movie synchronization, and status updates remain unchanged. + +**Tech Stack:** C#, .NET 8, NUnit, FluentAssertions, Moq. + +--- + +### Task 1: Prove that a later list does not start while the first list is blocked + +**Files:** +- Modify: `src/NzbDrone.Core.Test/ImportListTests/FetchAndParseImportListServiceFixture.cs` +- Test: `src/NzbDrone.Core.Test/ImportListTests/FetchAndParseImportListServiceFixture.cs` + +- [ ] **Step 1: Write the failing regression test** + +```csharp +[Test] +public void should_not_start_the_next_list_until_the_current_list_finishes() +{ + using var firstListStarted = new ManualResetEventSlim(); + using var releaseFirstList = new ManualResetEventSlim(); + var first = CreateListResult(1, true, true, new ImportListFetchResult()); + var second = CreateListResult(2, true, true, new ImportListFetchResult()); + + first.Setup(x => x.Fetch()).Returns(() => + { + firstListStarted.Set(); + releaseFirstList.Wait(); + return new ImportListFetchResult(); + }); + + var fetch = Task.Run(() => Subject.Fetch()); + firstListStarted.Wait(TimeSpan.FromSeconds(1)).Should().BeTrue(); + second.Verify(x => x.Fetch(), Times.Never()); + + releaseFirstList.Set(); + fetch.Wait(TimeSpan.FromSeconds(1)).Should().BeTrue(); + second.Verify(x => x.Fetch(), Times.Once()); +} +``` + +- [ ] **Step 2: Verify the test is red** + +Run: + +```bash +dotnet test src/NzbDrone.Core.Test/Radarr.Core.Test.csproj /p:RunAnalyzers=false --filter 'FullyQualifiedName~FetchAndParseImportListServiceFixture.should_not_start_the_next_list_until_the_current_list_finishes' +``` + +Expected: FAIL because the current `TaskFactory.StartNew` immediately invokes the second list's `Fetch()`. + +### Task 2: Make all-list processing sequential + +**Files:** +- Modify: `src/NzbDrone.Core/ImportLists/FetchAndParseImportListService.cs` +- Test: `src/NzbDrone.Core.Test/ImportListTests/FetchAndParseImportListServiceFixture.cs` + +- [ ] **Step 1: Replace worker creation with direct execution** + +Remove `System.Threading.Tasks`, the task list, and `TaskFactory`. Execute the existing try/catch and `lock (result)` body directly after the blocked-list checks, then remove `Task.WaitAll(...)`. + +- [ ] **Step 2: Verify the focused fixture is green** + +Run: + +```bash +dotnet test src/NzbDrone.Core.Test/Radarr.Core.Test.csproj /p:RunAnalyzers=false --filter 'FullyQualifiedName~FetchAndParseImportListServiceFixture' +``` + +Expected: all fixture tests pass, including the new gating regression. + +### Task 3: Verify, publish, and stage production + +**Files:** +- Modify: `docs/superpowers/specs/2026-08-17-import-list-worker-serialization-design.md` +- Modify: `docs/superpowers/plans/2026-08-17-import-list-worker-serialization.md` + +- [ ] **Step 1: Run the focused import-list and queue persistence suites** + +```bash +dotnet test src/NzbDrone.Core.Test/Radarr.Core.Test.csproj /p:RunAnalyzers=false --filter 'FullyQualifiedName~FetchAndParseImportListServiceFixture|FullyQualifiedName~CommandQueueManagerFixture|FullyQualifiedName~CommandQueueFixture' +``` + +Expected: all selected tests pass. + +- [ ] **Step 2: Build the version-matched Core assembly and update PR #5** + +Commit the test, implementation, design, and plan; push `fix/import-list-sync-db-lock` to `johoja12/Radarr`. Confirm PR #5 contains both `7df4bfa` and the new serialization commit. + +- [ ] **Step 3: Deploy only Radarr4K and validate staged scheduler cycles** + +Create a fresh verified SQLite backup, build the overlay image from the pinned 6.5.1.2032 production image, recreate only `radarr4k`, then check `/ping`, `PRAGMA quick_check`, no active commands, and no fresh locked/busy logs. Enable IDs 4, 13, and 12 one at a time, observing two scheduled cycles for each before proceeding. Disable the active list and stop if any rollback condition occurs. diff --git a/docs/superpowers/specs/2026-08-17-import-list-worker-serialization-design.md b/docs/superpowers/specs/2026-08-17-import-list-worker-serialization-design.md new file mode 100644 index 00000000000..f4f69199ffa --- /dev/null +++ b/docs/superpowers/specs/2026-08-17-import-list-worker-serialization-design.md @@ -0,0 +1,27 @@ +# Import-list Worker Serialization Design + +## Goal + +Prevent a single `ImportListSync` command from running multiple enabled import-list pipelines concurrently and contending for Radarr's SQLite database. + +## Decision + +`FetchAndParseImportListService.Fetch()` will process enabled lists in their existing factory order. A list's fetch, TMDb mapping, import-list-movie persistence, and sync-status update must finish before the next list begins. + +The existing `lock (result)` serializes only the result-update block after independently scheduled long-running workers have started. Replacing the worker task fan-out with direct loop execution makes the no-overlap invariant explicit for the entire pipeline, including fetch-start timing and any writes reached from mapping or provider callbacks. + +## Scope + +Only the all-lists `Fetch()` path changes. `FetchSingleList()` retains its current behavior. No list settings, command-queue behavior, database schema, or provider behavior changes. + +## Failure Handling + +An exception in one list remains logged and does not prevent the next enabled list from being considered, matching the current worker-body behavior. Failed reports are not persisted; blocked lists continue to set `AnyFailure`. + +## Regression Evidence + +The focused fixture will make the first list's `Fetch()` wait on a test gate. While it is blocked, the test verifies the second list's `Fetch()` has not begun. This fails on the existing task-fan-out implementation and passes only when the next list is not started until the first pipeline returns. + +## Release Validation + +Build and run focused import-list and command-queue tests, push the follow-up commit to PR #5, build a production-version-matched overlay, and deploy only Radarr4K. Keep the verified SQLite backup. Re-enable RadarrIndian (4), RadarrKids (13), and Radarr-Movies (12), in that order, one at a time; observe two five-minute scheduled cycles per list before enabling the next. On any fresh SQLite busy/locked entry, over-cadence command, or command accumulation, disable the current list and stop the rollout. diff --git a/src/NzbDrone.Core.Test/ImportListTests/FetchAndParseImportListServiceFixture.cs b/src/NzbDrone.Core.Test/ImportListTests/FetchAndParseImportListServiceFixture.cs index e89ed32d751..5be88160fb8 100644 --- a/src/NzbDrone.Core.Test/ImportListTests/FetchAndParseImportListServiceFixture.cs +++ b/src/NzbDrone.Core.Test/ImportListTests/FetchAndParseImportListServiceFixture.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; +using System.Threading.Tasks; using FizzWare.NBuilder; using FluentAssertions; using Moq; @@ -185,5 +187,44 @@ public void should_return_all_results_for_all_lists() listResult.AnyFailure.Should().BeFalse(); listResult.Movies.Count.Should().Be(5); } + + [Test] + public void should_not_start_the_next_list_until_the_current_list_finishes() + { + using var firstListStarted = new ManualResetEventSlim(); + using var releaseFirstList = new ManualResetEventSlim(); + using var secondListStarted = new ManualResetEventSlim(); + + var first = CreateListResult(1, true, true, new ImportListFetchResult()); + var second = CreateListResult(2, true, true, new ImportListFetchResult()); + + first.Setup(x => x.Fetch()).Returns(() => + { + firstListStarted.Set(); + releaseFirstList.Wait(); + return new ImportListFetchResult(); + }); + + second.Setup(x => x.Fetch()).Returns(() => + { + secondListStarted.Set(); + return new ImportListFetchResult(); + }); + + var fetch = Task.Run(() => Subject.Fetch()); + + try + { + firstListStarted.Wait(TimeSpan.FromSeconds(1)).Should().BeTrue(); + secondListStarted.Wait(TimeSpan.FromSeconds(1)).Should().BeFalse(); + } + finally + { + releaseFirstList.Set(); + } + + fetch.Wait(TimeSpan.FromSeconds(1)).Should().BeTrue(); + second.Verify(x => x.Fetch(), Times.Once()); + } } } diff --git a/src/NzbDrone.Core/ImportLists/FetchAndParseImportListService.cs b/src/NzbDrone.Core/ImportLists/FetchAndParseImportListService.cs index d47aeb71f7a..c537e22c85c 100644 --- a/src/NzbDrone.Core/ImportLists/FetchAndParseImportListService.cs +++ b/src/NzbDrone.Core/ImportLists/FetchAndParseImportListService.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Threading.Tasks; using NLog; using NzbDrone.Common.Instrumentation.Extensions; using NzbDrone.Common.TPL; @@ -58,9 +57,6 @@ public ImportListFetchResult Fetch() _logger.Debug("Available import lists {0}", importLists.Count); - var taskList = new List(); - var taskFactory = new TaskFactory(TaskCreationOptions.LongRunning, TaskContinuationOptions.None); - foreach (var importList in importLists) { var importListLocal = importList; @@ -89,46 +85,39 @@ public ImportListFetchResult Fetch() continue; } - var task = taskFactory.StartNew(() => + try { - try + var importListReports = importListLocal.Fetch(); + + lock (result) { - var importListReports = importListLocal.Fetch(); + _logger.Debug("Found {0} from Import List {1} ({2})", importListReports.Movies.Count, importList.Name, importListLocal.Definition.Name); - lock (result) + if (!importListReports.AnyFailure) { - _logger.Debug("Found {0} from Import List {1} ({2})", importListReports.Movies.Count, importList.Name, importListLocal.Definition.Name); - - if (!importListReports.AnyFailure) - { - var alreadyMapped = result.Movies.Where(x => importListReports.Movies.Any(r => r.TmdbId == x.TmdbId)); - var listMovies = MapMovieReports(importListReports.Movies.Where(x => result.Movies.All(r => r.TmdbId != x.TmdbId))).Where(x => x.TmdbId > 0).ToList(); + var alreadyMapped = result.Movies.Where(x => importListReports.Movies.Any(r => r.TmdbId == x.TmdbId)); + var listMovies = MapMovieReports(importListReports.Movies.Where(x => result.Movies.All(r => r.TmdbId != x.TmdbId))).Where(x => x.TmdbId > 0).ToList(); - listMovies.AddRange(alreadyMapped); - listMovies = listMovies.DistinctBy(x => x.TmdbId).ToList(); - listMovies.ForEach(m => m.ListId = importList.Definition.Id); + listMovies.AddRange(alreadyMapped); + listMovies = listMovies.DistinctBy(x => x.TmdbId).ToList(); + listMovies.ForEach(m => m.ListId = importList.Definition.Id); - result.Movies.AddRange(listMovies); - _listMovieService.SyncMoviesForList(listMovies, importList.Definition.Id); - } + result.Movies.AddRange(listMovies); + _listMovieService.SyncMoviesForList(listMovies, importList.Definition.Id); + } - result.AnyFailure |= importListReports.AnyFailure; - result.SyncedLists++; + result.AnyFailure |= importListReports.AnyFailure; + result.SyncedLists++; - _importListStatusService.UpdateListSyncStatus(importList.Definition.Id); - } - } - catch (Exception e) - { - _logger.Error(e, "Error during Import List Sync of {0} ({1})", importList.Name, importListLocal.Definition.Name); + _importListStatusService.UpdateListSyncStatus(importList.Definition.Id); } - }).LogExceptions(); - - taskList.Add(task); + } + catch (Exception e) + { + _logger.Error(e, "Error during Import List Sync of {0} ({1})", importList.Name, importListLocal.Definition.Name); + } } - Task.WaitAll(taskList.ToArray()); - result.Movies = result.Movies.DistinctBy(r => new { r.TmdbId, r.ImdbId, r.Title }).ToList(); _logger.Debug("Found {0} total reports from {1} lists", result.Movies.Count, result.SyncedLists); From 6895c30ff73624acef52b06ac92a3e21d97d0adb Mon Sep 17 00:00:00 2001 From: johoja12 <223961+johoja12@users.noreply.github.com> Date: Tue, 18 Aug 2026 08:43:16 +0000 Subject: [PATCH 04/12] docs: define import-list sqlite contention fix --- ...18-import-list-sqlite-contention-design.md | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-18-import-list-sqlite-contention-design.md diff --git a/docs/superpowers/specs/2026-08-18-import-list-sqlite-contention-design.md b/docs/superpowers/specs/2026-08-18-import-list-sqlite-contention-design.md new file mode 100644 index 00000000000..d15b58ad251 --- /dev/null +++ b/docs/superpowers/specs/2026-08-18-import-list-sqlite-contention-design.md @@ -0,0 +1,45 @@ +# Import-list SQLite Contention Design + +## Goal + +Allow all three Radarr4K import lists to remain enabled without recurring SQLite `Busy` failures, while preserving the configured `keepAndUnmonitor` library policy. + +## Evidence + +- `FetchAndParseImportListService` now processes import lists sequentially, so the original parallel per-list writer collision is removed. +- `ImportListMovieService.SyncMoviesForList` still calls `UpdateMany` for every existing list movie on every refresh. The live lists contain 1,322 Indian, 3,241 Movies, and 609 Kids records: 5,172 unchanged records would be rewritten every five minutes. +- With `keepAndUnmonitor`, `ImportListSyncService.CleanLibrary` currently includes movies that are already unmonitored. The live database has 40 such movies outside all lists, so every sync re-saves unchanged records. +- The deployed version configures SQLite with a 100 ms busy timeout. Current upstream Radarr commit `520bf4215a13` increases it to 1,000 ms specifically for SQLite busy handling. + +## Design + +### SQLite tuning + +Set the SQLite connection busy timeout to 1,000 ms, matching upstream Radarr. This lets a legitimate short WAL write complete instead of failing an unrelated reader immediately; it does not conceal a long-running transaction. + +### Import-list persistence + +Treat the existing import-list record as authoritative when its `TmdbId` already maps to the current metadata row. During a normal refresh, only: + +- insert records newly returned by a list; +- delete records removed from a list; and +- update an existing row if its persisted `MovieMetadataId` differs from the current metadata row. + +The operation must not issue an update for an unchanged row. The existing sequential list-worker behavior remains unchanged. + +### Library cleanup + +For `keepAndUnmonitor`, collect only movies that are both absent from all synced lists and currently monitored. Skip the bulk update entirely when the collection is empty. This preserves the policy: any movie requiring an unmonitor transition is still updated exactly once. + +## Validation + +Automated tests will prove that unchanged list rows do not reach `UpdateMany`, unchanged already-unmonitored movies do not reach `UpdateMovie`, and changed metadata mappings do still update. The source test suite must pass before deployment. + +After deployment, leave Indian enabled and enable Kids, then Movies. For each step, observe two scheduled import cycles, verify each command completes before a backlog forms, and require zero new SQLite `Busy` entries. Disable only the list under test if a gate fails. + +## Non-goals + +- No migration to PostgreSQL. +- No change to `keepAndUnmonitor` semantics. +- No increase to the five-minute scheduler interval. +- No retry loop or global lock around unrelated API traffic. From bc520131e98134abd2dd4c0b5409631a9597236a Mon Sep 17 00:00:00 2001 From: johoja12 <223961+johoja12@users.noreply.github.com> Date: Tue, 18 Aug 2026 08:46:49 +0000 Subject: [PATCH 05/12] docs: plan import-list sqlite remediation --- ...026-08-18-import-list-sqlite-contention.md | 260 ++++++++++++++++++ 1 file changed, 260 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-18-import-list-sqlite-contention.md diff --git a/docs/superpowers/plans/2026-08-18-import-list-sqlite-contention.md b/docs/superpowers/plans/2026-08-18-import-list-sqlite-contention.md new file mode 100644 index 00000000000..0cb32f8b4a8 --- /dev/null +++ b/docs/superpowers/plans/2026-08-18-import-list-sqlite-contention.md @@ -0,0 +1,260 @@ +# Import-list SQLite Contention Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Eliminate the unnecessary SQLite write pressure that prevents all Radarr4K import lists from coexisting, without changing import-list or library-cleanup semantics. + +**Architecture:** Keep the existing serial per-list pipeline and make its persistence delta-based: insert new list mappings, update only mappings whose metadata identity changes, and delete removed mappings. Keep `keepAndUnmonitor`, but write only movies that transition from monitored to unmonitored. Use the current upstream Radarr SQLite busy timeout of one second so short WAL conflicts are waited out rather than returned as errors. + +**Tech Stack:** C#, .NET, System.Data.SQLite, NUnit, Moq, FluentAssertions. + +--- + +### Task 1: Test and tune the SQLite connection timeout + +**Files:** +- Create: `src/NzbDrone.Core.Test/Datastore/ConnectionStringFactoryFixture.cs` +- Modify: `src/NzbDrone.Core/Datastore/ConnectionStringFactory.cs:44-53` + +- [ ] **Step 1: Write the failing test** + +```csharp +[Test] +public void should_configure_sqlite_busy_timeout_for_one_second() +{ + var connection = Mocker.Resolve().MainDbConnection.ConnectionString; + var builder = new SQLiteConnectionStringBuilder(connection); + + builder.BusyTimeout.Should().Be(1000); +} +``` + +Create the fixture as `CoreTest`, import `System.Data.SQLite`, `FluentAssertions`, `NUnit.Framework`, `NzbDrone.Core.Datastore`, and `NzbDrone.Core.Test.Framework`. + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `dotnet test src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj --filter FullyQualifiedName~ConnectionStringFactoryFixture --no-restore` + +Expected: the assertion reports the current `BusyTimeout` of `100` instead of `1000`. + +- [ ] **Step 3: Make the minimal implementation change** + +In `ConnectionStringFactory.GetConnectionString`, change only: + +```csharp +BusyTimeout = 100 +``` + +to: + +```csharp +BusyTimeout = 1000 +``` + +- [ ] **Step 4: Run the focused test to verify it passes** + +Run: `dotnet test src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj --filter FullyQualifiedName~ConnectionStringFactoryFixture --no-restore` + +Expected: one passing test, zero failures. + +- [ ] **Step 5: Commit the completed task** + +```bash +git add src/NzbDrone.Core/Datastore/ConnectionStringFactory.cs src/NzbDrone.Core.Test/Datastore/ConnectionStringFactoryFixture.cs +git commit -m "fix: wait for transient sqlite locks" +``` + +### Task 2: Persist import-list mappings only when they change + +**Files:** +- Create: `src/NzbDrone.Core.Test/ImportListTests/ImportListMovieServiceFixture.cs` +- Modify: `src/NzbDrone.Core/ImportLists/ImportListMovies/ImportListMovieService.cs:43-57` + +- [ ] **Step 1: Write failing unit tests** + +Create a `CoreTest` fixture. Build `ImportListMovie` helpers with a valid `Id`, `ListId`, `MovieMetadataId`, and lazy `MovieMetadata` containing a `TmdbId`. Configure `IImportListMovieRepository.GetAllForLists` for the target list. Add these tests: + +```csharp +[Test] +public void should_not_update_an_existing_mapping_with_the_same_metadata() +{ + GivenExisting(1, 101, 5001); + var incoming = GivenIncoming(101, 5001); + + Subject.SyncMoviesForList(new List { incoming }, 1); + + Mocker.GetMock() + .Verify(x => x.UpdateMany(It.IsAny>()), Times.Never()); +} + +[Test] +public void should_update_an_existing_mapping_when_its_metadata_identity_changes() +{ + GivenExisting(1, 101, 5001); + var incoming = GivenIncoming(101, 5002); + + Subject.SyncMoviesForList(new List { incoming }, 1); + + Mocker.GetMock() + .Verify(x => x.UpdateMany(It.Is>(x => x.Single().Id == 1 && x.Single().MovieMetadataId == 5002)), Times.Once()); +} + +[Test] +public void should_insert_new_and_delete_removed_mappings() +{ + GivenExisting(1, 101, 5001); + var incoming = GivenIncoming(202, 6001); + + Subject.SyncMoviesForList(new List { incoming }, 1); + + Mocker.GetMock().Verify(x => x.InsertMany(It.Is>(m => m.Single().TmdbId == 202)), Times.Once()); + Mocker.GetMock().Verify(x => x.DeleteMany(It.Is>(ids => ids.Single() == 1)), Times.Once()); +} +``` + +- [ ] **Step 2: Run the fixture to verify the no-op test fails** + +Run: `dotnet test src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj --filter FullyQualifiedName~ImportListMovieServiceFixture --no-restore` + +Expected: `should_not_update_an_existing_mapping_with_the_same_metadata` fails because the current implementation calls `UpdateMany` for every existing mapping. + +- [ ] **Step 3: Implement delta-based persistence** + +In `SyncMoviesForList`: + +```csharp +var existingListMovies = GetAllForLists(new List { listId }); +var existingByTmdbId = existingListMovies.ToDictionary(x => x.TmdbId); + +var inserts = listMovies.Where(x => !existingByTmdbId.TryGetValue(x.TmdbId, out _)).ToList(); +var updates = new List(); + +foreach (var listMovie in listMovies.Where(x => existingByTmdbId.TryGetValue(x.TmdbId, out _))) +{ + var existing = existingByTmdbId[listMovie.TmdbId]; + listMovie.Id = existing.Id; + + if (listMovie.MovieMetadataId != existing.MovieMetadataId) + { + updates.Add(listMovie); + } +} + +var deletes = existingListMovies.Where(x => listMovies.All(y => y.TmdbId != x.TmdbId)).Select(x => x.Id).ToList(); + +if (inserts.Any()) { _importListMovieRepository.InsertMany(inserts); } +if (updates.Any()) { _importListMovieRepository.UpdateMany(updates); } +if (deletes.Any()) { _importListMovieRepository.DeleteMany(deletes); } +``` + +Keep the returned list and the existing `TmdbId` identity behavior unchanged. + +- [ ] **Step 4: Run the fixture to verify all delta cases pass** + +Run: `dotnet test src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj --filter FullyQualifiedName~ImportListMovieServiceFixture --no-restore` + +Expected: three passing tests, zero failures. + +- [ ] **Step 5: Commit the completed task** + +```bash +git add src/NzbDrone.Core/ImportLists/ImportListMovies/ImportListMovieService.cs src/NzbDrone.Core.Test/ImportListTests/ImportListMovieServiceFixture.cs +git commit -m "fix: skip unchanged import-list writes" +``` + +### Task 3: Update only movies that need the unmonitor transition + +**Files:** +- Modify: `src/NzbDrone.Core/ImportLists/ImportListSyncService.cs:201-227` +- Modify: `src/NzbDrone.Core.Test/ImportListTests/ImportListSyncServiceFixture.cs:202-257` + +- [ ] **Step 1: Write a failing regression test** + +Add this test to `ImportListSyncServiceFixture`: + +```csharp +[Test] +public void should_not_update_already_unmonitored_movies_when_cleaning_library() +{ + _importListFetch.Movies.ForEach(m => m.ListId = 1); + GivenList(1, true); + GivenCleanLevel("keepAndUnmonitor"); + + var alreadyUnmonitored = _existingMovies.Select(x => + { + x.Monitored = false; + return x; + }).ToList(); + + Mocker.GetMock().Setup(v => v.GetAllMovies()).Returns(alreadyUnmonitored); + Mocker.GetMock().Setup(v => v.GetAllListMovies()).Returns(_list1Movies); + + Subject.Execute(_commandAll); + + Mocker.GetMock().Verify(v => v.UpdateMovie(It.IsAny>(), true), Times.Never()); +} +``` + +- [ ] **Step 2: Run the fixture to verify the test fails** + +Run: `dotnet test src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj --filter FullyQualifiedName~ImportListSyncServiceFixture --no-restore` + +Expected: the new test fails because the current code sends already-unmonitored movies to `UpdateMovie`. + +- [ ] **Step 3: Implement the minimal cleanup guard** + +In the `keepAndUnmonitor` branch, require `movie.Monitored` before logging and adding the movie: + +```csharp +case "keepAndUnmonitor" when movie.Monitored: + _logger.Info("{0} was in your library, but not found in your lists --> Keeping in library but Unmonitoring it", movie); + movie.Monitored = false; + moviesToUpdate.Add(movie); + break; +``` + +Then invoke `_movieService.UpdateMovie(moviesToUpdate, true)` only when `moviesToUpdate.Any()`. + +- [ ] **Step 4: Run the focused fixture and preserve the transition behavior** + +Run: `dotnet test src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj --filter FullyQualifiedName~ImportListSyncServiceFixture --no-restore` + +Expected: all existing cleanup tests and the new already-unmonitored regression pass. + +- [ ] **Step 5: Commit the completed task** + +```bash +git add src/NzbDrone.Core/ImportLists/ImportListSyncService.cs src/NzbDrone.Core.Test/ImportListTests/ImportListSyncServiceFixture.cs +git commit -m "fix: skip unchanged library cleanup writes" +``` + +### Task 4: Verify, publish, deploy, and stage-enable lists + +**Files:** +- Modify: no source files + +- [ ] **Step 1: Run the complete targeted regression suite** + +Run: + +```bash +dotnet test src/NzbDrone.Core.Test/NzbDrone.Core.Test.csproj --filter 'FullyQualifiedName~ConnectionStringFactoryFixture|FullyQualifiedName~ImportListMovieServiceFixture|FullyQualifiedName~ImportListSyncServiceFixture|FullyQualifiedName~FetchAndParseImportListServiceFixture|FullyQualifiedName~CommandQueueManagerFixture|FullyQualifiedName~CommandQueueFixture' --no-restore +``` + +Expected: zero test failures. + +- [ ] **Step 2: Build the production image and preserve a database backup** + +Build the Radarr image using the established `radarr4k:6.5.1.2032-import-list-delta-persistence-` tag. Before recreating the container, create a timestamped, verified SQLite backup on Synology; do not modify list state during backup. + +- [ ] **Step 3: Deploy only Radarr4K and verify readiness** + +Replace only the Radarr4K image/container. Verify `/ping`, `PRAGMA quick_check`, WAL journal mode, the running image tag, no started commands, and zero new `Busy` entries after startup. + +- [ ] **Step 4: Stage-enable remaining lists without a manual sync** + +Keep Indian enabled. Enable Kids through Radarr’s API, then observe two natural scheduler cycles. If both finish without a command backlog or `Busy`, enable Movies and observe two more natural cycles. Do not enable a subsequent list if the current gate fails; immediately re-pause only the list under test. + +- [ ] **Step 5: Create the PR and report live verification separately** + +Push the branch and update the existing pull request with the design, source changes, targeted test output, image tag, backup confirmation, and each scheduler-cycle result. Do not claim all-list success unless all four staged cycles complete with no fresh lock entries. From 1afbcff522170d17bb45da9a93f5c9f18343bc10 Mon Sep 17 00:00:00 2001 From: johoja12 <223961+johoja12@users.noreply.github.com> Date: Tue, 18 Aug 2026 08:47:25 +0000 Subject: [PATCH 06/12] docs: correct sqlite timeout test setup --- .../plans/2026-08-18-import-list-sqlite-contention.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/superpowers/plans/2026-08-18-import-list-sqlite-contention.md b/docs/superpowers/plans/2026-08-18-import-list-sqlite-contention.md index 0cb32f8b4a8..3ac176f14d3 100644 --- a/docs/superpowers/plans/2026-08-18-import-list-sqlite-contention.md +++ b/docs/superpowers/plans/2026-08-18-import-list-sqlite-contention.md @@ -29,7 +29,7 @@ public void should_configure_sqlite_busy_timeout_for_one_second() } ``` -Create the fixture as `CoreTest`, import `System.Data.SQLite`, `FluentAssertions`, `NUnit.Framework`, `NzbDrone.Core.Datastore`, and `NzbDrone.Core.Test.Framework`. +Create the fixture as `DbTest`, which provisions the real `IConnectionStringFactory` and SQLite configuration. Import `System.Data.SQLite`, `FluentAssertions`, `NUnit.Framework`, `NzbDrone.Core.Datastore`, and `NzbDrone.Core.Test.Framework`. - [ ] **Step 2: Run the test to verify it fails** From d393de2403afe35d81e423ec32a22e5ba3011a6b Mon Sep 17 00:00:00 2001 From: johoja12 <223961+johoja12@users.noreply.github.com> Date: Tue, 18 Aug 2026 08:57:19 +0000 Subject: [PATCH 07/12] fix: wait for transient sqlite locks --- .../ConnectionStringFactoryFixture.cs | 21 +++++++++++++++++++ .../Datastore/ConnectionStringFactory.cs | 2 +- 2 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 src/NzbDrone.Core.Test/Datastore/ConnectionStringFactoryFixture.cs diff --git a/src/NzbDrone.Core.Test/Datastore/ConnectionStringFactoryFixture.cs b/src/NzbDrone.Core.Test/Datastore/ConnectionStringFactoryFixture.cs new file mode 100644 index 00000000000..5332c50b11d --- /dev/null +++ b/src/NzbDrone.Core.Test/Datastore/ConnectionStringFactoryFixture.cs @@ -0,0 +1,21 @@ +using System.Data.SQLite; +using FluentAssertions; +using NUnit.Framework; +using NzbDrone.Core.Datastore; +using NzbDrone.Core.Test.Framework; + +namespace NzbDrone.Core.Test.Datastore +{ + [TestFixture] + public class ConnectionStringFactoryFixture : DbTest + { + [Test] + public void should_configure_sqlite_busy_timeout_for_one_second() + { + var connection = Mocker.Resolve().MainDbConnection.ConnectionString; + var builder = new SQLiteConnectionStringBuilder(connection); + + builder.BusyTimeout.Should().Be(1000); + } + } +} diff --git a/src/NzbDrone.Core/Datastore/ConnectionStringFactory.cs b/src/NzbDrone.Core/Datastore/ConnectionStringFactory.cs index 19c938737c2..72244099334 100644 --- a/src/NzbDrone.Core/Datastore/ConnectionStringFactory.cs +++ b/src/NzbDrone.Core/Datastore/ConnectionStringFactory.cs @@ -49,7 +49,7 @@ private static DatabaseConnectionInfo GetConnectionString(string dbPath) JournalMode = OsInfo.IsOsx ? SQLiteJournalModeEnum.Truncate : SQLiteJournalModeEnum.Wal, Pooling = true, Version = 3, - BusyTimeout = 100 + BusyTimeout = 1000 }; if (OsInfo.IsOsx) From fd2ab4828a0512ae2d978b9ce5c04b744aaaa25d Mon Sep 17 00:00:00 2001 From: johoja12 <223961+johoja12@users.noreply.github.com> Date: Tue, 18 Aug 2026 09:00:29 +0000 Subject: [PATCH 08/12] fix: skip unchanged import-list writes --- .../ImportListMovieServiceFixture.cs | 88 +++++++++++++++++++ .../ImportListMovieService.cs | 43 +++++++-- 2 files changed, 125 insertions(+), 6 deletions(-) create mode 100644 src/NzbDrone.Core.Test/ImportListTests/ImportListMovieServiceFixture.cs diff --git a/src/NzbDrone.Core.Test/ImportListTests/ImportListMovieServiceFixture.cs b/src/NzbDrone.Core.Test/ImportListTests/ImportListMovieServiceFixture.cs new file mode 100644 index 00000000000..a02007501bb --- /dev/null +++ b/src/NzbDrone.Core.Test/ImportListTests/ImportListMovieServiceFixture.cs @@ -0,0 +1,88 @@ +using System.Collections.Generic; +using System.Linq; +using Moq; +using NUnit.Framework; +using NzbDrone.Core.Datastore; +using NzbDrone.Core.ImportLists.ImportListMovies; +using NzbDrone.Core.Movies; +using NzbDrone.Core.Test.Framework; + +namespace NzbDrone.Core.Test.ImportListTests +{ + [TestFixture] + public class ImportListMovieServiceFixture : CoreTest + { + private List _existingListMovies; + + [SetUp] + public void Setup() + { + _existingListMovies = new List(); + + Mocker.GetMock() + .Setup(v => v.GetAllForLists(It.IsAny>())) + .Returns(_existingListMovies); + } + + private ImportListMovie GivenExisting(int id, int tmdbId, int movieMetadataId) + { + var listMovie = GivenIncoming(tmdbId, movieMetadataId); + listMovie.Id = id; + _existingListMovies.Add(listMovie); + + return listMovie; + } + + private ImportListMovie GivenIncoming(int tmdbId, int movieMetadataId) + { + return new ImportListMovie + { + ListId = 1, + MovieMetadataId = movieMetadataId, + MovieMetadata = new LazyLoaded(new MovieMetadata + { + Id = movieMetadataId, + TmdbId = tmdbId + }) + }; + } + + [Test] + public void should_not_update_an_existing_mapping_with_the_same_metadata() + { + GivenExisting(1, 101, 5001); + var incoming = GivenIncoming(101, 5001); + + Subject.SyncMoviesForList(new List { incoming }, 1); + + Mocker.GetMock() + .Verify(v => v.UpdateMany(It.IsAny>()), Times.Never()); + } + + [Test] + public void should_update_an_existing_mapping_when_its_metadata_identity_changes() + { + GivenExisting(1, 101, 5001); + var incoming = GivenIncoming(101, 5002); + + Subject.SyncMoviesForList(new List { incoming }, 1); + + Mocker.GetMock() + .Verify(v => v.UpdateMany(It.Is>(x => x.Single().Id == 1 && x.Single().MovieMetadataId == 5002)), Times.Once()); + } + + [Test] + public void should_insert_new_and_delete_removed_mappings() + { + GivenExisting(1, 101, 5001); + var incoming = GivenIncoming(202, 6001); + + Subject.SyncMoviesForList(new List { incoming }, 1); + + Mocker.GetMock() + .Verify(v => v.InsertMany(It.Is>(x => x.Single().TmdbId == 202)), Times.Once()); + Mocker.GetMock() + .Verify(v => v.DeleteMany(It.Is>(x => x.Single().Id == 1)), Times.Once()); + } + } +} diff --git a/src/NzbDrone.Core/ImportLists/ImportListMovies/ImportListMovieService.cs b/src/NzbDrone.Core/ImportLists/ImportListMovies/ImportListMovieService.cs index f414984816a..ca0d2ef2a0e 100644 --- a/src/NzbDrone.Core/ImportLists/ImportListMovies/ImportListMovieService.cs +++ b/src/NzbDrone.Core/ImportLists/ImportListMovies/ImportListMovieService.cs @@ -45,12 +45,43 @@ public List AddListMovies(List listMovies) public List SyncMoviesForList(List listMovies, int listId) { var existingListMovies = GetAllForLists(new List { listId }); - - listMovies.ForEach(l => l.Id = existingListMovies.FirstOrDefault(e => e.TmdbId == l.TmdbId)?.Id ?? 0); - - _importListMovieRepository.InsertMany(listMovies.Where(l => l.Id == 0).ToList()); - _importListMovieRepository.UpdateMany(listMovies.Where(l => l.Id > 0).ToList()); - _importListMovieRepository.DeleteMany(existingListMovies.Where(l => listMovies.All(x => x.TmdbId != l.TmdbId)).ToList()); + var existingByTmdbId = existingListMovies.ToDictionary(x => x.TmdbId); + var listMoviesToInsert = new List(); + var listMoviesToUpdate = new List(); + + foreach (var listMovie in listMovies) + { + if (existingByTmdbId.TryGetValue(listMovie.TmdbId, out var existingListMovie)) + { + listMovie.Id = existingListMovie.Id; + + if (listMovie.MovieMetadataId != existingListMovie.MovieMetadataId) + { + listMoviesToUpdate.Add(listMovie); + } + } + else + { + listMoviesToInsert.Add(listMovie); + } + } + + var listMoviesToDelete = existingListMovies.Where(l => listMovies.All(x => x.TmdbId != l.TmdbId)).ToList(); + + if (listMoviesToInsert.Any()) + { + _importListMovieRepository.InsertMany(listMoviesToInsert); + } + + if (listMoviesToUpdate.Any()) + { + _importListMovieRepository.UpdateMany(listMoviesToUpdate); + } + + if (listMoviesToDelete.Any()) + { + _importListMovieRepository.DeleteMany(listMoviesToDelete); + } return listMovies; } From b86f75ca60009224d26d1e4b735b4af220f36027 Mon Sep 17 00:00:00 2001 From: johoja12 <223961+johoja12@users.noreply.github.com> Date: Tue, 18 Aug 2026 09:03:36 +0000 Subject: [PATCH 09/12] fix: skip unchanged library cleanup writes --- .../ImportListSyncServiceFixture.cs | 35 +++++++++++++++++-- .../ImportLists/ImportListSyncService.cs | 15 +++++--- 2 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncServiceFixture.cs b/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncServiceFixture.cs index 876f99a1fd6..689174d0f58 100644 --- a/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncServiceFixture.cs +++ b/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncServiceFixture.cs @@ -195,7 +195,7 @@ public void should_log_only_on_clean_library_if_config_value_logonly() .Verify(v => v.DeleteMovie(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never()); Mocker.GetMock() - .Verify(v => v.UpdateMovie(new List(), true), Times.Once()); + .Verify(v => v.UpdateMovie(It.IsAny>(), true), Times.Never()); } [Test] @@ -205,6 +205,8 @@ public void should_unmonitor_on_clean_library_if_config_value_keepAndUnmonitor() GivenList(1, true); GivenCleanLevel("keepAndUnmonitor"); + _existingMovies.ForEach(m => m.Monitored = true); + Mocker.GetMock() .Setup(v => v.GetAllMovies()) .Returns(_existingMovies); @@ -225,6 +227,29 @@ public void should_unmonitor_on_clean_library_if_config_value_keepAndUnmonitor() .Verify(v => v.UpdateMovie(It.Is>(s => s.Count == 3 && s.All(m => !m.Monitored)), true), Times.Once()); } + [Test] + public void should_not_update_already_unmonitored_movies_when_cleaning_library() + { + _importListFetch.Movies.ForEach(m => m.ListId = 1); + GivenList(1, true); + GivenCleanLevel("keepAndUnmonitor"); + + _existingMovies.ForEach(m => m.Monitored = false); + + Mocker.GetMock() + .Setup(v => v.GetAllMovies()) + .Returns(_existingMovies); + + Mocker.GetMock() + .Setup(v => v.GetAllListMovies()) + .Returns(_list1Movies); + + Subject.Execute(_commandAll); + + Mocker.GetMock() + .Verify(v => v.UpdateMovie(It.IsAny>(), true), Times.Never()); + } + [Test] public void should_not_clean_on_clean_library_if_tmdb_match() { @@ -234,6 +259,8 @@ public void should_not_clean_on_clean_library_if_tmdb_match() GivenList(1, true); GivenCleanLevel("keepAndUnmonitor"); + _existingMovies.ForEach(m => m.Monitored = true); + Mocker.GetMock() .Setup(v => v.GetAllMovies()) .Returns(_existingMovies); @@ -258,6 +285,8 @@ public void should_fallback_to_imdbid_on_clean_library_if_tmdb_not_found() GivenList(1, true); GivenCleanLevel("keepAndUnmonitor"); + _existingMovies.ForEach(m => m.Monitored = true); + Mocker.GetMock() .Setup(v => v.GetAllMovies()) .Returns(_existingMovies); @@ -299,7 +328,7 @@ public void should_delete_movies_not_files_on_clean_library_if_config_value_logo .Verify(v => v.DeleteMovie(It.IsAny(), true, It.IsAny()), Times.Never()); Mocker.GetMock() - .Verify(v => v.UpdateMovie(new List(), true), Times.Once()); + .Verify(v => v.UpdateMovie(It.IsAny>(), true), Times.Never()); } [Test] @@ -329,7 +358,7 @@ public void should_delete_movies_and_files_on_clean_library_if_config_value_logo .Verify(v => v.DeleteMovie(It.IsAny(), true, It.IsAny()), Times.Exactly(3)); Mocker.GetMock() - .Verify(v => v.UpdateMovie(new List(), true), Times.Once()); + .Verify(v => v.UpdateMovie(It.IsAny>(), true), Times.Never()); } [Test] diff --git a/src/NzbDrone.Core/ImportLists/ImportListSyncService.cs b/src/NzbDrone.Core/ImportLists/ImportListSyncService.cs index b96e07a0fb2..fecba1b0021 100644 --- a/src/NzbDrone.Core/ImportLists/ImportListSyncService.cs +++ b/src/NzbDrone.Core/ImportLists/ImportListSyncService.cs @@ -208,9 +208,13 @@ private void CleanLibrary() _logger.Info("{0} was in your library, but not found in your lists --> You might want to unmonitor or remove it", movie); break; case "keepAndUnmonitor": - _logger.Info("{0} was in your library, but not found in your lists --> Keeping in library but Unmonitoring it", movie); - movie.Monitored = false; - moviesToUpdate.Add(movie); + if (movie.Monitored) + { + _logger.Info("{0} was in your library, but not found in your lists --> Keeping in library but Unmonitoring it", movie); + movie.Monitored = false; + moviesToUpdate.Add(movie); + } + break; case "removeAndKeep": _logger.Info("{0} was in your library, but not found in your lists --> Removing from library (keeping files)", movie); @@ -224,7 +228,10 @@ private void CleanLibrary() } } - _movieService.UpdateMovie(moviesToUpdate, true); + if (moviesToUpdate.Any()) + { + _movieService.UpdateMovie(moviesToUpdate, true); + } } } } From 78f00b4302215787f5de5b2434798dd367dec623 Mon Sep 17 00:00:00 2001 From: johoja12 <223961+johoja12@users.noreply.github.com> Date: Tue, 18 Aug 2026 10:33:44 +0000 Subject: [PATCH 10/12] fix: serialize sqlite repository writes --- .../Datastore/BasicRepositoryFixture.cs | 29 ++++++ .../Datastore/BasicRepository.cs | 88 ++++++++++++------- .../Datastore/SqliteWriteCoordinator.cs | 37 ++++++++ 3 files changed, 122 insertions(+), 32 deletions(-) create mode 100644 src/NzbDrone.Core/Datastore/SqliteWriteCoordinator.cs diff --git a/src/NzbDrone.Core.Test/Datastore/BasicRepositoryFixture.cs b/src/NzbDrone.Core.Test/Datastore/BasicRepositoryFixture.cs index fac7e7f00d6..52b14c24678 100644 --- a/src/NzbDrone.Core.Test/Datastore/BasicRepositoryFixture.cs +++ b/src/NzbDrone.Core.Test/Datastore/BasicRepositoryFixture.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; +using System.Threading.Tasks; using FizzWare.NBuilder; using FluentAssertions; using NUnit.Framework; @@ -298,6 +300,33 @@ public void should_be_able_to_call_ToList_on_empty_queryable() Subject.All().ToList().Should().BeEmpty(); } + [Test] + public void should_serialize_sqlite_writes_for_the_same_database() + { + using var firstEntered = new ManualResetEventSlim(); + using var releaseFirst = new ManualResetEventSlim(); + using var secondEntered = new ManualResetEventSlim(); + + var database = Mocker.Resolve(); + + var first = Task.Run(() => SqliteWriteCoordinator.Execute(database, () => + { + firstEntered.Set(); + releaseFirst.Wait(); + })); + + firstEntered.Wait(); + + var second = Task.Run(() => SqliteWriteCoordinator.Execute(database, secondEntered.Set)); + + secondEntered.Wait(TimeSpan.FromMilliseconds(100)).Should().BeFalse(); + + releaseFirst.Set(); + Task.WaitAll(first, second); + + secondEntered.IsSet.Should().BeTrue(); + } + [TestCase(1, 2)] [TestCase(2, 2)] [TestCase(3, 1)] diff --git a/src/NzbDrone.Core/Datastore/BasicRepository.cs b/src/NzbDrone.Core/Datastore/BasicRepository.cs index 618999992a6..4df944b3d0f 100644 --- a/src/NzbDrone.Core/Datastore/BasicRepository.cs +++ b/src/NzbDrone.Core/Datastore/BasicRepository.cs @@ -165,10 +165,13 @@ public TModel Insert(TModel model) throw new InvalidOperationException("Can't insert model with existing ID " + model.Id); } - using (var conn = _database.OpenConnection()) + model = SqliteWriteCoordinator.Execute(_database, () => { - model = Insert(conn, null, model); - } + using (var conn = _database.OpenConnection()) + { + return Insert(conn, null, model); + } + }); ModelCreated(model); @@ -227,18 +230,21 @@ public void InsertMany(IList models) throw new InvalidOperationException("Can't insert model with existing ID != 0"); } - using (var conn = _database.OpenConnection()) + SqliteWriteCoordinator.Execute(_database, () => { - using (var tran = conn.BeginTransaction(IsolationLevel.ReadCommitted)) + using (var conn = _database.OpenConnection()) { - foreach (var model in models) + using (var tran = conn.BeginTransaction(IsolationLevel.ReadCommitted)) { - Insert(conn, tran, model); - } + foreach (var model in models) + { + Insert(conn, tran, model); + } - tran.Commit(); + tran.Commit(); + } } - } + }); } public TModel Update(TModel model) @@ -248,10 +254,13 @@ public TModel Update(TModel model) throw new InvalidOperationException("Can't update model with ID 0"); } - using (var conn = _database.OpenConnection()) + SqliteWriteCoordinator.Execute(_database, () => { - UpdateFields(conn, null, model, _properties); - } + using (var conn = _database.OpenConnection()) + { + UpdateFields(conn, null, model, _properties); + } + }); ModelUpdated(model); @@ -265,10 +274,13 @@ public void UpdateMany(IList models) throw new InvalidOperationException("Can't update model with ID 0"); } - using (var conn = _database.OpenConnection()) + SqliteWriteCoordinator.Execute(_database, () => { - UpdateFields(conn, null, models, _properties); - } + using (var conn = _database.OpenConnection()) + { + UpdateFields(conn, null, models, _properties); + } + }); } protected void Delete(Expression> where) @@ -280,10 +292,13 @@ protected void Delete(SqlBuilder builder) { var sql = builder.AddDeleteTemplate(typeof(TModel)); - using (var conn = _database.OpenConnection()) + SqliteWriteCoordinator.Execute(_database, () => { - conn.Execute(sql.RawSql, sql.Parameters); - } + using (var conn = _database.OpenConnection()) + { + conn.Execute(sql.RawSql, sql.Parameters); + } + }); } public void Delete(TModel model) @@ -323,15 +338,18 @@ public TModel Upsert(TModel model) public void Purge(bool vacuum = false) { - using (var conn = _database.OpenConnection()) + SqliteWriteCoordinator.Execute(_database, () => { - conn.Execute($"DELETE FROM \"{_table}\""); - } + using (var conn = _database.OpenConnection()) + { + conn.Execute($"DELETE FROM \"{_table}\""); + } - if (vacuum) - { - Vacuum(); - } + if (vacuum) + { + Vacuum(); + } + }); } protected void Vacuum() @@ -353,10 +371,13 @@ public void SetFields(TModel model, params Expression>[] pr var propertiesToUpdate = properties.Select(x => x.GetMemberName()).ToList(); - using (var conn = _database.OpenConnection()) + SqliteWriteCoordinator.Execute(_database, () => { - UpdateFields(conn, null, model, propertiesToUpdate); - } + using (var conn = _database.OpenConnection()) + { + UpdateFields(conn, null, model, propertiesToUpdate); + } + }); ModelUpdated(model); } @@ -370,10 +391,13 @@ public void SetFields(IList models, params Expression x.GetMemberName()).ToList(); - using (var conn = _database.OpenConnection()) + SqliteWriteCoordinator.Execute(_database, () => { - UpdateFields(conn, null, models, propertiesToUpdate); - } + using (var conn = _database.OpenConnection()) + { + UpdateFields(conn, null, models, propertiesToUpdate); + } + }); foreach (var model in models) { diff --git a/src/NzbDrone.Core/Datastore/SqliteWriteCoordinator.cs b/src/NzbDrone.Core/Datastore/SqliteWriteCoordinator.cs new file mode 100644 index 00000000000..1b28ab92845 --- /dev/null +++ b/src/NzbDrone.Core/Datastore/SqliteWriteCoordinator.cs @@ -0,0 +1,37 @@ +using System; +using System.Runtime.CompilerServices; + +namespace NzbDrone.Core.Datastore +{ + internal static class SqliteWriteCoordinator + { + private static readonly ConditionalWeakTable Locks = new ConditionalWeakTable(); + + public static void Execute(IDatabase database, Action action) + { + if (database.DatabaseType != DatabaseType.SQLite) + { + action(); + return; + } + + lock (Locks.GetValue(database, _ => new object())) + { + action(); + } + } + + public static T Execute(IDatabase database, Func action) + { + if (database.DatabaseType != DatabaseType.SQLite) + { + return action(); + } + + lock (Locks.GetValue(database, _ => new object())) + { + return action(); + } + } + } +} From fd442970db254450279efdf33515f11c4dbc66a0 Mon Sep 17 00:00:00 2001 From: johoja12 <223961+johoja12@users.noreply.github.com> Date: Tue, 18 Aug 2026 10:44:14 +0000 Subject: [PATCH 11/12] fix: tolerate longer sqlite writer windows --- .../Datastore/ConnectionStringFactoryFixture.cs | 2 +- src/NzbDrone.Core/Datastore/ConnectionStringFactory.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/NzbDrone.Core.Test/Datastore/ConnectionStringFactoryFixture.cs b/src/NzbDrone.Core.Test/Datastore/ConnectionStringFactoryFixture.cs index 5332c50b11d..676114f446c 100644 --- a/src/NzbDrone.Core.Test/Datastore/ConnectionStringFactoryFixture.cs +++ b/src/NzbDrone.Core.Test/Datastore/ConnectionStringFactoryFixture.cs @@ -15,7 +15,7 @@ public void should_configure_sqlite_busy_timeout_for_one_second() var connection = Mocker.Resolve().MainDbConnection.ConnectionString; var builder = new SQLiteConnectionStringBuilder(connection); - builder.BusyTimeout.Should().Be(1000); + builder.BusyTimeout.Should().Be(5000); } } } diff --git a/src/NzbDrone.Core/Datastore/ConnectionStringFactory.cs b/src/NzbDrone.Core/Datastore/ConnectionStringFactory.cs index 72244099334..cafc2022c4f 100644 --- a/src/NzbDrone.Core/Datastore/ConnectionStringFactory.cs +++ b/src/NzbDrone.Core/Datastore/ConnectionStringFactory.cs @@ -49,7 +49,7 @@ private static DatabaseConnectionInfo GetConnectionString(string dbPath) JournalMode = OsInfo.IsOsx ? SQLiteJournalModeEnum.Truncate : SQLiteJournalModeEnum.Wal, Pooling = true, Version = 3, - BusyTimeout = 1000 + BusyTimeout = 5000 }; if (OsInfo.IsOsx) From d812646a3dd6fe0c3b1f6070f4c6f5649b0a69bd Mon Sep 17 00:00:00 2001 From: johoja12 <223961+johoja12@users.noreply.github.com> Date: Tue, 18 Aug 2026 10:51:29 +0000 Subject: [PATCH 12/12] fix: allow import sync alongside long-running commands --- .../ImportListSyncCommandFixture.cs | 19 +++++++++++++++++++ .../ImportLists/ImportListSyncCommand.cs | 2 -- 2 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 src/NzbDrone.Core.Test/ImportListTests/ImportListSyncCommandFixture.cs diff --git a/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncCommandFixture.cs b/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncCommandFixture.cs new file mode 100644 index 00000000000..619a05532eb --- /dev/null +++ b/src/NzbDrone.Core.Test/ImportListTests/ImportListSyncCommandFixture.cs @@ -0,0 +1,19 @@ +using FluentAssertions; +using NUnit.Framework; +using NzbDrone.Core.ImportLists; + +namespace NzbDrone.Core.Test.ImportListTests +{ + [TestFixture] + public class ImportListSyncCommandFixture + { + [Test] + public void should_be_type_exclusive_without_globally_blocking_commands() + { + var command = new ImportListSyncCommand(); + + command.IsExclusive.Should().BeFalse(); + command.IsTypeExclusive.Should().BeTrue(); + } + } +} diff --git a/src/NzbDrone.Core/ImportLists/ImportListSyncCommand.cs b/src/NzbDrone.Core/ImportLists/ImportListSyncCommand.cs index ab0c294e3cd..9051d205fba 100644 --- a/src/NzbDrone.Core/ImportLists/ImportListSyncCommand.cs +++ b/src/NzbDrone.Core/ImportLists/ImportListSyncCommand.cs @@ -17,8 +17,6 @@ public ImportListSyncCommand(int? definition) public override bool SendUpdatesToClient => true; - public override bool IsExclusive => true; - public override bool IsTypeExclusive => true; public override bool UpdateScheduledTask => !DefinitionId.HasValue;