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

Add safeguard for MigrateAsync with targetMigration #34819

Open
Ben555555 opened this issue Oct 3, 2024 · 1 comment
Open

Add safeguard for MigrateAsync with targetMigration #34819

Ben555555 opened this issue Oct 3, 2024 · 1 comment

Comments

@Ben555555
Copy link

Ben555555 commented Oct 3, 2024

We are using the following code to run each migration one by one, so we can better determine which migration failed:

                var pendingMigrations = await dbContext.Database.GetPendingMigrationsAsync(cancellationToken);

                var migrator = dbContext.Database.GetInfrastructure().GetRequiredService<IMigrator>();

                foreach (string migration in pendingMigrations)
                {
                    logger.LogInformation($"Applying migration '{migration}'...");

                    await migrator.MigrateAsync(migration, cancellationToken);
                }

Recently when we deployed an application we noticed an unfamiliar behaviour. A field disappeared and was gone after the migrations were executed.
It turned out that for one migration, the Down-Migration was called, because that migration was already deployed and had a higher timestamp in the filename than the pending migrations. This happened because the migration was added on a branch that was going live, while on a dev branch already new migrations were added before.
So when the first pending migration was executed via IMigrator.MigrateAsync, the down migration of that already deployed migration was called. As far as I understand this is to make sure that the migrations always run on the "correct" state of the database.
In our case this caused an issue because the Down-Migration was called, but the Up-Migration was never called again. I assume this is because we use the migrator to explicitly execute migrations and it doesn't know when the last migration was done. I also assume this works correctly with the usual Database.MigrateAsync() method.

So my questions are:

  • Is it even safe to use IMigrator.MigrateAsync, when it can implicitly call down migrations but in the end will never call the Up migration again?
  • Is there a way we can ensure that the Up-Migration is executed in the end?
  • Is this behaviour even documented somewhere? It took a lot of time to figure out why a field was deleted.

Include provider and version information

EF Core version:
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 8.0
Operating system: Windows 11
IDE: Visual Studio 2022

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Oct 4, 2024

Is it even safe to use IMigrator.MigrateAsync, when it can implicitly call down migrations but in the end will never call the Up migration again?

It isn't safe to do it like in the code snippet provided. MigrateAsync was designed to be called once per session, with the target migration you'd want as the final state for your database.

We can add a parameter that would make Migrate throw if any migration would be reverted or if a migration to be applied is older than the newest applied migration.

Also, there are already logging events for when each migration is applied.

Is there a way we can ensure that the Up-Migration is executed in the end?

MigrateAsync will only call Up or Down for any particular migration in a single call. The logic it uses is fairly straightforward:

  • It calls Up for any unapplied migration older than the target migration
  • It calls Down for any applied migration newer than the target migration

Is this behaviour even documented somewhere? It took a lot of time to figure out why a field was deleted.

No, filed dotnet/EntityFramework.Docs#4824

@AndriySvyryd AndriySvyryd added this to the Backlog milestone Oct 4, 2024
@AndriySvyryd AndriySvyryd changed the title MigrateAsync with targetMigration causes issue with down Migration Add safeguard for MigrateAsync with targetMigration Oct 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

2 participants