From ad9aabe46cbe2257ce4cc3a5e038cd0e7079a322 Mon Sep 17 00:00:00 2001 From: Leonid Tsarev Date: Fri, 9 Aug 2024 17:21:30 +0300 Subject: [PATCH] Fix race condition in migrator (#2730) --- .../{ => Ef6}/JoinMigrationsConfig.cs | 2 +- .../MigrateMyDbContextService.cs} | 14 ++--- .../MigrationsLoggerILoggerAdapter.cs | 2 +- .../EfCore/EfCoreMigrationExtensions.cs | 9 +-- .../EfCore/MigrateEfCoreHostService.cs | 56 ++++++------------- src/Joinrpg.Dal.Migrate/IMigratorService.cs | 5 ++ .../OneTimeOperationHostedServiceBase.cs | 25 ++++----- src/Joinrpg.Dal.Migrate/Program.cs | 5 +- 8 files changed, 52 insertions(+), 66 deletions(-) rename src/Joinrpg.Dal.Migrate/{ => Ef6}/JoinMigrationsConfig.cs (93%) rename src/Joinrpg.Dal.Migrate/{MigrateHostService.cs => Ef6/MigrateMyDbContextService.cs} (82%) rename src/Joinrpg.Dal.Migrate/{ => Ef6}/MigrationsLoggerILoggerAdapter.cs (93%) create mode 100644 src/Joinrpg.Dal.Migrate/IMigratorService.cs diff --git a/src/Joinrpg.Dal.Migrate/JoinMigrationsConfig.cs b/src/Joinrpg.Dal.Migrate/Ef6/JoinMigrationsConfig.cs similarity index 93% rename from src/Joinrpg.Dal.Migrate/JoinMigrationsConfig.cs rename to src/Joinrpg.Dal.Migrate/Ef6/JoinMigrationsConfig.cs index 3051c749f..9116dcd4b 100644 --- a/src/Joinrpg.Dal.Migrate/JoinMigrationsConfig.cs +++ b/src/Joinrpg.Dal.Migrate/Ef6/JoinMigrationsConfig.cs @@ -1,7 +1,7 @@ using System.Data.Entity.Migrations; using JoinRpg.Dal.Impl; -namespace Joinrpg.Dal.Migrate; +namespace Joinrpg.Dal.Migrate.Ef6; internal class JoinMigrationsConfig : DbMigrationsConfiguration { diff --git a/src/Joinrpg.Dal.Migrate/MigrateHostService.cs b/src/Joinrpg.Dal.Migrate/Ef6/MigrateMyDbContextService.cs similarity index 82% rename from src/Joinrpg.Dal.Migrate/MigrateHostService.cs rename to src/Joinrpg.Dal.Migrate/Ef6/MigrateMyDbContextService.cs index 95a69c2e3..431687adb 100644 --- a/src/Joinrpg.Dal.Migrate/MigrateHostService.cs +++ b/src/Joinrpg.Dal.Migrate/Ef6/MigrateMyDbContextService.cs @@ -1,18 +1,16 @@ using System.Data.Entity.Migrations; using System.Data.Entity.Migrations.Infrastructure; using Microsoft.Extensions.Configuration; -using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; -namespace Joinrpg.Dal.Migrate; +namespace Joinrpg.Dal.Migrate.Ef6; -internal class MigrateHostService( - IHostApplicationLifetime applicationLifetime, - ILogger logger, +internal class MigrateMyDbContextService( + ILogger logger, IConfiguration configuration) - : OneTimeOperationHostedServiceBase(applicationLifetime, logger) + : IMigratorService { - internal override void DoWork() + public Task MigrateAsync(CancellationToken ct) { logger.LogInformation("Create migration"); @@ -33,5 +31,7 @@ internal override void DoWork() logger.LogInformation("Pending migrations {pending}", string.Join("\n", pending)); migrator.Update(); // TODO pass migration name from command line to allow reverts logger.LogInformation("Migration completed"); + + return Task.CompletedTask; } } diff --git a/src/Joinrpg.Dal.Migrate/MigrationsLoggerILoggerAdapter.cs b/src/Joinrpg.Dal.Migrate/Ef6/MigrationsLoggerILoggerAdapter.cs similarity index 93% rename from src/Joinrpg.Dal.Migrate/MigrationsLoggerILoggerAdapter.cs rename to src/Joinrpg.Dal.Migrate/Ef6/MigrationsLoggerILoggerAdapter.cs index 3b954040e..9e81bb6f7 100644 --- a/src/Joinrpg.Dal.Migrate/MigrationsLoggerILoggerAdapter.cs +++ b/src/Joinrpg.Dal.Migrate/Ef6/MigrationsLoggerILoggerAdapter.cs @@ -1,7 +1,7 @@ using System.Data.Entity.Migrations.Infrastructure; using Microsoft.Extensions.Logging; -namespace Joinrpg.Dal.Migrate; +namespace Joinrpg.Dal.Migrate.Ef6; internal class MigrationsLoggerILoggerAdapter : MigrationsLogger { diff --git a/src/Joinrpg.Dal.Migrate/EfCore/EfCoreMigrationExtensions.cs b/src/Joinrpg.Dal.Migrate/EfCore/EfCoreMigrationExtensions.cs index ff5312dd4..298d9f0ee 100644 --- a/src/Joinrpg.Dal.Migrate/EfCore/EfCoreMigrationExtensions.cs +++ b/src/Joinrpg.Dal.Migrate/EfCore/EfCoreMigrationExtensions.cs @@ -7,12 +7,13 @@ internal static class EfCoreMigrationExtensions internal static void RegisterMigrator(this IServiceCollection services, string connectionString) where TContext : DbContext { - _ = services.AddHostedService>(); + _ = services.AddScoped>(); _ = services.AddDbContext(options => { - options.UseNpgsql(connectionString); - options.EnableSensitiveDataLogging(false); // Logs of migration is publicly accessible - options.EnableDetailedErrors(true); // This will be helpful + _ = options + .UseNpgsql(connectionString) + .EnableSensitiveDataLogging(false) // Logs of migration is publicly accessible + .EnableDetailedErrors(true); // This will be helpful }); } } diff --git a/src/Joinrpg.Dal.Migrate/EfCore/MigrateEfCoreHostService.cs b/src/Joinrpg.Dal.Migrate/EfCore/MigrateEfCoreHostService.cs index 8baeb7cee..6ec1f2cba 100644 --- a/src/Joinrpg.Dal.Migrate/EfCore/MigrateEfCoreHostService.cs +++ b/src/Joinrpg.Dal.Migrate/EfCore/MigrateEfCoreHostService.cs @@ -1,24 +1,24 @@ using Microsoft.EntityFrameworkCore; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; namespace Joinrpg.Dal.Migrate; internal class MigrateEfCoreHostService( - IServiceProvider services, - ILogger> logger, - IHostApplicationLifetime applicationLifetime) : BackgroundService + TContext dbContext, + ILogger> logger) : IMigratorService where TContext : DbContext { - private async Task MigrateAsync(DbContext dbContext, CancellationToken stoppingToken) + private static readonly string contextName = typeof(TContext).Name!; + public async Task MigrateAsync(CancellationToken stoppingToken) { - logger.LogInformation("Start migration of {contextName}", typeof(TContext).FullName); + using var logScope = logger.BeginScope("Migration of {dbContext}", contextName); + + logger.LogInformation("Start migration of {dbContext}", contextName); var lastAppliedMigration = (await dbContext.Database.GetAppliedMigrationsAsync(stoppingToken)).LastOrDefault(); if (!string.IsNullOrEmpty(lastAppliedMigration)) { - logger.LogInformation("Last applied migration: {LastAppliedMigration}", lastAppliedMigration); + logger.LogInformation("Last applied migration for {dbContext}: {LastAppliedMigration}", contextName, lastAppliedMigration); } if (stoppingToken.IsCancellationRequested) @@ -26,10 +26,17 @@ private async Task MigrateAsync(DbContext dbContext, CancellationToken stoppingT return; } + if (dbContext.Database.HasPendingModelChanges()) + { + logger.LogError("There is pending changes in model!"); + throw new InvalidOperationException("Pending changes in model"); + } + var pendingMigrations = await dbContext.Database.GetPendingMigrationsAsync(stoppingToken); + foreach (var pm in pendingMigrations) { - logger.LogInformation("Pending migration: {PendingMigration}", pm); + logger.LogInformation("Pending migration for {dbContext}: {PendingMigration}", contextName, pm); } if (stoppingToken.IsCancellationRequested) @@ -37,35 +44,8 @@ private async Task MigrateAsync(DbContext dbContext, CancellationToken stoppingT return; } - logger.LogInformation("Applying migrations..."); + logger.LogInformation("Applying migrations for {dbContext} ...", contextName); await dbContext.Database.MigrateAsync(stoppingToken); - logger.LogInformation("Database has been successfully migrated"); - } - - /// - protected override async Task ExecuteAsync(CancellationToken stoppingToken) - { - logger.LogInformation("Starting migrator..."); - await using var scope = services.CreateAsyncScope(); - try - { - await MigrateAsync( - scope.ServiceProvider.GetRequiredService(), - stoppingToken); - - if (stoppingToken.IsCancellationRequested) - { - logger.LogInformation("Terminating by cancellation token"); - } - } - catch (Exception ex) - { - logger.LogError(ex, "Error executing migrator"); - Environment.ExitCode = 1; - } - finally - { - applicationLifetime.StopApplication(); - } + logger.LogInformation("Database {dbContext} has been successfully migrated", contextName); } } diff --git a/src/Joinrpg.Dal.Migrate/IMigratorService.cs b/src/Joinrpg.Dal.Migrate/IMigratorService.cs new file mode 100644 index 000000000..a9a661db2 --- /dev/null +++ b/src/Joinrpg.Dal.Migrate/IMigratorService.cs @@ -0,0 +1,5 @@ +namespace Joinrpg.Dal.Migrate; +internal interface IMigratorService +{ + internal abstract Task MigrateAsync(CancellationToken ct); +} diff --git a/src/Joinrpg.Dal.Migrate/OneTimeOperationHostedServiceBase.cs b/src/Joinrpg.Dal.Migrate/OneTimeOperationHostedServiceBase.cs index 97cdcf469..680a7f9c0 100644 --- a/src/Joinrpg.Dal.Migrate/OneTimeOperationHostedServiceBase.cs +++ b/src/Joinrpg.Dal.Migrate/OneTimeOperationHostedServiceBase.cs @@ -3,28 +3,27 @@ namespace Joinrpg.Dal.Migrate; -internal abstract class OneTimeOperationHostedServiceBase : IHostedService +internal class OneTimeOperationHostedServiceBase( + IHostApplicationLifetime applicationLifetime, + ILogger logger, + IEnumerable migrators) : IHostedService { private CancellationTokenSource CancellationTokenSource { get; } = new CancellationTokenSource(); private Task? task; - private readonly IHostApplicationLifetime applicationLifetime; - protected readonly ILogger logger; - - public OneTimeOperationHostedServiceBase(IHostApplicationLifetime applicationLifetime, ILogger logger) - { - this.applicationLifetime = applicationLifetime; - this.logger = logger; - } Task IHostedService.StartAsync(CancellationToken cancellationToken) { logger.LogInformation("Starting task"); - task = Task.Run(() => + task = Task.Run(async () => { try { - DoWork(); + foreach (var migrator in migrators) + { + using var scope = logger.BeginScope("Migration of {migrator}", migrator.GetType().Name); + await migrator.MigrateAsync(cancellationToken); + } } catch (Exception ex) { @@ -35,12 +34,10 @@ Task IHostedService.StartAsync(CancellationToken cancellationToken) { applicationLifetime.StopApplication(); } - }, CancellationToken.None); + }, cancellationToken); return Task.CompletedTask; } - internal abstract void DoWork(); - Task IHostedService.StopAsync(CancellationToken cancellationToken) { CancellationTokenSource.Cancel(); diff --git a/src/Joinrpg.Dal.Migrate/Program.cs b/src/Joinrpg.Dal.Migrate/Program.cs index d49823ae6..34f44971e 100644 --- a/src/Joinrpg.Dal.Migrate/Program.cs +++ b/src/Joinrpg.Dal.Migrate/Program.cs @@ -1,3 +1,4 @@ +using Joinrpg.Dal.Migrate.Ef6; using Joinrpg.Dal.Migrate.EfCore; using JoinRpg.Dal.JobService; using JoinRpg.Portal.Infrastructure; @@ -19,7 +20,9 @@ public static IHostBuilder CreateHostBuilder(string[] args) => Host.CreateDefaultBuilder(args) .ConfigureServices((hostContext, services) => { - _ = services.AddHostedService(); + _ = services.AddHostedService(); + + services.AddScoped(); services.RegisterMigrator(hostContext.Configuration.GetConnectionString("DataProtection")!); services.RegisterMigrator(hostContext.Configuration.GetConnectionString("DailyJob")!);