Fix: skip SQLite backup requirements for non-SQLite DBs

Previously, the migration system would attempt to back up the legacy SQLite library.db file even when running with non-SQLite providers (e.g., PostgreSQL), causing startup errors. This was due to SQLite-specific migrations not being filtered out during backup requirement aggregation. This commit adds filtering to exclude migrations marked as RequiresSqlite from backup processing unless the provider is actually SQLite (currently unsupported). This prevents erroneous backup attempts and ensures only relevant migrations and backups are processed. Documentation has been updated to explain the issue and solution.
2026-03-02 10:12:20 -05:00
parent 623af06e46
commit 163a037642
2 changed files with 164 additions and 0 deletions
@@ -397,8 +397,13 @@ internal class JellyfinMigrationService
            };
        }

+        // Filter out SQLite-specific migrations when determining backup requirements
+        // SQLite is no longer supported as a runtime provider
+        bool isSqliteProvider = false;
+
        backupInstruction = Migrations.SelectMany(e => e)
           .Where(e => appliedMigrations.All(f => f.MigrationId != e.BuildCodeMigrationId()))
+           .Where(e => isSqliteProvider || !e.Metadata.RequiresSqlite)  // Skip SQLite migrations for non-SQLite providers
           .Select(e => e.BackupRequirements)
           .Where(e => e is not null)
           .Aggregate(backupInstruction, MergeBackupAttributes!);
