Merge pull request #17455 from Shadowghost/fix-migrations

Reevaluate pending migrations after each one instead of per stage

1 files changed, 73 insertions, 55 deletions

diff --git a/Jellyfin.Server/Migrations/JellyfinMigrationService.cs b/Jellyfin.Server/Migrations/JellyfinMigrationService.cs
index a10be76e05..beafc3916f 100644
--- a/Jellyfin.Server/Migrations/JellyfinMigrationService.cs
+++ b/Jellyfin.Server/Migrations/JellyfinMigrationService.cs
@@ -193,10 +193,15 @@ internal class JellyfinMigrationService
         {
             var historyRepository = dbContext.GetService<IHistoryRepository>();
             var migrationsAssembly = dbContext.GetService<IMigrationsAssembly>();
-            (string Key, IInternalMigration Migration)[] migrations = [];
+            var completedMigrations = 0;
+            string? lastMigrationKey = null;
 
-            do
-            { // migrations may alter the migration state. Reevaluate the applicable migrations after every stage ran until there are no more to apply.
+            while (true)
+            {
+                // A single migration can change which migrations still apply: IMigrator.MigrateAsync treats its argument as the
+                // state to end up in, so it reverts everything applied after it, and a reverted migration can take code migrations
+                // with it (AddNormalizedUsername.Down drops the UpdateNormalizedUsername history row). Anything computed before
+                // that point is stale, so only ever run the next migration and then work out the pending set again.
                 var appliedMigrations = await historyRepository.GetAppliedMigrationsAsync().ConfigureAwait(false);
                 var pendingCodeMigrations = migrationStage
                     .Where(e => appliedMigrations.All(f => f.MigrationId != e.BuildCodeMigrationId()))
@@ -212,73 +217,86 @@ internal class JellyfinMigrationService
                 }
 
                 (string Key, IInternalMigration Migration)[] pendingMigrations = [.. pendingCodeMigrations, .. pendingDatabaseMigrations];
-                logger.LogInformation("There are {Pending} migrations for stage {Stage}.", pendingCodeMigrations.Length, stage);
-                migrations = pendingMigrations.OrderBy(e => e.Key).ToArray();
+                if (pendingMigrations.Length == 0)
+                {
+                    break;
+                }
 
-                var migrationIndex = 0;
-                foreach (var item in migrations)
+                if (completedMigrations == 0)
                 {
-                    // Surface generic "Running migration X of Y" progress in the always-visible startup UI header.
-                    SetupServer.ReportActivity(StartupActivity.Migration(++migrationIndex, migrations.Length));
-                    var migrationLogger = logger.With(_loggerFactory.CreateLogger(item.Migration.GetType().Name)).BeginGroup($"{item.Key}");
-                    try
-                    {
-                        migrationLogger.LogInformation("Perform migration {Name}", item.Key);
-                        await item.Migration.PerformAsync(migrationLogger).ConfigureAwait(false);
-                        migrationLogger.LogInformation("Migration {Name} was successfully applied", item.Key);
-                    }
-                    catch (Exception ex)
-                    {
-                        migrationLogger.LogCritical("Error: {Error}", ex.Message);
-                        migrationLogger.LogError(ex, "Migration {Name} failed", item.Key);
+                    logger.LogInformation("There are {Pending} migrations for stage {Stage}.", pendingMigrations.Length, stage);
+                }
+
+                var item = pendingMigrations.OrderBy(e => e.Key, StringComparer.Ordinal).First();
+                if (string.Equals(item.Key, lastMigrationKey, StringComparison.Ordinal))
+                {
+                    throw new InvalidOperationException($"Migration {item.Key} ran but did not record itself as applied and would repeat indefinitely.");
+                }
+
+                lastMigrationKey = item.Key;
+
+                // Surface generic "Running migration X of Y" progress in the always-visible startup UI header.
+                SetupServer.ReportActivity(StartupActivity.Migration(completedMigrations + 1, completedMigrations + pendingMigrations.Length));
+                var migrationLogger = logger.With(_loggerFactory.CreateLogger(item.Migration.GetType().Name)).BeginGroup($"{item.Key}");
+                try
+                {
+                    migrationLogger.LogInformation("Perform migration {Name}", item.Key);
+                    await item.Migration.PerformAsync(migrationLogger).ConfigureAwait(false);
+                    migrationLogger.LogInformation("Migration {Name} was successfully applied", item.Key);
+                }
+                catch (Exception ex)
+                {
+                    migrationLogger.LogCritical("Error: {Error}", ex.Message);
+                    migrationLogger.LogError(ex, "Migration {Name} failed", item.Key);
 
-                        if (_backupKey != default && _backupService is not null && _jellyfinDatabaseProvider is not null)
+                    if (_backupKey != default && _backupService is not null && _jellyfinDatabaseProvider is not null)
+                    {
+                        if (_backupKey.LibraryDb is not null)
                         {
-                            if (_backupKey.LibraryDb is not null)
+                            migrationLogger.LogInformation("Attempt to rollback librarydb.");
+                            try
                             {
-                                migrationLogger.LogInformation("Attempt to rollback librarydb.");
-                                try
-                                {
-                                    var libraryDbPath = Path.Combine(_applicationPaths.DataPath, DbFilename);
-                                    File.Move(_backupKey.LibraryDb, libraryDbPath, true);
-                                }
-                                catch (Exception inner)
-                                {
-                                    migrationLogger.LogCritical(inner, "Could not rollback {LibraryPath}. Manual intervention might be required to restore a operational state.", _backupKey.LibraryDb);
-                                }
+                                var libraryDbPath = Path.Combine(_applicationPaths.DataPath, DbFilename);
+                                File.Move(_backupKey.LibraryDb, libraryDbPath, true);
                             }
-
-                            if (_backupKey.JellyfinDb is not null)
+                            catch (Exception inner)
                             {
-                                migrationLogger.LogInformation("Attempt to rollback JellyfinDb.");
-                                try
-                                {
-                                    await _jellyfinDatabaseProvider.RestoreBackupFast(_backupKey.JellyfinDb, CancellationToken.None).ConfigureAwait(false);
-                                }
-                                catch (Exception inner)
-                                {
-                                    migrationLogger.LogCritical(inner, "Could not rollback {LibraryPath}. Manual intervention might be required to restore a operational state.", _backupKey.JellyfinDb);
-                                }
+                                migrationLogger.LogCritical(inner, "Could not rollback {LibraryPath}. Manual intervention might be required to restore a operational state.", _backupKey.LibraryDb);
                             }
+                        }
 
-                            if (_backupKey.FullBackup is not null)
+                        if (_backupKey.JellyfinDb is not null)
+                        {
+                            migrationLogger.LogInformation("Attempt to rollback JellyfinDb.");
+                            try
                             {
-                                migrationLogger.LogInformation("Attempt to rollback from backup.");
-                                try
-                                {
-                                    await _backupService.RestoreBackupAsync(_backupKey.FullBackup.Path).ConfigureAwait(false);
-                                }
-                                catch (Exception inner)
-                                {
-                                    migrationLogger.LogCritical(inner, "Could not rollback from backup {Backup}. Manual intervention might be required to restore a operational state.", _backupKey.FullBackup.Path);
-                                }
+                                await _jellyfinDatabaseProvider.RestoreBackupFast(_backupKey.JellyfinDb, CancellationToken.None).ConfigureAwait(false);
+                            }
+                            catch (Exception inner)
+                            {
+                                migrationLogger.LogCritical(inner, "Could not rollback {LibraryPath}. Manual intervention might be required to restore a operational state.", _backupKey.JellyfinDb);
                             }
                         }
 
-                        throw;
+                        if (_backupKey.FullBackup is not null)
+                        {
+                            migrationLogger.LogInformation("Attempt to rollback from backup.");
+                            try
+                            {
+                                await _backupService.RestoreBackupAsync(_backupKey.FullBackup.Path).ConfigureAwait(false);
+                            }
+                            catch (Exception inner)
+                            {
+                                migrationLogger.LogCritical(inner, "Could not rollback from backup {Backup}. Manual intervention might be required to restore a operational state.", _backupKey.FullBackup.Path);
+                            }
+                        }
                     }
+
+                    throw;
                 }
-            } while (migrations.Length != 0);
+
+                completedMigrations++;
+            }
         }
     }