Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Executing Migrator by having an ambient transaction #31507

Closed
agustinsilvano opened this issue Aug 18, 2023 · 7 comments
Closed

Executing Migrator by having an ambient transaction #31507

agustinsilvano opened this issue Aug 18, 2023 · 7 comments

Comments

@agustinsilvano
Copy link

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
Copy link
Member

@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
Copy link
Author

agustinsilvano commented Aug 20, 2023

@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
Copy link
Contributor

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

@agustinsilvano
Copy link
Author

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

@bricelam
Copy link
Contributor

bricelam commented Sep 5, 2023

Yeah, it should be.

@agustinsilvano
Copy link
Author

@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
Copy link
Member

AndriySvyryd commented Sep 4, 2024

Duplicate of #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.

@AndriySvyryd AndriySvyryd closed this as not planned Won't fix, can't repro, duplicate, stale Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants