dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.63k stars 3.15k forks source link

Executing Migrator by having an ambient transaction #31507

Closed agustinsilvano closed 3 days ago

agustinsilvano commented 1 year ago

Executing Migrator by having an ambient transaction.

I've been working on a feature to implement an all-or-nothing approach by using the migrator.

Background

We have a multi-tenant application where each tenant has its own database per DbContext (about 5 tenants and 4 DbContexts each). We have a migrator project that calls the migrator for each DbContext, which causes if the migrations for tenant 1 are successfully applied but the migrations for tenant 2 fail, there is no option to roll back the already migrated databases of tenant 1.

What I did

I wrapped all the MigrateAsync calls into a global transaction, where the transaction is committed if everything succeed otherwise the transaction is rolled back in order to undo all the applied migrations.

This is my implementation:

public async Task MigrateAsync()
{
    using (TransactionScope globalScope = new(TransactionScopeAsyncFlowOption.Enabled))
    {
        var initialMigrationAdded = AddInitialMigrationIfNotExist();

        if (initialMigrationAdded)
        {
            return;
        }

        Logger.LogInformation("Started database migrations...");

        await MigrateDatabaseSchemaAsync();
        await SeedDataAsync();

        Logger.LogInformation($"Successfully completed host database migrations.");

        var tenants = await _tenantRepository.GetListAsync(includeDetails: true);

        var migratedDatabaseSchemas = new HashSet<string>();
        foreach (var tenant in tenants)
        {
            try
            {
                using (_currentTenant.Change(tenant.Id))
                {
                    if (tenant.ConnectionStrings.Any())
                    {
                        var tenantConnectionStrings = tenant.ConnectionStrings
                            .Select(x => x.Value)
                            .ToList();

                        if (!migratedDatabaseSchemas.IsSupersetOf(tenantConnectionStrings))
                        {
                            await MigrateDatabaseSchemaAsync(tenant);

                            migratedDatabaseSchemas.AddIfNotContains(tenantConnectionStrings);
                        }
                    }

                    await SeedDataAsync(tenant);
                }

                Logger.LogInformation($"Successfully completed {tenant.Name} tenant database migrations.");
            }
            catch (Exception ex)
            {
                // Handle migration failure
                Console.WriteLine($"Migration failed for {tenant.Name}. Rolling back all databases.");
                globalScope.Dispose();
                throw;
            }
        }

        Logger.LogInformation("Successfully completed all database migrations.");
        Logger.LogInformation("You can safely end this process...");
        globalScope.Complete();
    }
}

 private async Task MigrateDatabaseSchemaAsync(Tenant tenant = null)
 {
     Logger.LogInformation(
         $"Migrating schema for {(tenant == null ? "host" : tenant.Name + " tenant")} database...");

     foreach (var migrator in _dbSchemaMigrators)
     {
         await migrator.MigrateAsync();
     }
 }

This MigrateAsync() method call this method, where the MigrateAsync() of the Migrator of EntityFramework is being called:

public async Task MigrateAsync()
{
    await _serviceProvider
   .GetRequiredService<MigrationsDbContext>()
   .Database
   .MigrateAsync();
}

By having that, the first time that the MigrateAsync is called it returns an exception that says:

Error

Message: System.InvalidOperationException: 'This connection was used with an ambient transaction. The original ambient transaction needs to be completed before this connection can be used outside of it.'

image

Include provider and version information

EF Core version: Database provider: (Microsoft.EntityFrameworkCore.SqlServer) Target framework: (.NET 7.0) Operating system: Windows 11 KB5029263 (OS Build 22621.2134) IDE: Visual Studio 2022 17.7)

ajcvickers commented 1 year ago

@agustinsilvano What is your question here? The error message seems correct for the code you wrote; is there some confusion as to what it means?

agustinsilvano commented 1 year ago

@ajcvickers, sorry, my question is if that all-or-nothing approach can be implemented by using EF Core.

Is there a way to wrap the database connection into an ambient transaction?

bricelam commented 1 year ago

I thought we already enabled this scenario in #12325...

agustinsilvano commented 1 year ago

@bricelam it sounds like it was added in ef core 6, right?

bricelam commented 1 year ago

Yeah, it should be.

agustinsilvano commented 1 year ago

@bricelam I was checking the referenced issue, and it looks a bit different. He was trying to apply migrations as "all-or-nothing" within the same DbContext. For that reason his stack trace there is using the ExecuteNonQuery (the method that was fixed in the referenced PR).

The thing is that my call stack is not going through that method, there are 2 major differences between his code and mine:

  1. He was using the IDbContextTransaction (calling the BeginTransaction) while I'm using a TransactionScope.
  2. He was using the sync version of the Migrate method while I'm using the async MigrateAsync method.

My main goal is to have the capability to roll back migrations applied in different DbContexts (i.e. if migration in DbContext2 failed, the migrations applied in DbContext 1 must be rolled back).

Am I missing something?

AndriySvyryd commented 3 days ago

Duplicate of https://github.com/dotnet/efcore/issues/22616

We now execute all migrations in a single transaction when possible (most of the times). We don't plan on adding support for ambient transactions.

You can use a transaction that spans multiple contexts, but it would disable resiliency and concurrency control.