@@ -0,0 +1,159 @@
+# SQLite Migration Filtering Fix
+
+## Problem
+
+When running Jellyfin with PostgreSQL, the startup was failing with an error:
+
+```
+[ERR] Main: Cannot make a backup of "library.db" at path "/var/lib/jellyfin/data/library.db" 
+because file could not be found at "/var/lib/jellyfin/data"
+```
+
+This occurred even though:
+1. The migration system was using PostgreSQL (not SQLite)
+2. SQLite-specific migrations were marked with `RequiresSqlite = true`
+3. SQLite migrations were supposed to be skipped
+
+## Root Cause
+
+The issue was in the **backup preparation phase** of the migration system (`JellyfinMigrationService.cs`).
+
+### What Was Happening
+
+1. **Migration Execution** - SQLite-specific migrations were correctly filtered out (lines 222-233)
+   - ✅ This worked correctly
+
+2. **Backup Preparation** - ALL migrations were included when determining backup requirements (line 400-404)
+   - ❌ This included SQLite-specific migrations
+   - ❌ Their backup requirements were aggregated even though the migrations wouldn't run
+   - ❌ System tried to backup `library.db` that doesn't exist in PostgreSQL setup
+
+### Code Flow
+
+```csharp
+// Line 400-404 - BEFORE FIX
+backupInstruction = Migrations.SelectMany(e => e)
+   .Where(e => appliedMigrations.All(f => f.MigrationId != e.BuildCodeMigrationId()))
+   .Select(e => e.BackupRequirements)  // ← Includes SQLite migration requirements!
+   .Where(e => e is not null)
+   .Aggregate(backupInstruction, MergeBackupAttributes!);
+
+// Line 406 - Then tries to backup based on aggregated requirements
+if (backupInstruction.LegacyLibraryDb)  // ← True because SQLite migration requested it
+{
+    // Tries to backup library.db even though we're using PostgreSQL!
+    var libraryDbPath = Path.Combine(_applicationPaths.DataPath, DbFilename);
+    if (File.Exists(libraryDbPath))  // ← File doesn't exist → ERROR
+```
+
+## Solution
+
+Added filtering to skip SQLite-specific migrations when aggregating backup requirements:
+
+```csharp
+// Filter out SQLite-specific migrations when determining backup requirements
+bool isSqliteProvider = false; // SQLite is no longer supported as runtime provider
+
+backupInstruction = Migrations.SelectMany(e => e)
+   .Where(e => appliedMigrations.All(f => f.MigrationId != e.BuildCodeMigrationId()))
+   .Where(e => isSqliteProvider || !e.Metadata.RequiresSqlite)  // ← NEW: Skip SQLite migrations
+   .Select(e => e.BackupRequirements)
+   .Where(e => e is not null)
+   .Aggregate(backupInstruction, MergeBackupAttributes!);
+```
+
+### What This Fixes
+
+1. ✅ SQLite-specific migrations are filtered out BEFORE aggregating backup requirements
+2. ✅ System no longer tries to backup `library.db` when using PostgreSQL
+3. ✅ Startup completes successfully without SQLite file errors
+4. ✅ Maintains compatibility if SQLite provider is ever re-enabled (checks `isSqliteProvider`)
+
+## Impact
+
+### Before Fix
+- ❌ Jellyfin startup failed when using PostgreSQL
+- ❌ Error: "Cannot make a backup of library.db"
+- ❌ Migration system tried to backup non-existent SQLite files
+
+### After Fix
+- ✅ Jellyfin starts successfully with PostgreSQL
+- ✅ SQLite-specific migrations are completely skipped (both execution AND backup)
+- ✅ Only relevant migrations run and their files are backed up
+
+## Testing
+
+To verify the fix works:
+
+1. **Start Jellyfin with PostgreSQL** - Should complete startup without errors
+2. **Check logs** - Should see:
+   ```
+   [INF] Skipping X SQLite-specific migrations because current provider is not SQLite
+   ```
+3. **No backup errors** - Should NOT see:
+   ```
+   [ERR] Cannot make a backup of "library.db"
+   ```
+
+## Related Files
+
+- **`Jellyfin.Server/Migrations/JellyfinMigrationService.cs`** (Line 400-407)
+  - Modified to filter SQLite migrations before aggregating backup requirements
+
+- **`Jellyfin.Server/Migrations/Routines/ReseedFolderFlag.cs`**
+  - Example SQLite-specific migration with `RequiresSqlite = true`
+  - Has `JellyfinMigrationBackupAttribute(JellyfinDb = true)` which was causing backup attempt
+
+## Migration Attributes
+
+### RequiresSqlite
+
+```csharp
+[JellyfinMigration("2025-07-30T21:50:00", nameof(ReseedFolderFlag), RequiresSqlite = true)]
+public class ReseedFolderFlag : IAsyncMigrationRoutine
+```
+
+**Purpose:** Marks migrations that require SQLite's `library.db` file
+
+**Usage:** Set to `true` for legacy migrations that:
+- Read from old SQLite `library.db.old` files
+- Require SQLite-specific features
+- Should only run when SQLite is the active database provider
+
+### Backup Filtering
+
+The fix ensures that `RequiresSqlite` is respected in TWO places:
+
+1. **Migration Execution** (already working)
+   ```csharp
+   migrationStage = migrationStage.Where(m => !m.Metadata.RequiresSqlite).ToList();
+   ```
+
+2. **Backup Preparation** (NOW FIXED)
+   ```csharp
+   .Where(e => isSqliteProvider || !e.Metadata.RequiresSqlite)
+   ```
+
+## Background: SQLite Removal
+
+Jellyfin is migrating away from SQLite to support multiple database providers (PostgreSQL, SQL Server, etc.):
+
+1. **Runtime Support** - SQLite is no longer a supported runtime database provider
+2. **Migration Support** - SQLite migration code remains for users upgrading from old versions
+3. **Legacy File Support** - Some migrations read old `library.db` files to migrate data
+4. **Flag Value** - `isSqliteProvider` is hardcoded to `false` (line 224)
+
+This fix ensures the migration system properly handles this transition state where:
+- SQLite migrations exist for backward compatibility
+- But the runtime only supports PostgreSQL/SQL Server
+- Backup system shouldn't try to backup files that don't exist
+
+## Future Considerations
+
+If SQLite runtime support is ever re-added:
+
+1. Set `isSqliteProvider = true` based on actual database provider
+2. SQLite migrations will run again
+3. `library.db` backups will be attempted (and should exist)
+
+The current implementation is ready for this - it just needs the provider detection logic updated.