From 1c77f69b0fea3a9379d107e883dc5798d908b116 Mon Sep 17 00:00:00 2001 From: jordan Date: Tue, 29 Sep 2026 19:13:22 +0000 Subject: [PATCH] perf(db): index MetadataFiles/ExtraFiles by (AuthorId, BookId) ExtraFileService reads (GetFilesByBook) and deletes (DeleteForBook) its rows with WHERE "AuthorId" = @a AND "BookId" = @b, and DeleteForBook runs once per deleted book for every ExtraFile type via BookDeletedEvent. Neither MetadataFiles nor ExtraFiles had an index on those columns (only the primary key, plus BookFileId on MetadataFiles), so each call was a sequential scan. On a live database (MetadataFiles ~69.5k rows) pg_stat_statements showed that DELETE at ~5 ms/call against ~0.05-0.1 ms for every other statement on the delete path, and 38% of all database execution time over a 20 s window of an author refresh that was pruning books. EXPLAIN ANALYZE on a copy: 4.2 ms Seq Scan -> 0.019 ms Index Scan (index 1.3 MB). Migration 110 adds IX_MetadataFiles_AuthorId_BookId and IX_ExtraFiles_AuthorId_BookId, guarded like 105: skipped when the table/columns are absent or the index already exists. Tests (SQLite): both indexes exist in query column order; the per-book delete is planned as an index search; missing tables/columns and a pre-existing index do not throw. Two of them fail if the index is not created. --- ...xtraFileAuthorBookIndexMigrationFixture.cs | 135 ++++++++++++++++++ .../110_add_extra_file_author_book_indexes.cs | 36 +++++ 2 files changed, 171 insertions(+) create mode 100644 src/Chaptarr.Core.Test/Datastore/ExtraFileAuthorBookIndexMigrationFixture.cs create mode 100644 src/NzbDrone.Core/Datastore/Migration/110_add_extra_file_author_book_indexes.cs diff --git a/src/Chaptarr.Core.Test/Datastore/ExtraFileAuthorBookIndexMigrationFixture.cs b/src/Chaptarr.Core.Test/Datastore/ExtraFileAuthorBookIndexMigrationFixture.cs new file mode 100644 index 00000000..b4dddcb2 --- /dev/null +++ b/src/Chaptarr.Core.Test/Datastore/ExtraFileAuthorBookIndexMigrationFixture.cs @@ -0,0 +1,135 @@ +using System; +using System.IO; +using System.Linq; +using Dapper; +using Microsoft.Data.Sqlite; +using NLog; +using NUnit.Framework; +using NzbDrone.Core.Datastore; +using NzbDrone.Core.Datastore.Migration.Framework; + +namespace Chaptarr.Core.Test.Datastore +{ + [TestFixture] + public class ExtraFileAuthorBookIndexMigrationFixture + { + private string _databasePath; + private string _connectionString; + private SqliteConnection _connection; + + [SetUp] + public void SetUp() + { + _databasePath = Path.Combine(TestContext.CurrentContext.WorkDirectory, $"extra_file_index_{Guid.NewGuid():N}.db"); + _connectionString = new SqliteConnectionStringBuilder + { + DataSource = _databasePath, + Mode = SqliteOpenMode.ReadWriteCreate, + Pooling = false + }.ToString(); + + _connection = new SqliteConnection(_connectionString); + _connection.Open(); + _connection.Execute(@" + CREATE TABLE ""VersionInfo"" ( + ""Version"" INTEGER PRIMARY KEY, + ""AppliedOn"" TEXT NULL, + ""Description"" TEXT NULL + ); + WITH RECURSIVE versions(version) AS + ( + SELECT 1 + UNION ALL + SELECT version + 1 FROM versions WHERE version < 109 + ) + INSERT INTO ""VersionInfo"" (""Version"", ""AppliedOn"", ""Description"") + SELECT version, CURRENT_TIMESTAMP, 'test baseline' FROM versions; + "); + } + + [TearDown] + public void TearDown() + { + _connection?.Dispose(); + SqliteConnection.ClearAllPools(); + + if (File.Exists(_databasePath)) + { + File.Delete(_databasePath); + } + } + + private void Migrate() + { + var migrationController = new MigrationController(LogManager.GetLogger("ExtraFileAuthorBookIndexMigrationFixture"), null); + migrationController.Migrate(_connectionString, new MigrationContext(MigrationType.Main, 110), DatabaseType.SQLite); + } + + private string[] IndexColumns(string tableName, string indexName) + { + var exists = _connection.QuerySingle($"SELECT COUNT(*) FROM pragma_index_list('{tableName}') WHERE name = '{indexName}';"); + return exists == 0 + ? Array.Empty() + : _connection.Query($"SELECT name FROM pragma_index_info('{indexName}') ORDER BY seqno;").ToArray(); + } + + [Test] + public void should_index_author_and_book_on_both_extra_file_tables_in_query_order() + { + _connection.Execute(@" + CREATE TABLE ""MetadataFiles"" (""Id"" INTEGER PRIMARY KEY, ""AuthorId"" INTEGER NOT NULL, ""BookId"" INTEGER NULL); + CREATE TABLE ""ExtraFiles"" (""Id"" INTEGER PRIMARY KEY, ""AuthorId"" INTEGER NOT NULL, ""BookId"" INTEGER NULL);"); + + Migrate(); + + Assert.Multiple(() => + { + Assert.That(IndexColumns("MetadataFiles", "IX_MetadataFiles_AuthorId_BookId"), Is.EqualTo(new[] { "AuthorId", "BookId" })); + Assert.That(IndexColumns("ExtraFiles", "IX_ExtraFiles_AuthorId_BookId"), Is.EqualTo(new[] { "AuthorId", "BookId" })); + }); + } + + [Test] + public void should_make_the_per_book_delete_use_the_index_instead_of_scanning() + { + _connection.Execute(@"CREATE TABLE ""MetadataFiles"" (""Id"" INTEGER PRIMARY KEY, ""AuthorId"" INTEGER NOT NULL, ""BookId"" INTEGER NULL);"); + + Migrate(); + + Assert.That(IndexColumns("MetadataFiles", "IX_MetadataFiles_AuthorId_BookId"), Is.EqualTo(new[] { "AuthorId", "BookId" })); + + // A fresh connection: the migration ran on its own connection, and a connection opened before + // it keeps planning against the schema it saw then. + using var fresh = new SqliteConnection(_connectionString); + fresh.Open(); + + // EXPLAIN QUERY PLAN returns (id, parent, notused, detail); the detail column names the access path. + var plan = string.Join(" | ", fresh.Query( + @"EXPLAIN QUERY PLAN DELETE FROM ""MetadataFiles"" WHERE ""AuthorId"" = 1 AND ""BookId"" = 2;") + .Select(row => (string)row.detail)); + + Assert.That(plan, Does.Contain("IX_MetadataFiles_AuthorId_BookId"), "the delete must be planned as an index search, not a table scan"); + } + + [Test] + public void should_tolerate_missing_tables_and_missing_columns() + { + // ExtraFiles present but without the columns (older/odd schema): nothing to index, must not throw. + _connection.Execute(@"CREATE TABLE ""ExtraFiles"" (""Id"" INTEGER PRIMARY KEY, ""Path"" TEXT NULL);"); + + Assert.DoesNotThrow(Migrate); + Assert.That(IndexColumns("ExtraFiles", "IX_ExtraFiles_AuthorId_BookId"), Is.Empty); + } + + [Test] + public void should_not_fail_when_the_index_already_exists() + { + _connection.Execute(@" + CREATE TABLE ""MetadataFiles"" (""Id"" INTEGER PRIMARY KEY, ""AuthorId"" INTEGER NOT NULL, ""BookId"" INTEGER NULL); + CREATE INDEX ""IX_MetadataFiles_AuthorId_BookId"" ON ""MetadataFiles"" (""AuthorId"", ""BookId"");"); + + Assert.DoesNotThrow(Migrate); + Assert.That(IndexColumns("MetadataFiles", "IX_MetadataFiles_AuthorId_BookId"), Is.EqualTo(new[] { "AuthorId", "BookId" })); + } + } +} diff --git a/src/NzbDrone.Core/Datastore/Migration/110_add_extra_file_author_book_indexes.cs b/src/NzbDrone.Core/Datastore/Migration/110_add_extra_file_author_book_indexes.cs new file mode 100644 index 00000000..7042a6de --- /dev/null +++ b/src/NzbDrone.Core/Datastore/Migration/110_add_extra_file_author_book_indexes.cs @@ -0,0 +1,36 @@ +using FluentMigrator; +using NzbDrone.Core.Datastore.Migration.Framework; + +namespace NzbDrone.Core.Datastore.Migration +{ + // ExtraFileService looks its rows up (GetFilesByBook) and deletes them (DeleteForBook, run once per + // deleted book by every BookDeletedEvent) with WHERE "AuthorId" = @a AND "BookId" = @b. Neither table + // had an index on those columns, so each lookup/delete was a sequential scan: ~5 ms against + // MetadataFiles at ~70k rows, which made it the single most expensive statement of an author refresh + // that prunes books (a book deletion costs ~0.1 ms everywhere else on the delete path). + [Migration(110)] + public class add_extra_file_author_book_indexes : NzbDroneMigrationBase + { + protected override void MainDbUpgrade() + { + AddIndex("MetadataFiles", "IX_MetadataFiles_AuthorId_BookId"); + AddIndex("ExtraFiles", "IX_ExtraFiles_AuthorId_BookId"); + } + + private void AddIndex(string tableName, string indexName) + { + if (!Schema.Table(tableName).Exists() || + !Schema.Table(tableName).Column("AuthorId").Exists() || + !Schema.Table(tableName).Column("BookId").Exists() || + Schema.Table(tableName).Index(indexName).Exists()) + { + return; + } + + Create.Index(indexName) + .OnTable(tableName) + .OnColumn("AuthorId").Ascending() + .OnColumn("BookId").Ascending(); + } + } +}