From 953a4d966576d9095dc9532eef6d155b52adfec8 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Tue, 20 Aug 2024 22:35:29 -0700 Subject: [PATCH 1/4] Acquire the migrations database lock in a transaction. Reacquire the lock when retrying. Execute all migrations in a single transaction. Fixes #34439 --- .../Migrations/HistoryRepository.cs | 7 +- .../HistoryRepositoryDependencies.cs | 9 +- .../Migrations/IHistoryRepository.cs | 10 +- .../Migrations/IMigrationCommandExecutor.cs | 30 +++ .../Migrations/IMigrationsDatabaseLock.cs | 28 +++ .../Internal/MigrationCommandExecutor.cs | 144 +++++++++---- .../Migrations/Internal/Migrator.cs | 193 ++++++++++++------ .../Migrations/MigrationExecutionState.cs | 30 +++ .../Internal/SqlServerHistoryRepository.cs | 9 +- .../SqlServerMigrationDatabaseLock.cs | 20 +- .../Internal/SqliteHistoryRepository.cs | 18 +- .../Internal/SqliteMigrationDatabaseLock.cs | 21 +- .../Design/DesignTimeServicesTest.cs | 8 +- .../Design/MigrationScaffolderTest.cs | 7 +- .../RelationalDatabaseFacadeExtensionsTest.cs | 4 +- 15 files changed, 402 insertions(+), 136 deletions(-) create mode 100644 src/EFCore.Relational/Migrations/IMigrationsDatabaseLock.cs create mode 100644 src/EFCore.Relational/Migrations/MigrationExecutionState.cs diff --git a/src/EFCore.Relational/Migrations/HistoryRepository.cs b/src/EFCore.Relational/Migrations/HistoryRepository.cs index 78dc3b24013..ea34efbd505 100644 --- a/src/EFCore.Relational/Migrations/HistoryRepository.cs +++ b/src/EFCore.Relational/Migrations/HistoryRepository.cs @@ -197,16 +197,19 @@ protected virtual IReadOnlyList GetCreateCommands() /// /// Gets an exclusive lock on the database. /// + /// The transaction currently in use. /// An object that can be disposed to release the lock. - public abstract IDisposable GetDatabaseLock(); + public abstract IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction); /// /// Gets an exclusive lock on the database. /// + /// The transaction currently in use. /// A to observe while waiting for the task to complete. /// An object that can be disposed to release the lock. /// If the is canceled. - public abstract Task GetDatabaseLockAsync(CancellationToken cancellationToken = default); + public abstract Task AcquireDatabaseLockAsync( + IDbContextTransaction transaction, CancellationToken cancellationToken = default); /// /// Configures the entity type mapped to the history table. diff --git a/src/EFCore.Relational/Migrations/HistoryRepositoryDependencies.cs b/src/EFCore.Relational/Migrations/HistoryRepositoryDependencies.cs index b5f40bd8ba2..a8bf28d3f22 100644 --- a/src/EFCore.Relational/Migrations/HistoryRepositoryDependencies.cs +++ b/src/EFCore.Relational/Migrations/HistoryRepositoryDependencies.cs @@ -59,7 +59,8 @@ public HistoryRepositoryDependencies( IRelationalTypeMappingSource typeMappingSource, ICurrentDbContext currentContext, IModelRuntimeInitializer modelRuntimeInitializer, - IRelationalCommandDiagnosticsLogger commandLogger) + IRelationalCommandDiagnosticsLogger commandLogger, + IDiagnosticsLogger migrationsLogger) { DatabaseCreator = databaseCreator; RawSqlCommandBuilder = rawSqlCommandBuilder; @@ -75,6 +76,7 @@ public HistoryRepositoryDependencies( CurrentContext = currentContext; ModelRuntimeInitializer = modelRuntimeInitializer; CommandLogger = commandLogger; + MigrationsLogger = migrationsLogger; } /// @@ -146,4 +148,9 @@ public HistoryRepositoryDependencies( /// The command logger /// public IRelationalCommandDiagnosticsLogger CommandLogger { get; init; } + + /// + /// The migrations logger + /// + public IDiagnosticsLogger MigrationsLogger { get; init; } } diff --git a/src/EFCore.Relational/Migrations/IHistoryRepository.cs b/src/EFCore.Relational/Migrations/IHistoryRepository.cs index 7bf3a461650..5ee3f3d07b5 100644 --- a/src/EFCore.Relational/Migrations/IHistoryRepository.cs +++ b/src/EFCore.Relational/Migrations/IHistoryRepository.cs @@ -75,18 +75,20 @@ Task> GetAppliedMigrationsAsync( CancellationToken cancellationToken = default); /// - /// Gets an exclusive lock on the database. + /// Acquires an exclusive lock on the database. /// + /// The transaction currently in use. /// An object that can be disposed to release the lock. - IDisposable GetDatabaseLock(); + IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction); /// - /// Gets an exclusive lock on the database. + /// Acquires an exclusive lock on the database asynchronously. /// + /// The transaction currently in use. /// A to observe while waiting for the task to complete. /// An object that can be disposed to release the lock. /// If the is canceled. - Task GetDatabaseLockAsync(CancellationToken cancellationToken = default); + Task AcquireDatabaseLockAsync(IDbContextTransaction transaction, CancellationToken cancellationToken = default); /// /// Generates a SQL script that will create the history table. diff --git a/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs b/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs index 7069ba9c045..f4fb2dd2bb2 100644 --- a/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs +++ b/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs @@ -28,6 +28,19 @@ void ExecuteNonQuery( IEnumerable migrationCommands, IRelationalConnection connection); + /// + /// Executes the given commands using the given database connection. + /// + /// The commands to execute. + /// The connection to use. + /// The state of the current migration execution. + /// Indicates whether the commands should be executed in a transaction. + void ExecuteNonQuery( + IReadOnlyList migrationCommands, + IRelationalConnection connection, + MigrationExecutionState executionState, + bool executeInTransaction); + /// /// Executes the given commands using the given database connection. /// @@ -40,4 +53,21 @@ Task ExecuteNonQueryAsync( IEnumerable migrationCommands, IRelationalConnection connection, CancellationToken cancellationToken = default); + + /// + /// Executes the given commands using the given database connection. + /// + /// The commands to execute. + /// The connection to use. + /// The state of the current migration execution. + /// Indicates whether the commands should be executed in a transaction. + /// A to observe while waiting for the task to complete. + /// A task that represents the asynchronous operation. + /// If the is canceled. + Task ExecuteNonQueryAsync( + IReadOnlyList migrationCommands, + IRelationalConnection connection, + MigrationExecutionState executionState, + bool executeInTransaction, + CancellationToken cancellationToken = default); } diff --git a/src/EFCore.Relational/Migrations/IMigrationsDatabaseLock.cs b/src/EFCore.Relational/Migrations/IMigrationsDatabaseLock.cs new file mode 100644 index 00000000000..001a2070eb3 --- /dev/null +++ b/src/EFCore.Relational/Migrations/IMigrationsDatabaseLock.cs @@ -0,0 +1,28 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.Migrations; + +/// +/// Represents an exclusive lock on the database that is used to ensure that only one migration application can be run at a time. +/// +/// +/// Database providers typically implement this. +/// +public interface IMigrationsDatabaseLock : IDisposable, IAsyncDisposable +{ + /// + /// Acquires an exclusive lock on the database again, if the current one was already released. + /// + /// The transaction currently in use. + /// An object that can be disposed to release the lock. + IMigrationsDatabaseLock Reacquire(IDbContextTransaction? transaction); + + /// + /// Acquires an exclusive lock on the database again, if the current one was already released. + /// + /// The transaction currently in use. + /// A to observe while waiting for the task to complete. + /// An object that can be disposed to release the lock. + Task ReacquireAsync(IDbContextTransaction? transaction, CancellationToken cancellationToken = default); +} diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs b/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs index 4b2d78a85bc..ea959c4aaa4 100644 --- a/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs +++ b/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs @@ -17,7 +17,7 @@ namespace Microsoft.EntityFrameworkCore.Migrations.Internal; /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// -public class MigrationCommandExecutor(IExecutionStrategy executionStrategy) : IMigrationCommandExecutor +public class MigrationCommandExecutor : IMigrationCommandExecutor { /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -28,39 +28,53 @@ public class MigrationCommandExecutor(IExecutionStrategy executionStrategy) : IM public virtual void ExecuteNonQuery( IEnumerable migrationCommands, IRelationalConnection connection) + => ExecuteNonQuery( + migrationCommands.ToList(), connection, new MigrationExecutionState(), executeInTransaction: true); + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public virtual void ExecuteNonQuery( + IReadOnlyList migrationCommands, + IRelationalConnection connection, + MigrationExecutionState executionState, + bool executeInTransaction) { - // TODO: Remove ToList, see #19710 - var commands = migrationCommands.ToList(); var userTransaction = connection.CurrentTransaction; if (userTransaction is not null - && (commands.Any(x => x.TransactionSuppressed) || executionStrategy.RetriesOnFailure)) + && migrationCommands.Any(x => x.TransactionSuppressed)) { throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction); } using (new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled)) { - var parameters = new ExecuteParameters(commands, connection); if (userTransaction is null) { - executionStrategy.Execute(parameters, static (_, p) => Execute(p, beginTransaction: true), verifySucceeded: null); + Execute(migrationCommands, connection, executionState, beginTransaction: true, commitTransaction: !executeInTransaction); } else { - Execute(parameters, beginTransaction: false); + Execute(migrationCommands, connection, executionState, beginTransaction: false, commitTransaction: false); } } } - private static bool Execute(ExecuteParameters parameters, bool beginTransaction) + private static IDbContextTransaction? Execute( + IReadOnlyList migrationCommands, + IRelationalConnection connection, + MigrationExecutionState executionState, + bool beginTransaction, + bool commitTransaction) { - var migrationCommands = parameters.MigrationCommands; - var connection = parameters.Connection; - IDbContextTransaction? transaction = null; + var transaction = executionState.Transaction; connection.Open(); try { - for (var i = parameters.CurrentCommandIndex; i < migrationCommands.Count; i++) + for (var i = executionState.LastCommittedCommandIndex; i < migrationCommands.Count; i++) { var command = migrationCommands[i]; if (transaction == null @@ -68,6 +82,12 @@ private static bool Execute(ExecuteParameters parameters, bool beginTransaction) && beginTransaction) { transaction = connection.BeginTransaction(); + executionState.Transaction = transaction; + + if (executionState.DatabaseLock != null) + { + executionState.DatabaseLock = executionState.DatabaseLock.Reacquire(transaction); + } } if (transaction != null @@ -76,26 +96,40 @@ private static bool Execute(ExecuteParameters parameters, bool beginTransaction) transaction.Commit(); transaction.Dispose(); transaction = null; - parameters.CurrentCommandIndex = i; + executionState.Transaction = null; + executionState.LastCommittedCommandIndex = i; } command.ExecuteNonQuery(connection); if (transaction == null) { - parameters.CurrentCommandIndex = i + 1; + executionState.LastCommittedCommandIndex = i + 1; } } - transaction?.Commit(); + if (commitTransaction) + { + transaction?.Commit(); + } } - finally + catch { transaction?.Dispose(); connection.Close(); + executionState.Transaction = null; + throw; + } + + if (commitTransaction) + { + transaction?.Dispose(); + transaction = null; + executionState.Transaction = null; } - return true; + connection.Close(); + return transaction; } /// @@ -104,45 +138,58 @@ private static bool Execute(ExecuteParameters parameters, bool beginTransaction) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual async Task ExecuteNonQueryAsync( + public virtual Task ExecuteNonQueryAsync( IEnumerable migrationCommands, IRelationalConnection connection, CancellationToken cancellationToken = default) + => ExecuteNonQueryAsync( + migrationCommands.ToList(), connection, new MigrationExecutionState(), executeInTransaction: true, cancellationToken); + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public virtual async Task ExecuteNonQueryAsync( + IReadOnlyList migrationCommands, + IRelationalConnection connection, + MigrationExecutionState executionState, + bool executeInTransaction, + CancellationToken cancellationToken = default) { - var commands = migrationCommands.ToList(); var userTransaction = connection.CurrentTransaction; if (userTransaction is not null - && (commands.Any(x => x.TransactionSuppressed) || executionStrategy.RetriesOnFailure)) + && (migrationCommands.Any(x => x.TransactionSuppressed))) { throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction); } using var transactionScope = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled); - var parameters = new ExecuteParameters(commands, connection); if (userTransaction is null) { - await executionStrategy.ExecuteAsync( - parameters, - static (_, p, ct) => ExecuteAsync(p, beginTransaction: true, ct), - verifySucceeded: null, - cancellationToken).ConfigureAwait(false); + await ExecuteAsync(migrationCommands, connection, executionState, beginTransaction: false, commitTransaction: !executeInTransaction, cancellationToken).ConfigureAwait(false); } else { - await ExecuteAsync(parameters, beginTransaction: false, cancellationToken).ConfigureAwait(false); + await ExecuteAsync(migrationCommands, connection, executionState, beginTransaction: false, commitTransaction: false, cancellationToken).ConfigureAwait(false); } } - private static async Task ExecuteAsync(ExecuteParameters parameters, bool beginTransaction, CancellationToken cancellationToken) + private static async Task ExecuteAsync( + IReadOnlyList migrationCommands, + IRelationalConnection connection, + MigrationExecutionState executionState, + bool beginTransaction, + bool commitTransaction, + CancellationToken cancellationToken) { - var migrationCommands = parameters.MigrationCommands; - var connection = parameters.Connection; - IDbContextTransaction? transaction = null; + var transaction = executionState.Transaction; await connection.OpenAsync(cancellationToken).ConfigureAwait(false); try { - for (var i = parameters.CurrentCommandIndex; i < migrationCommands.Count; i++) + for (var i = executionState.LastCommittedCommandIndex; i < migrationCommands.Count; i++) { var command = migrationCommands[i]; if (transaction == null @@ -151,6 +198,13 @@ private static async Task ExecuteAsync(ExecuteParameters parameters, bool { transaction = await connection.BeginTransactionAsync(cancellationToken) .ConfigureAwait(false); + executionState.Transaction = transaction; + + if (executionState.DatabaseLock != null) + { + executionState.DatabaseLock = await executionState.DatabaseLock.ReacquireAsync(transaction, cancellationToken) + .ConfigureAwait(false); + } } if (transaction != null @@ -159,7 +213,8 @@ private static async Task ExecuteAsync(ExecuteParameters parameters, bool await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); await transaction.DisposeAsync().ConfigureAwait(false); transaction = null; - parameters.CurrentCommandIndex = i; + executionState.Transaction = null; + executionState.LastCommittedCommandIndex = i; } await command.ExecuteNonQueryAsync(connection, cancellationToken: cancellationToken) @@ -167,7 +222,7 @@ await command.ExecuteNonQueryAsync(connection, cancellationToken: cancellationTo if (transaction == null) { - parameters.CurrentCommandIndex = i + 1; + executionState.LastCommittedCommandIndex = i + 1; } } @@ -176,23 +231,26 @@ await command.ExecuteNonQueryAsync(connection, cancellationToken: cancellationTo await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); } } - finally + catch { if (transaction != null) { await transaction.DisposeAsync().ConfigureAwait(false); } - await connection.CloseAsync().ConfigureAwait(false); + executionState.Transaction = null; + throw; } - return true; - } + if (commitTransaction + && transaction != null) + { + await transaction.DisposeAsync().ConfigureAwait(false); + transaction = null; + executionState.Transaction = null; + } - private sealed class ExecuteParameters(List migrationCommands, IRelationalConnection connection) - { - public int CurrentCommandIndex; - public List MigrationCommands { get; } = migrationCommands; - public IRelationalConnection Connection { get; } = connection; + await connection.CloseAsync().ConfigureAwait(false); + return transaction; } } diff --git a/src/EFCore.Relational/Migrations/Internal/Migrator.cs b/src/EFCore.Relational/Migrations/Internal/Migrator.cs index c10cdd105c0..75deea0d334 100644 --- a/src/EFCore.Relational/Migrations/Internal/Migrator.cs +++ b/src/EFCore.Relational/Migrations/Internal/Migrator.cs @@ -1,7 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Threading; using Microsoft.EntityFrameworkCore.Diagnostics.Internal; +using Microsoft.EntityFrameworkCore.Storage; +using static Microsoft.EntityFrameworkCore.DbLoggerCategory.Database; namespace Microsoft.EntityFrameworkCore.Migrations.Internal; @@ -29,6 +32,7 @@ public class Migrator : IMigrator private readonly IDesignTimeModel _designTimeModel; private readonly string _activeProvider; private readonly IDbContextOptions _contextOptions; + private readonly IExecutionStrategy _executionStrategy; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -52,7 +56,8 @@ public Migrator( IDatabaseProvider databaseProvider, IMigrationsModelDiffer migrationsModelDiffer, IDesignTimeModel designTimeModel, - IDbContextOptions contextOptions) + IDbContextOptions contextOptions, + IExecutionStrategy executionStrategy) { _migrationsAssembly = migrationsAssembly; _historyRepository = historyRepository; @@ -70,6 +75,7 @@ public Migrator( _designTimeModel = designTimeModel; _activeProvider = databaseProvider.Name; _contextOptions = contextOptions; + _executionStrategy = executionStrategy; } /// @@ -80,6 +86,12 @@ public Migrator( /// public virtual void Migrate(string? targetMigration) { + if (_connection.CurrentTransaction is not null + && _executionStrategy.RetriesOnFailure) + { + throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction); + } + if (RelationalResources.LogPendingModelChanges(_logger).WarningBehavior != WarningBehavior.Ignore && HasPendingModelChanges()) { @@ -97,48 +109,68 @@ public virtual void Migrate(string? targetMigration) { _connection.Open(); - _logger.AcquiringMigrationLock(); - using var _ = _historyRepository.GetDatabaseLock(); - if (!_historyRepository.Exists()) { _historyRepository.Create(); } - PopulateMigrations( - _historyRepository.GetAppliedMigrations().Select(t => t.MigrationId), - targetMigration, - out var migratorData); + _executionStrategy.Execute( + (Migrator: this, TargetMigration: targetMigration, State: new MigrationExecutionState()), + (_, s) => s.Migrator.MigrateImplementation(s.TargetMigration, s.State), verifySucceeded: null); + } + finally + { + _connection.Close(); + } + } - var commandLists = GetMigrationCommandLists(migratorData); - foreach (var commandList in commandLists) + private bool MigrateImplementation(string? targetMigration, MigrationExecutionState state) + { + _connection.Open(); + var context = _currentContext.Context; + using var transaction = context.Database.BeginTransaction(); + state.DatabaseLock = state.DatabaseLock == null + ? _historyRepository.AcquireDatabaseLock(transaction) + : state.DatabaseLock.Reacquire(transaction); + + PopulateMigrations( + _historyRepository.GetAppliedMigrations().Select(t => t.MigrationId), + targetMigration, + out var migratorData); + + var commandLists = GetMigrationCommandLists(migratorData); + foreach (var commandList in commandLists) + { + var (id, getCommands) = commandList; + + if (id != state.CurrentMigrationId) { - _migrationCommandExecutor.ExecuteNonQuery(commandList(), _connection); + state.CurrentMigrationId = id; + state.LastCommittedCommandIndex = 0; } - var coreOptionsExtension = - _contextOptions.FindExtension() - ?? new CoreOptionsExtension(); + _migrationCommandExecutor.ExecuteNonQuery(getCommands(), _connection); + } - var seed = coreOptionsExtension.Seeder; - if (seed != null) - { - var context = _currentContext.Context; - var operationsPerformed = migratorData.AppliedMigrations.Count != 0 - || migratorData.RevertedMigrations.Count != 0; - using var transaction = context.Database.BeginTransaction(); - seed(context, operationsPerformed); - transaction.Commit(); - } - else if (coreOptionsExtension.AsyncSeeder != null) - { - throw new InvalidOperationException(CoreStrings.MissingSeeder); - } + var coreOptionsExtension = + _contextOptions.FindExtension() + ?? new CoreOptionsExtension(); + + var seed = coreOptionsExtension.Seeder; + if (seed != null) + { + var operationsPerformed = migratorData.AppliedMigrations.Count != 0 + || migratorData.RevertedMigrations.Count != 0; + seed(context, operationsPerformed); } - finally + else if (coreOptionsExtension.AsyncSeeder != null) { - _connection.Close(); + throw new InvalidOperationException(CoreStrings.MissingSeeder); } + + transaction.Commit(); + _connection.Close(); + return true; } /// @@ -151,6 +183,12 @@ public virtual async Task MigrateAsync( string? targetMigration, CancellationToken cancellationToken = default) { + if (_connection.CurrentTransaction is not null + && _executionStrategy.RetriesOnFailure) + { + throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction); + } + if (RelationalResources.LogPendingModelChanges(_logger).WarningBehavior != WarningBehavior.Ignore && HasPendingModelChanges()) { @@ -168,54 +206,75 @@ public virtual async Task MigrateAsync( { await _connection.OpenAsync(cancellationToken).ConfigureAwait(false); - _logger.AcquiringMigrationLock(); - var dbLock = await _historyRepository.GetDatabaseLockAsync(cancellationToken).ConfigureAwait(false); - await using var _ = dbLock.ConfigureAwait(false); - if (!await _historyRepository.ExistsAsync(cancellationToken).ConfigureAwait(false)) { await _historyRepository.CreateAsync(cancellationToken).ConfigureAwait(false); } - PopulateMigrations( - (await _historyRepository.GetAppliedMigrationsAsync(cancellationToken).ConfigureAwait(false)).Select(t => t.MigrationId), - targetMigration, - out var migratorData); + await _executionStrategy.ExecuteAsync( + (Migrator: this, TargetMigration: targetMigration, State: new MigrationExecutionState()), + async (_, s, ct) => await s.Migrator.MigrateImplementationAsync(s.TargetMigration, s.State, ct).ConfigureAwait(false), verifySucceeded: null, cancellationToken) + .ConfigureAwait(false); + } + finally + { + _connection.Close(); + } + } - var commandLists = GetMigrationCommandLists(migratorData); - foreach (var commandList in commandLists) + private async Task MigrateImplementationAsync(string? targetMigration, MigrationExecutionState state, CancellationToken cancellationToken = default) + { + await _connection.OpenAsync(cancellationToken).ConfigureAwait(false); + var context = _currentContext.Context; + using var transaction = await context.Database.BeginTransactionAsync(cancellationToken).ConfigureAwait(false); + await using var _ = transaction.ConfigureAwait(false); + + state.DatabaseLock = state.DatabaseLock == null + ? await _historyRepository.AcquireDatabaseLockAsync(transaction, cancellationToken).ConfigureAwait(false) + : await state.DatabaseLock.ReacquireAsync(transaction, cancellationToken).ConfigureAwait(false); + + PopulateMigrations( + (await _historyRepository.GetAppliedMigrationsAsync(cancellationToken).ConfigureAwait(false)).Select(t => t.MigrationId), + targetMigration, + out var migratorData); + + var commandLists = GetMigrationCommandLists(migratorData); + foreach (var commandList in commandLists) + { + var (id, getCommands) = commandList; + + if (id != state.CurrentMigrationId) { - await _migrationCommandExecutor.ExecuteNonQueryAsync(commandList(), _connection, cancellationToken) - .ConfigureAwait(false); + state.CurrentMigrationId = id; + state.LastCommittedCommandIndex = 0; } - var coreOptionsExtension = - _contextOptions.FindExtension() - ?? new CoreOptionsExtension(); + await _migrationCommandExecutor.ExecuteNonQueryAsync(getCommands(), _connection, cancellationToken) + .ConfigureAwait(false); + } - var seedAsync = coreOptionsExtension.AsyncSeeder; - if (seedAsync != null) - { - var context = _currentContext.Context; - var operationsPerformed = migratorData.AppliedMigrations.Count != 0 - || migratorData.RevertedMigrations.Count != 0; - var transaction = await context.Database.BeginTransactionAsync(cancellationToken).ConfigureAwait(false); - await using var __ = transaction.ConfigureAwait(false); - await seedAsync(context, operationsPerformed, cancellationToken).ConfigureAwait(false); - await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); - } - else if (coreOptionsExtension.Seeder != null) - { - throw new InvalidOperationException(CoreStrings.MissingSeeder); - } + var coreOptionsExtension = + _contextOptions.FindExtension() + ?? new CoreOptionsExtension(); + + var seedAsync = coreOptionsExtension.AsyncSeeder; + if (seedAsync != null) + { + var operationsPerformed = migratorData.AppliedMigrations.Count != 0 + || migratorData.RevertedMigrations.Count != 0; + await seedAsync(context, operationsPerformed, cancellationToken).ConfigureAwait(false); } - finally + else if (coreOptionsExtension.Seeder != null) { - _connection.Close(); + throw new InvalidOperationException(CoreStrings.MissingSeeder); } + + await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); + _connection.Close(); + return true; } - private IEnumerable>> GetMigrationCommandLists(MigratorData parameters) + private IEnumerable<(string, Func>)> GetMigrationCommandLists(MigratorData parameters) { var migrationsToApply = parameters.AppliedMigrations; var migrationsToRevert = parameters.RevertedMigrations; @@ -226,7 +285,7 @@ private IEnumerable>> GetMigrationCommandLi var migration = migrationsToRevert[i]; var index = i; - yield return () => + yield return (migration.GetId(), () => { _logger.MigrationReverting(this, migration); @@ -242,12 +301,12 @@ private IEnumerable>> GetMigrationCommandLi } return commands; - }; + }); } foreach (var migration in migrationsToApply) { - yield return () => + yield return (migration.GetId(), () => { _logger.MigrationApplying(this, migration); @@ -259,7 +318,7 @@ private IEnumerable>> GetMigrationCommandLi } return commands; - }; + }); } if (migrationsToRevert.Count + migrationsToApply.Count == 0) diff --git a/src/EFCore.Relational/Migrations/MigrationExecutionState.cs b/src/EFCore.Relational/Migrations/MigrationExecutionState.cs new file mode 100644 index 00000000000..e3769da5e25 --- /dev/null +++ b/src/EFCore.Relational/Migrations/MigrationExecutionState.cs @@ -0,0 +1,30 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.Migrations; + +/// +/// Contains the state of the current migration execution. +/// +public sealed class MigrationExecutionState +{ + /// + /// The index of the last command that was committed to the database. + /// + public int LastCommittedCommandIndex { get; set; } + + /// + /// The id the migration that is currently being applied. + /// + public string? CurrentMigrationId { get; set; } + + /// + /// The database lock that is in use. + /// + public IMigrationsDatabaseLock? DatabaseLock { get; set; } + + /// + /// The transaction that is in use. + /// + public IDbContextTransaction? Transaction { get; set; } +} diff --git a/src/EFCore.SqlServer/Migrations/Internal/SqlServerHistoryRepository.cs b/src/EFCore.SqlServer/Migrations/Internal/SqlServerHistoryRepository.cs index cc22d67fca4..679fb5dd6be 100644 --- a/src/EFCore.SqlServer/Migrations/Internal/SqlServerHistoryRepository.cs +++ b/src/EFCore.SqlServer/Migrations/Internal/SqlServerHistoryRepository.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Data; using System.Text; namespace Microsoft.EntityFrameworkCore.SqlServer.Migrations.Internal; @@ -59,8 +60,10 @@ protected override bool InterpretExistsResult(object? value) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public override IDisposable GetDatabaseLock() + public override IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction) { + Dependencies.MigrationsLogger.AcquiringMigrationLock(); + var dbLock = CreateMigrationDatabaseLock(); int result; try @@ -91,8 +94,10 @@ public override IDisposable GetDatabaseLock() /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public override async Task GetDatabaseLockAsync(CancellationToken cancellationToken = default) + public override async Task AcquireDatabaseLockAsync(IDbContextTransaction transaction, CancellationToken cancellationToken = default) { + Dependencies.MigrationsLogger.AcquiringMigrationLock(); + var dbLock = CreateMigrationDatabaseLock(); int result; try diff --git a/src/EFCore.SqlServer/Migrations/Internal/SqlServerMigrationDatabaseLock.cs b/src/EFCore.SqlServer/Migrations/Internal/SqlServerMigrationDatabaseLock.cs index c102f6f9705..c069413fa34 100644 --- a/src/EFCore.SqlServer/Migrations/Internal/SqlServerMigrationDatabaseLock.cs +++ b/src/EFCore.SqlServer/Migrations/Internal/SqlServerMigrationDatabaseLock.cs @@ -19,7 +19,7 @@ public class SqlServerMigrationDatabaseLock( IRelationalCommand relationalCommand, RelationalCommandParameterObject relationalCommandParameters, CancellationToken cancellationToken = default) - : IDisposable, IAsyncDisposable + : IMigrationsDatabaseLock { /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -38,4 +38,22 @@ public void Dispose() /// public async ValueTask DisposeAsync() => await relationalCommand.ExecuteScalarAsync(relationalCommandParameters, cancellationToken).ConfigureAwait(false); + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public IMigrationsDatabaseLock Reacquire(IDbContextTransaction? transaction) + => this; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public Task ReacquireAsync(IDbContextTransaction? transaction, CancellationToken cancellationToken = default) + => Task.FromResult((IMigrationsDatabaseLock)this); } diff --git a/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteHistoryRepository.cs b/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteHistoryRepository.cs index e346fd1e698..332e8e4f402 100644 --- a/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteHistoryRepository.cs +++ b/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteHistoryRepository.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Data; using Microsoft.EntityFrameworkCore.Sqlite.Internal; namespace Microsoft.EntityFrameworkCore.Sqlite.Migrations.Internal; @@ -103,11 +104,13 @@ public override string GetEndIfScript() /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public override IDisposable GetDatabaseLock() + public override IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction) { + Dependencies.MigrationsLogger.AcquiringMigrationLock(); + if (!InterpretExistsResult( - Dependencies.RawSqlCommandBuilder.Build(CreateExistsSql(LockTableName)) - .ExecuteScalar(CreateRelationalCommandParameters()))) + Dependencies.RawSqlCommandBuilder.Build(CreateExistsSql(LockTableName)) + .ExecuteScalar(CreateRelationalCommandParameters()))) { CreateLockTableCommand().ExecuteNonQuery(CreateRelationalCommandParameters()); } @@ -139,11 +142,14 @@ public override IDisposable GetDatabaseLock() /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public override async Task GetDatabaseLockAsync(CancellationToken cancellationToken = default) + public override async Task AcquireDatabaseLockAsync( + IDbContextTransaction transaction, CancellationToken cancellationToken = default) { + Dependencies.MigrationsLogger.AcquiringMigrationLock(); + if (!InterpretExistsResult( - await Dependencies.RawSqlCommandBuilder.Build(CreateExistsSql(LockTableName)) - .ExecuteScalarAsync(CreateRelationalCommandParameters(), cancellationToken).ConfigureAwait(false))) + await Dependencies.RawSqlCommandBuilder.Build(CreateExistsSql(LockTableName)) + .ExecuteScalarAsync(CreateRelationalCommandParameters(), cancellationToken).ConfigureAwait(false))) { await CreateLockTableCommand().ExecuteNonQueryAsync(CreateRelationalCommandParameters(), cancellationToken) .ConfigureAwait(false); diff --git a/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteMigrationDatabaseLock.cs b/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteMigrationDatabaseLock.cs index 997cbcd4710..062dc1270b6 100644 --- a/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteMigrationDatabaseLock.cs +++ b/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteMigrationDatabaseLock.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. + namespace Microsoft.EntityFrameworkCore.Sqlite.Migrations.Internal; /// @@ -13,7 +14,7 @@ public class SqliteMigrationDatabaseLock( IRelationalCommand relationalCommand, RelationalCommandParameterObject relationalCommandParameters, CancellationToken cancellationToken = default) - : IDisposable, IAsyncDisposable + : IMigrationsDatabaseLock { /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -32,4 +33,22 @@ public void Dispose() /// public async ValueTask DisposeAsync() => await relationalCommand.ExecuteScalarAsync(relationalCommandParameters, cancellationToken).ConfigureAwait(false); + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public IMigrationsDatabaseLock Reacquire(IDbContextTransaction? transaction) + => this; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public Task ReacquireAsync(IDbContextTransaction? transaction, CancellationToken cancellationToken = default) + => Task.FromResult((IMigrationsDatabaseLock)this); } diff --git a/test/EFCore.Design.Tests/Design/DesignTimeServicesTest.cs b/test/EFCore.Design.Tests/Design/DesignTimeServicesTest.cs index c2c58289d54..caf478b65ce 100644 --- a/test/EFCore.Design.Tests/Design/DesignTimeServicesTest.cs +++ b/test/EFCore.Design.Tests/Design/DesignTimeServicesTest.cs @@ -222,10 +222,10 @@ public string GetCreateIfNotExistsScript() public string GetCreateScript() => throw new NotImplementedException(); - public IDisposable GetDatabaseLock() + public IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction) => throw new NotImplementedException(); - public Task GetDatabaseLockAsync(CancellationToken cancellationToken = default) + public Task AcquireDatabaseLockAsync(IDbContextTransaction transaction, CancellationToken cancellationToken = default) => throw new NotImplementedException(); public string GetDeleteScript(string migrationId) @@ -294,10 +294,10 @@ public void Create() public Task CreateAsync(CancellationToken cancellationToken = default) => throw new NotImplementedException(); - public IDisposable GetDatabaseLock() + public IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction) => throw new NotImplementedException(); - public Task GetDatabaseLockAsync(CancellationToken cancellationToken = default) + public Task AcquireDatabaseLockAsync(IDbContextTransaction transaction, CancellationToken cancellationToken = default) => throw new NotImplementedException(); public string GetDeleteScript(string migrationId) diff --git a/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs b/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs index 94fa478789e..d069fe3e7ed 100644 --- a/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs +++ b/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs @@ -125,7 +125,8 @@ var migrationAssembly services.GetRequiredService(), services.GetRequiredService(), services.GetRequiredService(), - services.GetRequiredService()))); + services.GetRequiredService(), + services.GetRequiredService()))); } // ReSharper disable once UnusedTypeParameter @@ -182,10 +183,10 @@ public void Create() public Task CreateAsync(CancellationToken cancellationToken = default) => throw new NotImplementedException(); - public IDisposable GetDatabaseLock() + public IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction) => throw new NotImplementedException(); - public Task GetDatabaseLockAsync(CancellationToken cancellationToken = default) + public Task AcquireDatabaseLockAsync(IDbContextTransaction transaction, CancellationToken cancellationToken = default) => throw new NotImplementedException(); } diff --git a/test/EFCore.Relational.Tests/Extensions/RelationalDatabaseFacadeExtensionsTest.cs b/test/EFCore.Relational.Tests/Extensions/RelationalDatabaseFacadeExtensionsTest.cs index 3e56284521a..adf167e3cc8 100644 --- a/test/EFCore.Relational.Tests/Extensions/RelationalDatabaseFacadeExtensionsTest.cs +++ b/test/EFCore.Relational.Tests/Extensions/RelationalDatabaseFacadeExtensionsTest.cs @@ -362,10 +362,10 @@ public void Create() public Task CreateAsync(CancellationToken cancellationToken = default) => throw new NotImplementedException(); - public IDisposable GetDatabaseLock() + public IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction) => throw new NotImplementedException(); - public Task GetDatabaseLockAsync(CancellationToken cancellationToken = default) + public Task AcquireDatabaseLockAsync(IDbContextTransaction transaction, CancellationToken cancellationToken = default) => throw new NotImplementedException(); } From 15dbb8c4042dbd972ee3de31ca36c80c993b9b34 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Wed, 21 Aug 2024 12:17:41 -0700 Subject: [PATCH 2/4] Review feedback --- .../Migrations/HistoryRepository.cs | 39 ++- .../Migrations/IHistoryRepository.cs | 47 ++- .../Migrations/IMigrationCommandExecutor.cs | 24 +- .../Migrations/IMigrationsDatabaseLock.cs | 46 ++- .../Internal/MigrationCommandExecutor.cs | 191 +++++++----- .../Migrations/Internal/Migrator.cs | 290 ++++++++++++------ .../Migrations/LockReleaseBehavior.cs | 25 ++ .../Migrations/MigrationExecutionState.cs | 5 + .../Storage/RelationalDatabaseCreator.cs | 4 +- .../Internal/SqlServerHistoryRepository.cs | 16 +- .../SqlServerMigrationDatabaseLock.cs | 21 +- .../Internal/SqlServerDatabaseCreator.cs | 44 ++- .../Internal/SqliteHistoryRepository.cs | 15 +- .../Internal/SqliteMigrationDatabaseLock.cs | 22 +- .../Design/DesignTimeServicesTest.cs | 12 +- .../Design/MigrationScaffolderTest.cs | 6 +- .../RelationalDatabaseFacadeExtensionsTest.cs | 6 +- .../MigrationsInfrastructureSqlServerTest.cs | 61 +++- .../TestSqlServerRetryingExecutionStrategy.cs | 7 + 19 files changed, 591 insertions(+), 290 deletions(-) create mode 100644 src/EFCore.Relational/Migrations/LockReleaseBehavior.cs diff --git a/src/EFCore.Relational/Migrations/HistoryRepository.cs b/src/EFCore.Relational/Migrations/HistoryRepository.cs index ea34efbd505..0e233377f7d 100644 --- a/src/EFCore.Relational/Migrations/HistoryRepository.cs +++ b/src/EFCore.Relational/Migrations/HistoryRepository.cs @@ -47,6 +47,14 @@ protected HistoryRepository(HistoryRepositoryDependencies dependencies) TableSchema = relationalOptions.MigrationsHistoryTableSchema; } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public abstract LockReleaseBehavior LockReleaseBehavior { get; } + /// /// Relational provider-specific dependencies for this service. /// @@ -173,13 +181,15 @@ public virtual string GetCreateScript() /// Creates the history table. /// public virtual void Create() - => Dependencies.MigrationCommandExecutor.ExecuteNonQuery(GetCreateCommands(), Dependencies.Connection); + => Dependencies.MigrationCommandExecutor.ExecuteNonQuery( + GetCreateCommands(), Dependencies.Connection, new MigrationExecutionState(), commitTransaction: true); /// /// Creates the history table. /// public virtual Task CreateAsync(CancellationToken cancellationToken = default) - => Dependencies.MigrationCommandExecutor.ExecuteNonQueryAsync(GetCreateCommands(), Dependencies.Connection, cancellationToken); + => Dependencies.MigrationCommandExecutor.ExecuteNonQueryAsync( + GetCreateCommands(), Dependencies.Connection, new MigrationExecutionState(), commitTransaction: true, cancellationToken: cancellationToken); /// /// Returns the migration commands that will create the history table. @@ -194,22 +204,37 @@ protected virtual IReadOnlyList GetCreateCommands() return commandList; } + bool IHistoryRepository.CreateIfNotExists() + => Dependencies.MigrationCommandExecutor.ExecuteNonQuery( + GetCreateIfNotExistsCommands(), Dependencies.Connection, new MigrationExecutionState(), commitTransaction: true) + != 0; + + async Task IHistoryRepository.CreateIfNotExistsAsync(CancellationToken cancellationToken) + => (await Dependencies.MigrationCommandExecutor.ExecuteNonQueryAsync( + GetCreateIfNotExistsCommands(), Dependencies.Connection, new MigrationExecutionState(), commitTransaction: true, cancellationToken: cancellationToken).ConfigureAwait(false)) + != 0; + + private IReadOnlyList GetCreateIfNotExistsCommands() + => Dependencies.MigrationsSqlGenerator.Generate([new SqlOperation + { + Sql = GetCreateIfNotExistsScript(), + SuppressTransaction = true + }]); + /// /// Gets an exclusive lock on the database. /// - /// The transaction currently in use. /// An object that can be disposed to release the lock. - public abstract IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction); + public abstract IMigrationsDatabaseLock AcquireDatabaseLock(); /// /// Gets an exclusive lock on the database. /// - /// The transaction currently in use. /// A to observe while waiting for the task to complete. + /// /// An object that can be disposed to release the lock. /// If the is canceled. - public abstract Task AcquireDatabaseLockAsync( - IDbContextTransaction transaction, CancellationToken cancellationToken = default); + public abstract Task AcquireDatabaseLockAsync(CancellationToken cancellationToken = default); /// /// Configures the entity type mapped to the history table. diff --git a/src/EFCore.Relational/Migrations/IHistoryRepository.cs b/src/EFCore.Relational/Migrations/IHistoryRepository.cs index 5ee3f3d07b5..2189e6cea87 100644 --- a/src/EFCore.Relational/Migrations/IHistoryRepository.cs +++ b/src/EFCore.Relational/Migrations/IHistoryRepository.cs @@ -50,12 +50,44 @@ public interface IHistoryRepository /// /// A to observe while waiting for the task to complete. /// - /// A task that represents the asynchronous operation. The task result contains - /// if the table already exists, otherwise. + /// A task that represents the asynchronous operation. /// /// If the is canceled. Task CreateAsync(CancellationToken cancellationToken = default); + /// + /// Creates the history table if it doesn't exist. + /// + /// if the table was created, otherwise. + bool CreateIfNotExists() + { + if (!Exists()) + { + Create(); + return true; + } + return false; + } + + /// + /// Creates the history table. + /// + /// A to observe while waiting for the task to complete. + /// + /// A task that represents the asynchronous operation. The task result contains + /// if the table was created, otherwise. + /// + /// If the is canceled. + async Task CreateIfNotExistsAsync(CancellationToken cancellationToken = default) + { + if (!await ExistsAsync(cancellationToken).ConfigureAwait(false)) + { + await CreateAsync(cancellationToken).ConfigureAwait(false); + return true; + } + return false; + } + /// /// Queries the history table for all migrations that have been applied. /// @@ -74,21 +106,24 @@ public interface IHistoryRepository Task> GetAppliedMigrationsAsync( CancellationToken cancellationToken = default); + /// + /// The condition under witch the lock is released implicitly. + /// + LockReleaseBehavior LockReleaseBehavior { get; } + /// /// Acquires an exclusive lock on the database. /// - /// The transaction currently in use. /// An object that can be disposed to release the lock. - IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction); + IMigrationsDatabaseLock AcquireDatabaseLock(); /// /// Acquires an exclusive lock on the database asynchronously. /// - /// The transaction currently in use. /// A to observe while waiting for the task to complete. /// An object that can be disposed to release the lock. /// If the is canceled. - Task AcquireDatabaseLockAsync(IDbContextTransaction transaction, CancellationToken cancellationToken = default); + Task AcquireDatabaseLockAsync(CancellationToken cancellationToken = default); /// /// Generates a SQL script that will create the history table. diff --git a/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs b/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs index f4fb2dd2bb2..0b18e7e84b7 100644 --- a/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs +++ b/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs @@ -1,6 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Data; + namespace Microsoft.EntityFrameworkCore.Migrations; /// @@ -34,12 +36,17 @@ void ExecuteNonQuery( /// The commands to execute. /// The connection to use. /// The state of the current migration execution. - /// Indicates whether the commands should be executed in a transaction. - void ExecuteNonQuery( + /// + /// Indicates whether the transaction started by this call should be commited. + /// If , the transaction will be made available in . + /// + /// The isolation level for the transaction. + int ExecuteNonQuery( IReadOnlyList migrationCommands, IRelationalConnection connection, MigrationExecutionState executionState, - bool executeInTransaction); + bool commitTransaction, + IsolationLevel isolationLevel = IsolationLevel.Unspecified); /// /// Executes the given commands using the given database connection. @@ -60,14 +67,19 @@ Task ExecuteNonQueryAsync( /// The commands to execute. /// The connection to use. /// The state of the current migration execution. - /// Indicates whether the commands should be executed in a transaction. + /// + /// Indicates whether the transaction started by this call should be commited. + /// If , the transaction will be made available in . + /// + /// The isolation level for the transaction. /// A to observe while waiting for the task to complete. /// A task that represents the asynchronous operation. /// If the is canceled. - Task ExecuteNonQueryAsync( + Task ExecuteNonQueryAsync( IReadOnlyList migrationCommands, IRelationalConnection connection, MigrationExecutionState executionState, - bool executeInTransaction, + bool commitTransaction, + IsolationLevel isolationLevel = IsolationLevel.Unspecified, CancellationToken cancellationToken = default); } diff --git a/src/EFCore.Relational/Migrations/IMigrationsDatabaseLock.cs b/src/EFCore.Relational/Migrations/IMigrationsDatabaseLock.cs index 001a2070eb3..9e1f0cc7803 100644 --- a/src/EFCore.Relational/Migrations/IMigrationsDatabaseLock.cs +++ b/src/EFCore.Relational/Migrations/IMigrationsDatabaseLock.cs @@ -7,22 +7,56 @@ namespace Microsoft.EntityFrameworkCore.Migrations; /// Represents an exclusive lock on the database that is used to ensure that only one migration application can be run at a time. /// /// -/// Database providers typically implement this. +/// Typically only database providers implement this. /// public interface IMigrationsDatabaseLock : IDisposable, IAsyncDisposable { /// - /// Acquires an exclusive lock on the database again, if the current one was already released. + /// The history repository. + /// + protected IHistoryRepository HistoryRepository { get; } + + /// + /// Acquires an exclusive lock on the database again if the current one was already released. /// - /// The transaction currently in use. + /// Indicates whether the connection was reopened. + /// + /// Indicates whether the transaction was restarted. + /// if there's no current transaction. + /// /// An object that can be disposed to release the lock. - IMigrationsDatabaseLock Reacquire(IDbContextTransaction? transaction); + IMigrationsDatabaseLock Refresh(bool connectionReopened, bool? transactionRestarted) + { + if ((connectionReopened && HistoryRepository.LockReleaseBehavior == LockReleaseBehavior.Connection) + || (transactionRestarted is true && HistoryRepository.LockReleaseBehavior == LockReleaseBehavior.Transaction)) + { + Dispose(); + return HistoryRepository.AcquireDatabaseLock(); + } + + return this; + } /// /// Acquires an exclusive lock on the database again, if the current one was already released. /// - /// The transaction currently in use. + /// Indicates whether the connection was reopened. + /// + /// Indicates whether the transaction was restarted. + /// if there's no current transaction. + /// /// A to observe while waiting for the task to complete. /// An object that can be disposed to release the lock. - Task ReacquireAsync(IDbContextTransaction? transaction, CancellationToken cancellationToken = default); + async Task RefreshAsync( + bool connectionReopened, bool? transactionRestarted, CancellationToken cancellationToken = default) + { + if ((connectionReopened && HistoryRepository.LockReleaseBehavior == LockReleaseBehavior.Connection) + || (transactionRestarted is true && HistoryRepository.LockReleaseBehavior == LockReleaseBehavior.Transaction)) + { + await DisposeAsync().ConfigureAwait(false); + return await HistoryRepository.AcquireDatabaseLockAsync(cancellationToken).ConfigureAwait(false); + } + + return this; + } } diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs b/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs index ea959c4aaa4..45ca493a03c 100644 --- a/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs +++ b/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs @@ -17,7 +17,7 @@ namespace Microsoft.EntityFrameworkCore.Migrations.Internal; /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// -public class MigrationCommandExecutor : IMigrationCommandExecutor +public class MigrationCommandExecutor(IExecutionStrategy executionStrategy) : IMigrationCommandExecutor { /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -29,7 +29,7 @@ public virtual void ExecuteNonQuery( IEnumerable migrationCommands, IRelationalConnection connection) => ExecuteNonQuery( - migrationCommands.ToList(), connection, new MigrationExecutionState(), executeInTransaction: true); + migrationCommands.ToList(), connection, new MigrationExecutionState(), commitTransaction: true); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -37,99 +37,110 @@ public virtual void ExecuteNonQuery( /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual void ExecuteNonQuery( + public virtual int ExecuteNonQuery( IReadOnlyList migrationCommands, IRelationalConnection connection, MigrationExecutionState executionState, - bool executeInTransaction) + bool commitTransaction, + System.Data.IsolationLevel isolationLevel = System.Data.IsolationLevel.Unspecified) { - var userTransaction = connection.CurrentTransaction; - if (userTransaction is not null - && migrationCommands.Any(x => x.TransactionSuppressed)) + var inUserTransaction = connection.CurrentTransaction is not null && executionState.Transaction == null; + if (inUserTransaction + && (migrationCommands.Any(x => x.TransactionSuppressed) || executionStrategy.RetriesOnFailure)) { throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction); } using (new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled)) { - if (userTransaction is null) - { - Execute(migrationCommands, connection, executionState, beginTransaction: true, commitTransaction: !executeInTransaction); - } - else - { - Execute(migrationCommands, connection, executionState, beginTransaction: false, commitTransaction: false); - } + return executionStrategy.Execute( + (migrationCommands, connection, inUserTransaction, executionState, commitTransaction, isolationLevel), + static (_, s) => Execute( + s.migrationCommands, + s.connection, + s.executionState, + beginTransaction: !s.inUserTransaction, + commitTransaction: !s.inUserTransaction && s.commitTransaction, + s.isolationLevel), + verifySucceeded: null); } } - private static IDbContextTransaction? Execute( + private static int Execute( IReadOnlyList migrationCommands, IRelationalConnection connection, MigrationExecutionState executionState, bool beginTransaction, - bool commitTransaction) + bool commitTransaction, + System.Data.IsolationLevel isolationLevel = System.Data.IsolationLevel.Unspecified) { - var transaction = executionState.Transaction; - connection.Open(); + var result = 0; + var connectionOpened = connection.Open(); + Check.DebugAssert(!connectionOpened || executionState.Transaction == null, + "executionState.Transaction should be null"); + try { for (var i = executionState.LastCommittedCommandIndex; i < migrationCommands.Count; i++) { var command = migrationCommands[i]; - if (transaction == null + if (executionState.Transaction == null && !command.TransactionSuppressed && beginTransaction) { - transaction = connection.BeginTransaction(); - executionState.Transaction = transaction; - + executionState.Transaction = connection.BeginTransaction(isolationLevel); if (executionState.DatabaseLock != null) { - executionState.DatabaseLock = executionState.DatabaseLock.Reacquire(transaction); + executionState.DatabaseLock = executionState.DatabaseLock.Refresh( + connectionOpened, transactionRestarted: true); + connectionOpened = false; } } - if (transaction != null + if (executionState.Transaction != null && command.TransactionSuppressed) { - transaction.Commit(); - transaction.Dispose(); - transaction = null; + executionState.Transaction.Commit(); + executionState.Transaction.Dispose(); executionState.Transaction = null; executionState.LastCommittedCommandIndex = i; + executionState.AnyOperationPerformed = true; + + if (executionState.DatabaseLock != null) + { + executionState.DatabaseLock = executionState.DatabaseLock.Refresh( + connectionOpened, transactionRestarted: null); + connectionOpened = false; + } } - command.ExecuteNonQuery(connection); + result = command.ExecuteNonQuery(connection); - if (transaction == null) + if (executionState.Transaction == null) { executionState.LastCommittedCommandIndex = i + 1; + executionState.AnyOperationPerformed = true; } } - if (commitTransaction) + if (commitTransaction + && executionState.Transaction != null) { - transaction?.Commit(); + executionState.Transaction.Commit(); + executionState.Transaction.Dispose(); + executionState.Transaction = null; } } catch { - transaction?.Dispose(); - connection.Close(); + executionState.Transaction?.Dispose(); executionState.Transaction = null; + connection.Close(); throw; } - if (commitTransaction) - { - transaction?.Dispose(); - transaction = null; - executionState.Transaction = null; - } - connection.Close(); - return transaction; + return result; } /// @@ -143,7 +154,7 @@ public virtual Task ExecuteNonQueryAsync( IRelationalConnection connection, CancellationToken cancellationToken = default) => ExecuteNonQueryAsync( - migrationCommands.ToList(), connection, new MigrationExecutionState(), executeInTransaction: true, cancellationToken); + migrationCommands.ToList(), connection, new MigrationExecutionState(), commitTransaction: true, System.Data.IsolationLevel.Unspecified, cancellationToken); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -151,106 +162,120 @@ public virtual Task ExecuteNonQueryAsync( /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual async Task ExecuteNonQueryAsync( + public virtual async Task ExecuteNonQueryAsync( IReadOnlyList migrationCommands, IRelationalConnection connection, MigrationExecutionState executionState, - bool executeInTransaction, + bool commitTransaction, + System.Data.IsolationLevel isolationLevel = System.Data.IsolationLevel.Unspecified, CancellationToken cancellationToken = default) { - var userTransaction = connection.CurrentTransaction; - if (userTransaction is not null - && (migrationCommands.Any(x => x.TransactionSuppressed))) + var inUserTransaction = connection.CurrentTransaction is not null && executionState.Transaction == null; + if (inUserTransaction + && (migrationCommands.Any(x => x.TransactionSuppressed) || executionStrategy.RetriesOnFailure)) { throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction); } using var transactionScope = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled); - if (userTransaction is null) - { - await ExecuteAsync(migrationCommands, connection, executionState, beginTransaction: false, commitTransaction: !executeInTransaction, cancellationToken).ConfigureAwait(false); - } - else - { - await ExecuteAsync(migrationCommands, connection, executionState, beginTransaction: false, commitTransaction: false, cancellationToken).ConfigureAwait(false); - } + return await executionStrategy.ExecuteAsync( + (migrationCommands, connection, inUserTransaction, executionState, commitTransaction, isolationLevel), + static (_, s, ct) => ExecuteAsync( + s.migrationCommands, + s.connection, + s.executionState, + beginTransaction: !s.inUserTransaction, + commitTransaction: !s.inUserTransaction && s.commitTransaction, + s.isolationLevel, + ct), + verifySucceeded: null, + cancellationToken).ConfigureAwait(false); } - private static async Task ExecuteAsync( + private static async Task ExecuteAsync( IReadOnlyList migrationCommands, IRelationalConnection connection, MigrationExecutionState executionState, bool beginTransaction, bool commitTransaction, + System.Data.IsolationLevel isolationLevel, CancellationToken cancellationToken) { - var transaction = executionState.Transaction; - await connection.OpenAsync(cancellationToken).ConfigureAwait(false); + var result = 0; + var connectionOpened = await connection.OpenAsync(cancellationToken).ConfigureAwait(false); + Check.DebugAssert(!connectionOpened || executionState.Transaction == null, + "executionState.Transaction should be null"); + try { for (var i = executionState.LastCommittedCommandIndex; i < migrationCommands.Count; i++) { var command = migrationCommands[i]; - if (transaction == null + if (executionState.Transaction == null && !command.TransactionSuppressed && beginTransaction) { - transaction = await connection.BeginTransactionAsync(cancellationToken) + executionState.Transaction = await connection.BeginTransactionAsync(isolationLevel, cancellationToken) .ConfigureAwait(false); - executionState.Transaction = transaction; if (executionState.DatabaseLock != null) { - executionState.DatabaseLock = await executionState.DatabaseLock.ReacquireAsync(transaction, cancellationToken) + executionState.DatabaseLock = await executionState.DatabaseLock.RefreshAsync( + connectionOpened, transactionRestarted: true, cancellationToken) .ConfigureAwait(false); + connectionOpened = false; } } - if (transaction != null + if (executionState.Transaction != null && command.TransactionSuppressed) { - await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); - await transaction.DisposeAsync().ConfigureAwait(false); - transaction = null; + await executionState.Transaction.CommitAsync(cancellationToken).ConfigureAwait(false); + await executionState.Transaction.DisposeAsync().ConfigureAwait(false); executionState.Transaction = null; executionState.LastCommittedCommandIndex = i; + executionState.AnyOperationPerformed = true; + + if (executionState.DatabaseLock != null) + { + executionState.DatabaseLock = await executionState.DatabaseLock.RefreshAsync( + connectionOpened, transactionRestarted: null, cancellationToken) + .ConfigureAwait(false); + connectionOpened = false; + } } - await command.ExecuteNonQueryAsync(connection, cancellationToken: cancellationToken) + result = await command.ExecuteNonQueryAsync(connection, cancellationToken: cancellationToken) .ConfigureAwait(false); - if (transaction == null) + if (executionState.Transaction == null) { executionState.LastCommittedCommandIndex = i + 1; + executionState.AnyOperationPerformed = true; } } - if (transaction != null) + if (commitTransaction + && executionState.Transaction != null) { - await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); + await executionState.Transaction.CommitAsync(cancellationToken).ConfigureAwait(false); + await executionState.Transaction.DisposeAsync().ConfigureAwait(false); + executionState.Transaction = null; } } catch { - if (transaction != null) + if (executionState.Transaction != null) { - await transaction.DisposeAsync().ConfigureAwait(false); + await executionState.Transaction.DisposeAsync().ConfigureAwait(false); + executionState.Transaction = null; } await connection.CloseAsync().ConfigureAwait(false); - executionState.Transaction = null; throw; } - if (commitTransaction - && transaction != null) - { - await transaction.DisposeAsync().ConfigureAwait(false); - transaction = null; - executionState.Transaction = null; - } - await connection.CloseAsync().ConfigureAwait(false); - return transaction; + return result; } } diff --git a/src/EFCore.Relational/Migrations/Internal/Migrator.cs b/src/EFCore.Relational/Migrations/Internal/Migrator.cs index 75deea0d334..7c322e2150b 100644 --- a/src/EFCore.Relational/Migrations/Internal/Migrator.cs +++ b/src/EFCore.Relational/Migrations/Internal/Migrator.cs @@ -1,10 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Threading; +using System.Data; using Microsoft.EntityFrameworkCore.Diagnostics.Internal; -using Microsoft.EntityFrameworkCore.Storage; -using static Microsoft.EntityFrameworkCore.DbLoggerCategory.Database; namespace Microsoft.EntityFrameworkCore.Migrations.Internal; @@ -78,6 +76,14 @@ public Migrator( _executionStrategy = executionStrategy; } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + protected virtual IsolationLevel MigrationTransactionIsolationLevel => IsolationLevel.Unspecified; + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in @@ -86,7 +92,8 @@ public Migrator( /// public virtual void Migrate(string? targetMigration) { - if (_connection.CurrentTransaction is not null + var useTransaction = _connection.CurrentTransaction is null; + if (!useTransaction && _executionStrategy.RetriesOnFailure) { throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction); @@ -105,18 +112,40 @@ public virtual void Migrate(string? targetMigration) _databaseCreator.Create(); } + _connection.Open(); try { - _connection.Open(); - - if (!_historyRepository.Exists()) + var state = new MigrationExecutionState(); + if (_historyRepository.LockReleaseBehavior != LockReleaseBehavior.Transaction) { - _historyRepository.Create(); + state.DatabaseLock = _historyRepository.AcquireDatabaseLock(); } _executionStrategy.Execute( - (Migrator: this, TargetMigration: targetMigration, State: new MigrationExecutionState()), - (_, s) => s.Migrator.MigrateImplementation(s.TargetMigration, s.State), verifySucceeded: null); + this, + static (_, migrator) => + { + migrator._connection.Open(); + try + { + return migrator._historyRepository.CreateIfNotExists(); + } + finally + { + migrator._connection.Close(); + } + }, + verifySucceeded: null); + + _executionStrategy.Execute( + (Migrator: this, + TargetMigration: targetMigration, + State: state, + UseTransaction: useTransaction), + static (c, s) => s.Migrator.MigrateImplementation(c, s.TargetMigration, s.State, s.UseTransaction), + static (_, s) => new ExecutionResult( + successful: s.Migrator.VerifyMigrationSucceeded(s.TargetMigration, s.State), + result: true)); } finally { @@ -124,53 +153,65 @@ public virtual void Migrate(string? targetMigration) } } - private bool MigrateImplementation(string? targetMigration, MigrationExecutionState state) + private bool MigrateImplementation( + DbContext context, string? targetMigration, MigrationExecutionState state, bool useTransaction) { - _connection.Open(); - var context = _currentContext.Context; - using var transaction = context.Database.BeginTransaction(); - state.DatabaseLock = state.DatabaseLock == null - ? _historyRepository.AcquireDatabaseLock(transaction) - : state.DatabaseLock.Reacquire(transaction); - - PopulateMigrations( - _historyRepository.GetAppliedMigrations().Select(t => t.MigrationId), - targetMigration, - out var migratorData); - - var commandLists = GetMigrationCommandLists(migratorData); - foreach (var commandList in commandLists) + var connectionOpened = _connection.Open(); + try { - var (id, getCommands) = commandList; + if (useTransaction) + { + state.Transaction = _connection.BeginTransaction(MigrationTransactionIsolationLevel); + } + + state.DatabaseLock = state.DatabaseLock == null + ? _historyRepository.AcquireDatabaseLock() + : state.DatabaseLock.Refresh(connectionOpened, useTransaction); - if (id != state.CurrentMigrationId) + PopulateMigrations( + _historyRepository.GetAppliedMigrations().Select(t => t.MigrationId), + targetMigration, + out var migratorData); + + var commandLists = GetMigrationCommandLists(migratorData); + foreach (var commandList in commandLists) { - state.CurrentMigrationId = id; - state.LastCommittedCommandIndex = 0; + var (id, getCommands) = commandList; + if (id != state.CurrentMigrationId) + { + state.CurrentMigrationId = id; + state.LastCommittedCommandIndex = 0; + } + + _migrationCommandExecutor.ExecuteNonQuery( + getCommands(), _connection, state, commitTransaction: false, MigrationTransactionIsolationLevel); } - _migrationCommandExecutor.ExecuteNonQuery(getCommands(), _connection); - } + var coreOptionsExtension = + _contextOptions.FindExtension() + ?? new CoreOptionsExtension(); - var coreOptionsExtension = - _contextOptions.FindExtension() - ?? new CoreOptionsExtension(); + var seed = coreOptionsExtension.Seeder; + if (seed != null) + { + seed(context, state.AnyOperationPerformed); + } + else if (coreOptionsExtension.AsyncSeeder != null) + { + throw new InvalidOperationException(CoreStrings.MissingSeeder); + } - var seed = coreOptionsExtension.Seeder; - if (seed != null) - { - var operationsPerformed = migratorData.AppliedMigrations.Count != 0 - || migratorData.RevertedMigrations.Count != 0; - seed(context, operationsPerformed); + state.Transaction?.Commit(); + return state.AnyOperationPerformed; } - else if (coreOptionsExtension.AsyncSeeder != null) + finally { - throw new InvalidOperationException(CoreStrings.MissingSeeder); + state.DatabaseLock?.Dispose(); + state.DatabaseLock = null; + state.Transaction?.Dispose(); + state.Transaction = null; + _connection.Close(); } - - transaction.Commit(); - _connection.Close(); - return true; } /// @@ -183,7 +224,8 @@ public virtual async Task MigrateAsync( string? targetMigration, CancellationToken cancellationToken = default) { - if (_connection.CurrentTransaction is not null + var useTransaction = _connection.CurrentTransaction is null; + if (!useTransaction && _executionStrategy.RetriesOnFailure) { throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction); @@ -202,76 +244,120 @@ public virtual async Task MigrateAsync( await _databaseCreator.CreateAsync(cancellationToken).ConfigureAwait(false); } + await _connection.OpenAsync(cancellationToken).ConfigureAwait(false); try { - await _connection.OpenAsync(cancellationToken).ConfigureAwait(false); - - if (!await _historyRepository.ExistsAsync(cancellationToken).ConfigureAwait(false)) + var state = new MigrationExecutionState(); + if (_historyRepository.LockReleaseBehavior != LockReleaseBehavior.Transaction) { - await _historyRepository.CreateAsync(cancellationToken).ConfigureAwait(false); + state.DatabaseLock = await _historyRepository.AcquireDatabaseLockAsync(cancellationToken).ConfigureAwait(false); } await _executionStrategy.ExecuteAsync( - (Migrator: this, TargetMigration: targetMigration, State: new MigrationExecutionState()), - async (_, s, ct) => await s.Migrator.MigrateImplementationAsync(s.TargetMigration, s.State, ct).ConfigureAwait(false), verifySucceeded: null, cancellationToken) + this, + static async (_, migrator, ct) => + { + await migrator._connection.OpenAsync(ct).ConfigureAwait(false); + try + { + return await migrator._historyRepository.CreateIfNotExistsAsync(ct).ConfigureAwait(false); + } + finally + { + await migrator._connection.CloseAsync().ConfigureAwait(false); + } + }, + verifySucceeded: null, + cancellationToken).ConfigureAwait(false); + + await _executionStrategy.ExecuteAsync( + (Migrator: this, + TargetMigration: targetMigration, + State: state, + UseTransaction: useTransaction), + async static (c, s, ct) => await s.Migrator.MigrateImplementationAsync( + c, s.TargetMigration, s.State, s.UseTransaction, ct).ConfigureAwait(false), + async static (_, s, ct) => new ExecutionResult( + successful: await s.Migrator.VerifyMigrationSucceededAsync(s.TargetMigration, s.State, ct).ConfigureAwait(false), + result: true), + cancellationToken) .ConfigureAwait(false); } finally { - _connection.Close(); + await _connection.CloseAsync().ConfigureAwait(false); } } - private async Task MigrateImplementationAsync(string? targetMigration, MigrationExecutionState state, CancellationToken cancellationToken = default) + private async Task MigrateImplementationAsync( + DbContext context, string? targetMigration, MigrationExecutionState state, bool useTransaction, CancellationToken cancellationToken = default) { - await _connection.OpenAsync(cancellationToken).ConfigureAwait(false); - var context = _currentContext.Context; - using var transaction = await context.Database.BeginTransactionAsync(cancellationToken).ConfigureAwait(false); - await using var _ = transaction.ConfigureAwait(false); - - state.DatabaseLock = state.DatabaseLock == null - ? await _historyRepository.AcquireDatabaseLockAsync(transaction, cancellationToken).ConfigureAwait(false) - : await state.DatabaseLock.ReacquireAsync(transaction, cancellationToken).ConfigureAwait(false); + var connectionOpened = await _connection.OpenAsync(cancellationToken).ConfigureAwait(false); + try + { + if (useTransaction) + { + state.Transaction = await context.Database.BeginTransactionAsync(MigrationTransactionIsolationLevel, cancellationToken).ConfigureAwait(false); + } - PopulateMigrations( - (await _historyRepository.GetAppliedMigrationsAsync(cancellationToken).ConfigureAwait(false)).Select(t => t.MigrationId), - targetMigration, - out var migratorData); + state.DatabaseLock = state.DatabaseLock == null + ? await _historyRepository.AcquireDatabaseLockAsync(cancellationToken).ConfigureAwait(false) + : await state.DatabaseLock.RefreshAsync(connectionOpened, useTransaction, cancellationToken).ConfigureAwait(false); - var commandLists = GetMigrationCommandLists(migratorData); - foreach (var commandList in commandLists) - { - var (id, getCommands) = commandList; + PopulateMigrations( + (await _historyRepository.GetAppliedMigrationsAsync(cancellationToken).ConfigureAwait(false)).Select(t => t.MigrationId), + targetMigration, + out var migratorData); - if (id != state.CurrentMigrationId) + var commandLists = GetMigrationCommandLists(migratorData); + foreach (var commandList in commandLists) { - state.CurrentMigrationId = id; - state.LastCommittedCommandIndex = 0; + var (id, getCommands) = commandList; + if (id != state.CurrentMigrationId) + { + state.CurrentMigrationId = id; + state.LastCommittedCommandIndex = 0; + } + + await _migrationCommandExecutor.ExecuteNonQueryAsync( + getCommands(), _connection, state, commitTransaction: false, MigrationTransactionIsolationLevel, cancellationToken) + .ConfigureAwait(false); } - await _migrationCommandExecutor.ExecuteNonQueryAsync(getCommands(), _connection, cancellationToken) - .ConfigureAwait(false); - } + var coreOptionsExtension = + _contextOptions.FindExtension() + ?? new CoreOptionsExtension(); - var coreOptionsExtension = - _contextOptions.FindExtension() - ?? new CoreOptionsExtension(); + var seedAsync = coreOptionsExtension.AsyncSeeder; + if (seedAsync != null) + { + await seedAsync(context, state.AnyOperationPerformed, cancellationToken).ConfigureAwait(false); + } + else if (coreOptionsExtension.Seeder != null) + { + throw new InvalidOperationException(CoreStrings.MissingSeeder); + } - var seedAsync = coreOptionsExtension.AsyncSeeder; - if (seedAsync != null) - { - var operationsPerformed = migratorData.AppliedMigrations.Count != 0 - || migratorData.RevertedMigrations.Count != 0; - await seedAsync(context, operationsPerformed, cancellationToken).ConfigureAwait(false); + if (state.Transaction != null) + { + await state.Transaction.CommitAsync(cancellationToken).ConfigureAwait(false); + } + return state.AnyOperationPerformed; } - else if (coreOptionsExtension.Seeder != null) + finally { - throw new InvalidOperationException(CoreStrings.MissingSeeder); + if (state.DatabaseLock != null) + { + state.DatabaseLock.Dispose(); + state.DatabaseLock = null; + } + if (state.Transaction != null) + { + await state.Transaction.DisposeAsync().ConfigureAwait(false); + state.Transaction = null; + } + await _connection.CloseAsync().ConfigureAwait(false); } - - await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); - _connection.Close(); - return true; } private IEnumerable<(string, Func>)> GetMigrationCommandLists(MigratorData parameters) @@ -399,6 +485,26 @@ protected virtual void PopulateMigrations( parameters = new MigratorData(migrationsToApply, migrationsToRevert, actualTargetMigration); } + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + protected virtual bool VerifyMigrationSucceeded( + string? targetMigration, MigrationExecutionState state) + => false; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + protected virtual Task VerifyMigrationSucceededAsync( + string? targetMigration, MigrationExecutionState state, CancellationToken cancellationToken) + => Task.FromResult(false); + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore.Relational/Migrations/LockReleaseBehavior.cs b/src/EFCore.Relational/Migrations/LockReleaseBehavior.cs new file mode 100644 index 00000000000..bd1f96e995c --- /dev/null +++ b/src/EFCore.Relational/Migrations/LockReleaseBehavior.cs @@ -0,0 +1,25 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.EntityFrameworkCore.Migrations; + +/// +/// Represents the conditions under which a lock is released implicitly. +/// +public enum LockReleaseBehavior +{ + /// + /// The lock is released when the transaction is committed or rolled back. + /// + Transaction, + + /// + /// The lock is released when the connection is closed. + /// + Connection, + + /// + /// The lock can only be released explicitly. + /// + Explicit +} diff --git a/src/EFCore.Relational/Migrations/MigrationExecutionState.cs b/src/EFCore.Relational/Migrations/MigrationExecutionState.cs index e3769da5e25..accd83e0cfc 100644 --- a/src/EFCore.Relational/Migrations/MigrationExecutionState.cs +++ b/src/EFCore.Relational/Migrations/MigrationExecutionState.cs @@ -18,6 +18,11 @@ public sealed class MigrationExecutionState /// public string? CurrentMigrationId { get; set; } + /// + /// Indicates whether any migration operation was performed. + /// + public bool AnyOperationPerformed { get; set; } + /// /// The database lock that is in use. /// diff --git a/src/EFCore.Relational/Storage/RelationalDatabaseCreator.cs b/src/EFCore.Relational/Storage/RelationalDatabaseCreator.cs index ad6a2bcd7df..96f5ca0f76a 100644 --- a/src/EFCore.Relational/Storage/RelationalDatabaseCreator.cs +++ b/src/EFCore.Relational/Storage/RelationalDatabaseCreator.cs @@ -116,7 +116,7 @@ public virtual Task DeleteAsync(CancellationToken cancellationToken = default) /// to incrementally update the schema. It is assumed that none of the tables exist in the database. /// public virtual void CreateTables() - => Dependencies.MigrationCommandExecutor.ExecuteNonQuery(GetCreateTablesCommands(), Dependencies.Connection); + => Dependencies.MigrationCommandExecutor.ExecuteNonQuery(GetCreateTablesCommands(), Dependencies.Connection, new MigrationExecutionState(), commitTransaction: true); /// /// Asynchronously creates all tables for the current model in the database. No attempt is made @@ -129,7 +129,7 @@ public virtual void CreateTables() /// If the is canceled. public virtual Task CreateTablesAsync(CancellationToken cancellationToken = default) => Dependencies.MigrationCommandExecutor.ExecuteNonQueryAsync( - GetCreateTablesCommands(), Dependencies.Connection, cancellationToken); + GetCreateTablesCommands(), Dependencies.Connection, new MigrationExecutionState(), commitTransaction: true, cancellationToken: cancellationToken); /// /// Gets the commands that will create all tables from the model. diff --git a/src/EFCore.SqlServer/Migrations/Internal/SqlServerHistoryRepository.cs b/src/EFCore.SqlServer/Migrations/Internal/SqlServerHistoryRepository.cs index 679fb5dd6be..202c7a2c131 100644 --- a/src/EFCore.SqlServer/Migrations/Internal/SqlServerHistoryRepository.cs +++ b/src/EFCore.SqlServer/Migrations/Internal/SqlServerHistoryRepository.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Data; using System.Text; namespace Microsoft.EntityFrameworkCore.SqlServer.Migrations.Internal; @@ -60,7 +59,15 @@ protected override bool InterpretExistsResult(object? value) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public override IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction) + public override LockReleaseBehavior LockReleaseBehavior => LockReleaseBehavior.Connection; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public override IMigrationsDatabaseLock AcquireDatabaseLock() { Dependencies.MigrationsLogger.AcquiringMigrationLock(); @@ -94,7 +101,7 @@ public override IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransactio /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public override async Task AcquireDatabaseLockAsync(IDbContextTransaction transaction, CancellationToken cancellationToken = default) + public override async Task AcquireDatabaseLockAsync(CancellationToken cancellationToken = default) { Dependencies.MigrationsLogger.AcquiringMigrationLock(); @@ -140,7 +147,8 @@ private SqlServerMigrationDatabaseLock CreateMigrationDatabaseLock() EXEC @result = sp_releaseapplock @Resource = '__EFMigrationsLock', @LockOwner = 'Session'; SELECT @result """), - CreateRelationalCommandParameters()); + CreateRelationalCommandParameters(), + this); private RelationalCommandParameterObject CreateRelationalCommandParameters() => new( diff --git a/src/EFCore.SqlServer/Migrations/Internal/SqlServerMigrationDatabaseLock.cs b/src/EFCore.SqlServer/Migrations/Internal/SqlServerMigrationDatabaseLock.cs index c069413fa34..c590fa9bb74 100644 --- a/src/EFCore.SqlServer/Migrations/Internal/SqlServerMigrationDatabaseLock.cs +++ b/src/EFCore.SqlServer/Migrations/Internal/SqlServerMigrationDatabaseLock.cs @@ -18,6 +18,7 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Migrations.Internal; public class SqlServerMigrationDatabaseLock( IRelationalCommand relationalCommand, RelationalCommandParameterObject relationalCommandParameters, + IHistoryRepository historyRepository, CancellationToken cancellationToken = default) : IMigrationsDatabaseLock { @@ -27,8 +28,7 @@ public class SqlServerMigrationDatabaseLock( /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public void Dispose() - => relationalCommand.ExecuteScalar(relationalCommandParameters); + public virtual IHistoryRepository HistoryRepository => historyRepository; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -36,17 +36,8 @@ public void Dispose() /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public async ValueTask DisposeAsync() - => await relationalCommand.ExecuteScalarAsync(relationalCommandParameters, cancellationToken).ConfigureAwait(false); - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public IMigrationsDatabaseLock Reacquire(IDbContextTransaction? transaction) - => this; + public void Dispose() + => relationalCommand.ExecuteScalar(relationalCommandParameters); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -54,6 +45,6 @@ public IMigrationsDatabaseLock Reacquire(IDbContextTransaction? transaction) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public Task ReacquireAsync(IDbContextTransaction? transaction, CancellationToken cancellationToken = default) - => Task.FromResult((IMigrationsDatabaseLock)this); + public async ValueTask DisposeAsync() + => await relationalCommand.ExecuteScalarAsync(relationalCommandParameters, cancellationToken).ConfigureAwait(false); } diff --git a/src/EFCore.SqlServer/Storage/Internal/SqlServerDatabaseCreator.cs b/src/EFCore.SqlServer/Storage/Internal/SqlServerDatabaseCreator.cs index 589440790a1..e3f3fcdb838 100644 --- a/src/EFCore.SqlServer/Storage/Internal/SqlServerDatabaseCreator.cs +++ b/src/EFCore.SqlServer/Storage/Internal/SqlServerDatabaseCreator.cs @@ -14,26 +14,19 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal; /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// -public class SqlServerDatabaseCreator : RelationalDatabaseCreator +/// +/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to +/// the same compatibility standards as public APIs. It may be changed or removed without notice in +/// any release. You should only use it directly in your code with extreme caution and knowing that +/// doing so can result in application failures when updating to a new Entity Framework Core release. +/// +public class SqlServerDatabaseCreator( + RelationalDatabaseCreatorDependencies dependencies, + ISqlServerConnection connection, + IRawSqlCommandBuilder rawSqlCommandBuilder) : RelationalDatabaseCreator(dependencies) { - private readonly ISqlServerConnection _connection; - private readonly IRawSqlCommandBuilder _rawSqlCommandBuilder; - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public SqlServerDatabaseCreator( - RelationalDatabaseCreatorDependencies dependencies, - ISqlServerConnection connection, - IRawSqlCommandBuilder rawSqlCommandBuilder) - : base(dependencies) - { - _connection = connection; - _rawSqlCommandBuilder = rawSqlCommandBuilder; - } + private readonly ISqlServerConnection _connection = connection; + private readonly IRawSqlCommandBuilder _rawSqlCommandBuilder = rawSqlCommandBuilder; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -62,7 +55,7 @@ public override void Create() using (var masterConnection = _connection.CreateMasterConnection()) { Dependencies.MigrationCommandExecutor - .ExecuteNonQuery(CreateCreateOperations(), masterConnection); + .ExecuteNonQuery(CreateCreateOperations(), masterConnection, new MigrationExecutionState(), commitTransaction: true); ClearPool(); } @@ -82,7 +75,7 @@ public override async Task CreateAsync(CancellationToken cancellationToken = def await using (masterConnection.ConfigureAwait(false)) { await Dependencies.MigrationCommandExecutor - .ExecuteNonQueryAsync(CreateCreateOperations(), masterConnection, cancellationToken) + .ExecuteNonQueryAsync(CreateCreateOperations(), masterConnection, new MigrationExecutionState(), commitTransaction: true, cancellationToken: cancellationToken) .ConfigureAwait(false); ClearPool(); @@ -157,8 +150,7 @@ private IReadOnlyList CreateCreateOperations() { var builder = new SqlConnectionStringBuilder(_connection.DbConnection.ConnectionString); return Dependencies.MigrationsSqlGenerator.Generate( - new[] - { + [ new SqlServerCreateDatabaseOperation { Name = builder.InitialCatalog, @@ -166,7 +158,7 @@ private IReadOnlyList CreateCreateOperations() Collation = Dependencies.CurrentContext.Context.GetService() .Model.GetRelationalModel().Collation } - }); + ]); } /// @@ -345,7 +337,7 @@ public override void Delete() using var masterConnection = _connection.CreateMasterConnection(); Dependencies.MigrationCommandExecutor - .ExecuteNonQuery(CreateDropCommands(), masterConnection); + .ExecuteNonQuery(CreateDropCommands(), masterConnection, new MigrationExecutionState(), commitTransaction: true); } /// @@ -361,7 +353,7 @@ public override async Task DeleteAsync(CancellationToken cancellationToken = def var masterConnection = _connection.CreateMasterConnection(); await using var _ = masterConnection.ConfigureAwait(false); await Dependencies.MigrationCommandExecutor - .ExecuteNonQueryAsync(CreateDropCommands(), masterConnection, cancellationToken) + .ExecuteNonQueryAsync(CreateDropCommands(), masterConnection, new MigrationExecutionState(), commitTransaction: true, cancellationToken: cancellationToken) .ConfigureAwait(false); } diff --git a/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteHistoryRepository.cs b/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteHistoryRepository.cs index 332e8e4f402..c72c0ff788d 100644 --- a/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteHistoryRepository.cs +++ b/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteHistoryRepository.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Data; using Microsoft.EntityFrameworkCore.Sqlite.Internal; namespace Microsoft.EntityFrameworkCore.Sqlite.Migrations.Internal; @@ -104,7 +103,15 @@ public override string GetEndIfScript() /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public override IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction) + public override LockReleaseBehavior LockReleaseBehavior => LockReleaseBehavior.Explicit; + + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public override IMigrationsDatabaseLock AcquireDatabaseLock() { Dependencies.MigrationsLogger.AcquiringMigrationLock(); @@ -143,7 +150,7 @@ public override IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransactio /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public override async Task AcquireDatabaseLockAsync( - IDbContextTransaction transaction, CancellationToken cancellationToken = default) + CancellationToken cancellationToken = default) { Dependencies.MigrationsLogger.AcquiringMigrationLock(); @@ -212,7 +219,7 @@ DELETE FROM "{LockTableName}" } private SqliteMigrationDatabaseLock CreateMigrationDatabaseLock() - => new(CreateDeleteLockCommand(), CreateRelationalCommandParameters()); + => new(CreateDeleteLockCommand(), CreateRelationalCommandParameters(), this); private RelationalCommandParameterObject CreateRelationalCommandParameters() => new( diff --git a/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteMigrationDatabaseLock.cs b/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteMigrationDatabaseLock.cs index 062dc1270b6..cec6f48c279 100644 --- a/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteMigrationDatabaseLock.cs +++ b/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteMigrationDatabaseLock.cs @@ -1,7 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. - namespace Microsoft.EntityFrameworkCore.Sqlite.Migrations.Internal; /// @@ -13,6 +12,7 @@ namespace Microsoft.EntityFrameworkCore.Sqlite.Migrations.Internal; public class SqliteMigrationDatabaseLock( IRelationalCommand relationalCommand, RelationalCommandParameterObject relationalCommandParameters, + IHistoryRepository historyRepository, CancellationToken cancellationToken = default) : IMigrationsDatabaseLock { @@ -22,17 +22,7 @@ public class SqliteMigrationDatabaseLock( /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public void Dispose() - => relationalCommand.ExecuteScalar(relationalCommandParameters); - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public async ValueTask DisposeAsync() - => await relationalCommand.ExecuteScalarAsync(relationalCommandParameters, cancellationToken).ConfigureAwait(false); + public virtual IHistoryRepository HistoryRepository => historyRepository; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -40,8 +30,8 @@ public async ValueTask DisposeAsync() /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public IMigrationsDatabaseLock Reacquire(IDbContextTransaction? transaction) - => this; + public void Dispose() + => relationalCommand.ExecuteScalar(relationalCommandParameters); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -49,6 +39,6 @@ public IMigrationsDatabaseLock Reacquire(IDbContextTransaction? transaction) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public Task ReacquireAsync(IDbContextTransaction? transaction, CancellationToken cancellationToken = default) - => Task.FromResult((IMigrationsDatabaseLock)this); + public async ValueTask DisposeAsync() + => await relationalCommand.ExecuteScalarAsync(relationalCommandParameters, cancellationToken).ConfigureAwait(false); } diff --git a/test/EFCore.Design.Tests/Design/DesignTimeServicesTest.cs b/test/EFCore.Design.Tests/Design/DesignTimeServicesTest.cs index caf478b65ce..ee13f8d8c34 100644 --- a/test/EFCore.Design.Tests/Design/DesignTimeServicesTest.cs +++ b/test/EFCore.Design.Tests/Design/DesignTimeServicesTest.cs @@ -192,6 +192,8 @@ public bool IsValidId(string value) public class ExtensionHistoryRepository : IHistoryRepository { + public virtual LockReleaseBehavior LockReleaseBehavior => LockReleaseBehavior.Explicit; + public void Create() => throw new NotImplementedException(); @@ -222,10 +224,10 @@ public string GetCreateIfNotExistsScript() public string GetCreateScript() => throw new NotImplementedException(); - public IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction) + public IMigrationsDatabaseLock AcquireDatabaseLock() => throw new NotImplementedException(); - public Task AcquireDatabaseLockAsync(IDbContextTransaction transaction, CancellationToken cancellationToken = default) + public Task AcquireDatabaseLockAsync(CancellationToken cancellationToken = default) => throw new NotImplementedException(); public string GetDeleteScript(string migrationId) @@ -264,6 +266,8 @@ public bool IsValidId(string value) public class ContextHistoryRepository : IHistoryRepository { + public virtual LockReleaseBehavior LockReleaseBehavior => LockReleaseBehavior.Explicit; + public bool Exists() => throw new NotImplementedException(); @@ -294,10 +298,10 @@ public void Create() public Task CreateAsync(CancellationToken cancellationToken = default) => throw new NotImplementedException(); - public IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction) + public IMigrationsDatabaseLock AcquireDatabaseLock() => throw new NotImplementedException(); - public Task AcquireDatabaseLockAsync(IDbContextTransaction transaction, CancellationToken cancellationToken = default) + public Task AcquireDatabaseLockAsync(CancellationToken cancellationToken = default) => throw new NotImplementedException(); public string GetDeleteScript(string migrationId) diff --git a/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs b/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs index d069fe3e7ed..45f138e904b 100644 --- a/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs +++ b/test/EFCore.Design.Tests/Migrations/Design/MigrationScaffolderTest.cs @@ -144,6 +144,8 @@ protected override void BuildModel(ModelBuilder modelBuilder) private class MockHistoryRepository : IHistoryRepository { + public virtual LockReleaseBehavior LockReleaseBehavior => LockReleaseBehavior.Explicit; + public string GetBeginIfExistsScript(string migrationId) => null; @@ -183,10 +185,10 @@ public void Create() public Task CreateAsync(CancellationToken cancellationToken = default) => throw new NotImplementedException(); - public IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction) + public IMigrationsDatabaseLock AcquireDatabaseLock() => throw new NotImplementedException(); - public Task AcquireDatabaseLockAsync(IDbContextTransaction transaction, CancellationToken cancellationToken = default) + public Task AcquireDatabaseLockAsync(CancellationToken cancellationToken = default) => throw new NotImplementedException(); } diff --git a/test/EFCore.Relational.Tests/Extensions/RelationalDatabaseFacadeExtensionsTest.cs b/test/EFCore.Relational.Tests/Extensions/RelationalDatabaseFacadeExtensionsTest.cs index adf167e3cc8..f2d2a935de3 100644 --- a/test/EFCore.Relational.Tests/Extensions/RelationalDatabaseFacadeExtensionsTest.cs +++ b/test/EFCore.Relational.Tests/Extensions/RelationalDatabaseFacadeExtensionsTest.cs @@ -321,6 +321,8 @@ protected override void BuildModel(ModelBuilder modelBuilder) private class FakeHistoryRepository : IHistoryRepository { + public virtual LockReleaseBehavior LockReleaseBehavior => LockReleaseBehavior.Explicit; + public List AppliedMigrations { get; set; } public IReadOnlyList GetAppliedMigrations() @@ -362,10 +364,10 @@ public void Create() public Task CreateAsync(CancellationToken cancellationToken = default) => throw new NotImplementedException(); - public IMigrationsDatabaseLock AcquireDatabaseLock(IDbContextTransaction transaction) + public IMigrationsDatabaseLock AcquireDatabaseLock() => throw new NotImplementedException(); - public Task AcquireDatabaseLockAsync(IDbContextTransaction transaction, CancellationToken cancellationToken = default) + public Task AcquireDatabaseLockAsync(CancellationToken cancellationToken = default) => throw new NotImplementedException(); } diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsInfrastructureSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsInfrastructureSqlServerTest.cs index 8aeb88a4e31..b487dc8c777 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsInfrastructureSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsInfrastructureSqlServerTest.cs @@ -1017,13 +1017,14 @@ SELECT 1 EXEC @result = sp_getapplock @Resource = '__EFMigrationsLock', @LockOwner = 'Session', @LockMode = 'Exclusive'; SELECT @result -SELECT OBJECT_ID(N'[__EFMigrationsHistory]'); - -CREATE TABLE [__EFMigrationsHistory] ( - [MigrationId] nvarchar(150) NOT NULL, - [ProductVersion] nvarchar(32) NOT NULL, - CONSTRAINT [PK___EFMigrationsHistory] PRIMARY KEY ([MigrationId]) -); +IF OBJECT_ID(N'[__EFMigrationsHistory]') IS NULL +BEGIN + CREATE TABLE [__EFMigrationsHistory] ( + [MigrationId] nvarchar(150) NOT NULL, + [ProductVersion] nvarchar(32) NOT NULL, + CONSTRAINT [PK___EFMigrationsHistory] PRIMARY KEY ([MigrationId]) + ); +END; SELECT OBJECT_ID(N'[__EFMigrationsHistory]'); @@ -1047,6 +1048,20 @@ CONSTRAINT [PK_Blogs] PRIMARY KEY ([Id]) THROW 65536, 'Test', 0; END +DECLARE @result int; +EXEC @result = sp_releaseapplock @Resource = '__EFMigrationsLock', @LockOwner = 'Session'; +SELECT @result + +DECLARE @result int; +EXEC @result = sp_getapplock @Resource = '__EFMigrationsLock', @LockOwner = 'Session', @LockMode = 'Exclusive'; +SELECT @result + +SELECT OBJECT_ID(N'[__EFMigrationsHistory]'); + +SELECT [MigrationId], [ProductVersion] +FROM [__EFMigrationsHistory] +ORDER BY [MigrationId]; + IF OBJECT_ID(N'Blogs', N'U') IS NULL BEGIN CREATE TABLE [Blogs] ( @@ -1110,13 +1125,14 @@ SELECT 1 EXEC @result = sp_getapplock @Resource = '__EFMigrationsLock', @LockOwner = 'Session', @LockMode = 'Exclusive'; SELECT @result -SELECT OBJECT_ID(N'[__EFMigrationsHistory]'); - -CREATE TABLE [__EFMigrationsHistory] ( - [MigrationId] nvarchar(150) NOT NULL, - [ProductVersion] nvarchar(32) NOT NULL, - CONSTRAINT [PK___EFMigrationsHistory] PRIMARY KEY ([MigrationId]) -); +IF OBJECT_ID(N'[__EFMigrationsHistory]') IS NULL +BEGIN + CREATE TABLE [__EFMigrationsHistory] ( + [MigrationId] nvarchar(150) NOT NULL, + [ProductVersion] nvarchar(32) NOT NULL, + CONSTRAINT [PK___EFMigrationsHistory] PRIMARY KEY ([MigrationId]) + ); +END; SELECT OBJECT_ID(N'[__EFMigrationsHistory]'); @@ -1140,6 +1156,20 @@ CONSTRAINT [PK_Blogs] PRIMARY KEY ([Id]) THROW 65536, 'Test', 0; END +DECLARE @result int; +EXEC @result = sp_releaseapplock @Resource = '__EFMigrationsLock', @LockOwner = 'Session'; +SELECT @result + +DECLARE @result int; +EXEC @result = sp_getapplock @Resource = '__EFMigrationsLock', @LockOwner = 'Session', @LockMode = 'Exclusive'; +SELECT @result + +SELECT OBJECT_ID(N'[__EFMigrationsHistory]'); + +SELECT [MigrationId], [ProductVersion] +FROM [__EFMigrationsHistory] +ORDER BY [MigrationId]; + IF OBJECT_ID(N'Blogs', N'U') IS NULL BEGIN CREATE TABLE [Blogs] ( @@ -2078,7 +2108,8 @@ IF EXISTS(select * from sys.databases where name='TransactionSuppressed') public override MigrationsContext CreateContext() { var options = AddOptions(TestStore.AddProviderOptions(new DbContextOptionsBuilder())) - .UseSqlServer(TestStore.ConnectionString, b => b.ApplyConfiguration()) + .UseSqlServer(TestStore.ConnectionString, b => b + .ApplyConfiguration()) .UseInternalServiceProvider(ServiceProvider) .Options; return new MigrationsContext(options); diff --git a/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestSqlServerRetryingExecutionStrategy.cs b/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestSqlServerRetryingExecutionStrategy.cs index ec517ff802f..ac0eea2e54c 100644 --- a/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestSqlServerRetryingExecutionStrategy.cs +++ b/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestSqlServerRetryingExecutionStrategy.cs @@ -43,6 +43,13 @@ public TestSqlServerRetryingExecutionStrategy(ExecutionStrategyDependencies depe { } + public TestSqlServerRetryingExecutionStrategy( + ExecutionStrategyDependencies dependencies, + IEnumerable errorNumbersToAdd) + : base(dependencies, DefaultMaxRetryCount, DefaultMaxDelay, _additionalErrorNumbers.Concat(errorNumbersToAdd)) + { + } + protected override bool ShouldRetryOn(Exception exception) { if (base.ShouldRetryOn(exception)) From af0968b8934d331deef51ac5c34057381d007b69 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Thu, 29 Aug 2024 12:36:08 -0700 Subject: [PATCH 3/4] More review feedback Fixes #34569 --- .../Migrations/HistoryRepository.cs | 36 ++++++------ .../Migrations/IMigrationCommandExecutor.cs | 4 +- .../Migrations/IMigrationsDatabaseLock.cs | 4 +- .../Internal/MigrationCommandExecutor.cs | 56 ++++++++++--------- .../Migrations/Internal/Migrator.cs | 23 +++++--- .../SqlServerMigrationDatabaseLock.cs | 6 +- .../Internal/SqliteHistoryRepository.cs | 4 -- .../Internal/SqliteMigrationDatabaseLock.cs | 6 +- .../MigrationsInfrastructureSqlServerTest.cs | 8 +++ 9 files changed, 83 insertions(+), 64 deletions(-) diff --git a/src/EFCore.Relational/Migrations/HistoryRepository.cs b/src/EFCore.Relational/Migrations/HistoryRepository.cs index 0e233377f7d..bfb565786c0 100644 --- a/src/EFCore.Relational/Migrations/HistoryRepository.cs +++ b/src/EFCore.Relational/Migrations/HistoryRepository.cs @@ -128,14 +128,15 @@ protected virtual string ProductVersionColumnName /// /// if the table already exists, otherwise. public virtual bool Exists() - => InterpretExistsResult( - Dependencies.RawSqlCommandBuilder.Build(ExistsSql).ExecuteScalar( - new RelationalCommandParameterObject( - Dependencies.Connection, - null, - null, - Dependencies.CurrentContext.Context, - Dependencies.CommandLogger, CommandSource.Migrations))); + => Dependencies.DatabaseCreator.Exists() + && InterpretExistsResult( + Dependencies.RawSqlCommandBuilder.Build(ExistsSql).ExecuteScalar( + new RelationalCommandParameterObject( + Dependencies.Connection, + null, + null, + Dependencies.CurrentContext.Context, + Dependencies.CommandLogger, CommandSource.Migrations))); /// /// Checks whether or not the history table exists. @@ -147,15 +148,16 @@ public virtual bool Exists() /// /// If the is canceled. public virtual async Task ExistsAsync(CancellationToken cancellationToken = default) - => InterpretExistsResult( - await Dependencies.RawSqlCommandBuilder.Build(ExistsSql).ExecuteScalarAsync( - new RelationalCommandParameterObject( - Dependencies.Connection, - null, - null, - Dependencies.CurrentContext.Context, - Dependencies.CommandLogger, CommandSource.Migrations), - cancellationToken).ConfigureAwait(false)); + => await Dependencies.DatabaseCreator.ExistsAsync(cancellationToken).ConfigureAwait(false) + && InterpretExistsResult( + await Dependencies.RawSqlCommandBuilder.Build(ExistsSql).ExecuteScalarAsync( + new RelationalCommandParameterObject( + Dependencies.Connection, + null, + null, + Dependencies.CurrentContext.Context, + Dependencies.CommandLogger, CommandSource.Migrations), + cancellationToken).ConfigureAwait(false)); /// /// Interprets the result of executing . diff --git a/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs b/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs index 0b18e7e84b7..039bbaa4f15 100644 --- a/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs +++ b/src/EFCore.Relational/Migrations/IMigrationCommandExecutor.cs @@ -46,7 +46,7 @@ int ExecuteNonQuery( IRelationalConnection connection, MigrationExecutionState executionState, bool commitTransaction, - IsolationLevel isolationLevel = IsolationLevel.Unspecified); + IsolationLevel? isolationLevel = null); /// /// Executes the given commands using the given database connection. @@ -80,6 +80,6 @@ Task ExecuteNonQueryAsync( IRelationalConnection connection, MigrationExecutionState executionState, bool commitTransaction, - IsolationLevel isolationLevel = IsolationLevel.Unspecified, + IsolationLevel? isolationLevel = null, CancellationToken cancellationToken = default); } diff --git a/src/EFCore.Relational/Migrations/IMigrationsDatabaseLock.cs b/src/EFCore.Relational/Migrations/IMigrationsDatabaseLock.cs index 9e1f0cc7803..b08ea1b9faa 100644 --- a/src/EFCore.Relational/Migrations/IMigrationsDatabaseLock.cs +++ b/src/EFCore.Relational/Migrations/IMigrationsDatabaseLock.cs @@ -25,7 +25,7 @@ public interface IMigrationsDatabaseLock : IDisposable, IAsyncDisposable /// if there's no current transaction. /// /// An object that can be disposed to release the lock. - IMigrationsDatabaseLock Refresh(bool connectionReopened, bool? transactionRestarted) + IMigrationsDatabaseLock ReacquireIfNeeded(bool connectionReopened, bool? transactionRestarted) { if ((connectionReopened && HistoryRepository.LockReleaseBehavior == LockReleaseBehavior.Connection) || (transactionRestarted is true && HistoryRepository.LockReleaseBehavior == LockReleaseBehavior.Transaction)) @@ -47,7 +47,7 @@ IMigrationsDatabaseLock Refresh(bool connectionReopened, bool? transactionRestar /// /// A to observe while waiting for the task to complete. /// An object that can be disposed to release the lock. - async Task RefreshAsync( + async Task ReacquireIfNeededAsync( bool connectionReopened, bool? transactionRestarted, CancellationToken cancellationToken = default) { if ((connectionReopened && HistoryRepository.LockReleaseBehavior == LockReleaseBehavior.Connection) diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs b/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs index 45ca493a03c..68c6ba4a886 100644 --- a/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs +++ b/src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs @@ -42,7 +42,7 @@ public virtual int ExecuteNonQuery( IRelationalConnection connection, MigrationExecutionState executionState, bool commitTransaction, - System.Data.IsolationLevel isolationLevel = System.Data.IsolationLevel.Unspecified) + System.Data.IsolationLevel? isolationLevel = null) { var inUserTransaction = connection.CurrentTransaction is not null && executionState.Transaction == null; if (inUserTransaction @@ -51,19 +51,18 @@ public virtual int ExecuteNonQuery( throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction); } - using (new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled)) - { - return executionStrategy.Execute( - (migrationCommands, connection, inUserTransaction, executionState, commitTransaction, isolationLevel), - static (_, s) => Execute( - s.migrationCommands, - s.connection, - s.executionState, - beginTransaction: !s.inUserTransaction, - commitTransaction: !s.inUserTransaction && s.commitTransaction, - s.isolationLevel), - verifySucceeded: null); - } + using var transactionScope = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled); + + return executionStrategy.Execute( + (migrationCommands, connection, inUserTransaction, executionState, commitTransaction, isolationLevel), + static (_, s) => Execute( + s.migrationCommands, + s.connection, + s.executionState, + beginTransaction: !s.inUserTransaction, + commitTransaction: !s.inUserTransaction && s.commitTransaction, + s.isolationLevel), + verifySucceeded: null); } private static int Execute( @@ -72,7 +71,7 @@ private static int Execute( MigrationExecutionState executionState, bool beginTransaction, bool commitTransaction, - System.Data.IsolationLevel isolationLevel = System.Data.IsolationLevel.Unspecified) + System.Data.IsolationLevel? isolationLevel) { var result = 0; var connectionOpened = connection.Open(); @@ -88,10 +87,12 @@ private static int Execute( && !command.TransactionSuppressed && beginTransaction) { - executionState.Transaction = connection.BeginTransaction(isolationLevel); + executionState.Transaction = isolationLevel == null + ? connection.BeginTransaction() + : connection.BeginTransaction(isolationLevel.Value); if (executionState.DatabaseLock != null) { - executionState.DatabaseLock = executionState.DatabaseLock.Refresh( + executionState.DatabaseLock = executionState.DatabaseLock.ReacquireIfNeeded( connectionOpened, transactionRestarted: true); connectionOpened = false; } @@ -108,7 +109,7 @@ private static int Execute( if (executionState.DatabaseLock != null) { - executionState.DatabaseLock = executionState.DatabaseLock.Refresh( + executionState.DatabaseLock = executionState.DatabaseLock.ReacquireIfNeeded( connectionOpened, transactionRestarted: null); connectionOpened = false; } @@ -167,7 +168,7 @@ public virtual async Task ExecuteNonQueryAsync( IRelationalConnection connection, MigrationExecutionState executionState, bool commitTransaction, - System.Data.IsolationLevel isolationLevel = System.Data.IsolationLevel.Unspecified, + System.Data.IsolationLevel? isolationLevel = null, CancellationToken cancellationToken = default) { var inUserTransaction = connection.CurrentTransaction is not null && executionState.Transaction == null; @@ -199,7 +200,7 @@ private static async Task ExecuteAsync( MigrationExecutionState executionState, bool beginTransaction, bool commitTransaction, - System.Data.IsolationLevel isolationLevel, + System.Data.IsolationLevel? isolationLevel, CancellationToken cancellationToken) { var result = 0; @@ -211,20 +212,23 @@ private static async Task ExecuteAsync( { for (var i = executionState.LastCommittedCommandIndex; i < migrationCommands.Count; i++) { + var lockReacquired = false; var command = migrationCommands[i]; if (executionState.Transaction == null && !command.TransactionSuppressed && beginTransaction) { - executionState.Transaction = await connection.BeginTransactionAsync(isolationLevel, cancellationToken) + executionState.Transaction = await (isolationLevel == null + ? connection.BeginTransactionAsync(cancellationToken) + : connection.BeginTransactionAsync(isolationLevel.Value, cancellationToken)) .ConfigureAwait(false); if (executionState.DatabaseLock != null) { - executionState.DatabaseLock = await executionState.DatabaseLock.RefreshAsync( + executionState.DatabaseLock = await executionState.DatabaseLock.ReacquireIfNeededAsync( connectionOpened, transactionRestarted: true, cancellationToken) .ConfigureAwait(false); - connectionOpened = false; + lockReacquired = true; } } @@ -237,12 +241,12 @@ private static async Task ExecuteAsync( executionState.LastCommittedCommandIndex = i; executionState.AnyOperationPerformed = true; - if (executionState.DatabaseLock != null) + if (executionState.DatabaseLock != null + && !lockReacquired) { - executionState.DatabaseLock = await executionState.DatabaseLock.RefreshAsync( + executionState.DatabaseLock = await executionState.DatabaseLock.ReacquireIfNeededAsync( connectionOpened, transactionRestarted: null, cancellationToken) .ConfigureAwait(false); - connectionOpened = false; } } diff --git a/src/EFCore.Relational/Migrations/Internal/Migrator.cs b/src/EFCore.Relational/Migrations/Internal/Migrator.cs index 7c322e2150b..9427082494d 100644 --- a/src/EFCore.Relational/Migrations/Internal/Migrator.cs +++ b/src/EFCore.Relational/Migrations/Internal/Migrator.cs @@ -1,7 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Data; +using System.Transactions; using Microsoft.EntityFrameworkCore.Diagnostics.Internal; namespace Microsoft.EntityFrameworkCore.Migrations.Internal; @@ -82,7 +82,7 @@ public Migrator( /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - protected virtual IsolationLevel MigrationTransactionIsolationLevel => IsolationLevel.Unspecified; + protected virtual System.Data.IsolationLevel? MigrationTransactionIsolationLevel => null; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -107,6 +107,8 @@ public virtual void Migrate(string? targetMigration) _logger.MigrateUsingConnection(this, _connection); + using var transactionScope = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled); + if (!_databaseCreator.Exists()) { _databaseCreator.Create(); @@ -161,12 +163,14 @@ private bool MigrateImplementation( { if (useTransaction) { - state.Transaction = _connection.BeginTransaction(MigrationTransactionIsolationLevel); + state.Transaction = MigrationTransactionIsolationLevel == null + ? _connection.BeginTransaction() + : _connection.BeginTransaction(MigrationTransactionIsolationLevel.Value); } state.DatabaseLock = state.DatabaseLock == null ? _historyRepository.AcquireDatabaseLock() - : state.DatabaseLock.Refresh(connectionOpened, useTransaction); + : state.DatabaseLock.ReacquireIfNeeded(connectionOpened, useTransaction); PopulateMigrations( _historyRepository.GetAppliedMigrations().Select(t => t.MigrationId), @@ -239,6 +243,8 @@ public virtual async Task MigrateAsync( _logger.MigrateUsingConnection(this, _connection); + using var transactionScope = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled); + if (!await _databaseCreator.ExistsAsync(cancellationToken).ConfigureAwait(false)) { await _databaseCreator.CreateAsync(cancellationToken).ConfigureAwait(false); @@ -268,7 +274,7 @@ static async (_, migrator, ct) => } }, verifySucceeded: null, - cancellationToken).ConfigureAwait(false); + cancellationToken).ConfigureAwait(false); await _executionStrategy.ExecuteAsync( (Migrator: this, @@ -297,12 +303,15 @@ private async Task MigrateImplementationAsync( { if (useTransaction) { - state.Transaction = await context.Database.BeginTransactionAsync(MigrationTransactionIsolationLevel, cancellationToken).ConfigureAwait(false); + state.Transaction = await (MigrationTransactionIsolationLevel == null + ? context.Database.BeginTransactionAsync(cancellationToken) + : context.Database.BeginTransactionAsync(MigrationTransactionIsolationLevel.Value, cancellationToken)) + .ConfigureAwait(false); } state.DatabaseLock = state.DatabaseLock == null ? await _historyRepository.AcquireDatabaseLockAsync(cancellationToken).ConfigureAwait(false) - : await state.DatabaseLock.RefreshAsync(connectionOpened, useTransaction, cancellationToken).ConfigureAwait(false); + : await state.DatabaseLock.ReacquireIfNeededAsync(connectionOpened, useTransaction, cancellationToken).ConfigureAwait(false); PopulateMigrations( (await _historyRepository.GetAppliedMigrationsAsync(cancellationToken).ConfigureAwait(false)).Select(t => t.MigrationId), diff --git a/src/EFCore.SqlServer/Migrations/Internal/SqlServerMigrationDatabaseLock.cs b/src/EFCore.SqlServer/Migrations/Internal/SqlServerMigrationDatabaseLock.cs index c590fa9bb74..b31ff900625 100644 --- a/src/EFCore.SqlServer/Migrations/Internal/SqlServerMigrationDatabaseLock.cs +++ b/src/EFCore.SqlServer/Migrations/Internal/SqlServerMigrationDatabaseLock.cs @@ -16,7 +16,7 @@ namespace Microsoft.EntityFrameworkCore.SqlServer.Migrations.Internal; /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public class SqlServerMigrationDatabaseLock( - IRelationalCommand relationalCommand, + IRelationalCommand releaseLockCommand, RelationalCommandParameterObject relationalCommandParameters, IHistoryRepository historyRepository, CancellationToken cancellationToken = default) @@ -37,7 +37,7 @@ public class SqlServerMigrationDatabaseLock( /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public void Dispose() - => relationalCommand.ExecuteScalar(relationalCommandParameters); + => releaseLockCommand.ExecuteScalar(relationalCommandParameters); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -46,5 +46,5 @@ public void Dispose() /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public async ValueTask DisposeAsync() - => await relationalCommand.ExecuteScalarAsync(relationalCommandParameters, cancellationToken).ConfigureAwait(false); + => await releaseLockCommand.ExecuteScalarAsync(relationalCommandParameters, cancellationToken).ConfigureAwait(false); } diff --git a/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteHistoryRepository.cs b/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteHistoryRepository.cs index c72c0ff788d..89d8fefc0c8 100644 --- a/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteHistoryRepository.cs +++ b/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteHistoryRepository.cs @@ -139,8 +139,6 @@ public override IMigrationsDatabaseLock AcquireDatabaseLock() retryDelay = retryDelay.Add(retryDelay); } } - - throw new TimeoutException(); } /// @@ -180,8 +178,6 @@ await CreateLockTableCommand().ExecuteNonQueryAsync(CreateRelationalCommandParam retryDelay = retryDelay.Add(retryDelay); } } - - throw new TimeoutException(); } private IRelationalCommand CreateLockTableCommand() diff --git a/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteMigrationDatabaseLock.cs b/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteMigrationDatabaseLock.cs index cec6f48c279..668d2107eaf 100644 --- a/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteMigrationDatabaseLock.cs +++ b/src/EFCore.Sqlite.Core/Migrations/Internal/SqliteMigrationDatabaseLock.cs @@ -10,7 +10,7 @@ namespace Microsoft.EntityFrameworkCore.Sqlite.Migrations.Internal; /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public class SqliteMigrationDatabaseLock( - IRelationalCommand relationalCommand, + IRelationalCommand releaseLockCommand, RelationalCommandParameterObject relationalCommandParameters, IHistoryRepository historyRepository, CancellationToken cancellationToken = default) @@ -31,7 +31,7 @@ public class SqliteMigrationDatabaseLock( /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public void Dispose() - => relationalCommand.ExecuteScalar(relationalCommandParameters); + => releaseLockCommand.ExecuteScalar(relationalCommandParameters); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -40,5 +40,5 @@ public void Dispose() /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public async ValueTask DisposeAsync() - => await relationalCommand.ExecuteScalarAsync(relationalCommandParameters, cancellationToken).ConfigureAwait(false); + => await releaseLockCommand.ExecuteScalarAsync(relationalCommandParameters, cancellationToken).ConfigureAwait(false); } diff --git a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsInfrastructureSqlServerTest.cs b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsInfrastructureSqlServerTest.cs index b487dc8c777..3d1c63e96ef 100644 --- a/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsInfrastructureSqlServerTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/Migrations/MigrationsInfrastructureSqlServerTest.cs @@ -1026,6 +1026,8 @@ CONSTRAINT [PK___EFMigrationsHistory] PRIMARY KEY ([MigrationId]) ); END; +SELECT 1 + SELECT OBJECT_ID(N'[__EFMigrationsHistory]'); SELECT [MigrationId], [ProductVersion] @@ -1056,6 +1058,8 @@ SELECT @result EXEC @result = sp_getapplock @Resource = '__EFMigrationsLock', @LockOwner = 'Session', @LockMode = 'Exclusive'; SELECT @result +SELECT 1 + SELECT OBJECT_ID(N'[__EFMigrationsHistory]'); SELECT [MigrationId], [ProductVersion] @@ -1134,6 +1138,8 @@ CONSTRAINT [PK___EFMigrationsHistory] PRIMARY KEY ([MigrationId]) ); END; +SELECT 1 + SELECT OBJECT_ID(N'[__EFMigrationsHistory]'); SELECT [MigrationId], [ProductVersion] @@ -1164,6 +1170,8 @@ SELECT @result EXEC @result = sp_getapplock @Resource = '__EFMigrationsLock', @LockOwner = 'Session', @LockMode = 'Exclusive'; SELECT @result +SELECT 1 + SELECT OBJECT_ID(N'[__EFMigrationsHistory]'); SELECT [MigrationId], [ProductVersion] From 3b884d94dae988324406ebf5d3cb294f629429a8 Mon Sep 17 00:00:00 2001 From: Andriy Svyryd Date: Fri, 30 Aug 2024 13:33:41 -0700 Subject: [PATCH 4/4] Add a warning for user transactions. --- .../Diagnostics/RelationalEventId.cs | 14 +++++ .../Diagnostics/RelationalLoggerExtensions.cs | 30 ++++++++++ .../RelationalLoggingDefinitions.cs | 9 +++ .../Migrations/Internal/Migrator.cs | 35 +++++++---- .../Properties/RelationalStrings.Designer.cs | 29 +++++++++- .../Properties/RelationalStrings.resx | 58 ++++++++++--------- 6 files changed, 135 insertions(+), 40 deletions(-) diff --git a/src/EFCore.Relational/Diagnostics/RelationalEventId.cs b/src/EFCore.Relational/Diagnostics/RelationalEventId.cs index cabde564e20..c05c73c46a3 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalEventId.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalEventId.cs @@ -80,6 +80,7 @@ private enum Id PendingModelChangesWarning, NonTransactionalMigrationOperationWarning, AcquiringMigrationLock, + MigrationsUserTransactionWarning, // Query events QueryClientEvaluationWarning = CoreEventId.RelationalBaseId + 500, @@ -764,6 +765,19 @@ private static EventId MakeMigrationsId(Id id) /// public static readonly EventId AcquiringMigrationLock = MakeMigrationsId(Id.AcquiringMigrationLock); + /// + /// A migration lock is being acquired. + /// + /// + /// + /// This event is in the category. + /// + /// + /// This event uses the payload when used with a . + /// + /// + public static readonly EventId MigrationsUserTransactionWarning = MakeMigrationsId(Id.MigrationsUserTransactionWarning); + private static readonly string _queryPrefix = DbLoggerCategory.Query.Name + "."; private static EventId MakeQueryId(Id id) diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs index a8afec569e1..177b7e90bba 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs @@ -2425,6 +2425,36 @@ private static string AcquiringMigrationLock(EventDefinitionBase definition, Eve return d.GenerateMessage(); } + /// + /// Logs for the event. + /// + /// The diagnostics logger to use. + public static void MigrationsUserTransactionWarning( + this IDiagnosticsLogger diagnostics) + { + var definition = RelationalResources.LogMigrationsUserTransaction(diagnostics); + + if (diagnostics.ShouldLog(definition)) + { + definition.Log(diagnostics); + } + + if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled)) + { + var eventData = new EventData( + definition, + MigrationsUserTransactionWarning); + + diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled); + } + } + + private static string MigrationsUserTransactionWarning(EventDefinitionBase definition, EventData payload) + { + var d = (EventDefinition)definition; + return d.GenerateMessage(); + } + /// /// Logs for the event. /// diff --git a/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs b/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs index 9b57a07998e..762172c8944 100644 --- a/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs +++ b/src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs @@ -367,6 +367,15 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions [EntityFrameworkInternal] public EventDefinitionBase? LogAcquiringMigrationLock; + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + [EntityFrameworkInternal] + public EventDefinitionBase? LogMigrationsUserTransactionWarning; + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore.Relational/Migrations/Internal/Migrator.cs b/src/EFCore.Relational/Migrations/Internal/Migrator.cs index 9427082494d..a6644ef6fc1 100644 --- a/src/EFCore.Relational/Migrations/Internal/Migrator.cs +++ b/src/EFCore.Relational/Migrations/Internal/Migrator.cs @@ -105,6 +105,11 @@ public virtual void Migrate(string? targetMigration) _logger.PendingModelChangesWarning(_currentContext.Context.GetType()); } + if (!useTransaction) + { + _logger.MigrationsUserTransactionWarning(); + } + _logger.MigrateUsingConnection(this, _connection); using var transactionScope = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled); @@ -118,7 +123,8 @@ public virtual void Migrate(string? targetMigration) try { var state = new MigrationExecutionState(); - if (_historyRepository.LockReleaseBehavior != LockReleaseBehavior.Transaction) + if (_historyRepository.LockReleaseBehavior != LockReleaseBehavior.Transaction + && useTransaction) { state.DatabaseLock = _historyRepository.AcquireDatabaseLock(); } @@ -166,11 +172,11 @@ private bool MigrateImplementation( state.Transaction = MigrationTransactionIsolationLevel == null ? _connection.BeginTransaction() : _connection.BeginTransaction(MigrationTransactionIsolationLevel.Value); - } - state.DatabaseLock = state.DatabaseLock == null - ? _historyRepository.AcquireDatabaseLock() - : state.DatabaseLock.ReacquireIfNeeded(connectionOpened, useTransaction); + state.DatabaseLock = state.DatabaseLock == null + ? _historyRepository.AcquireDatabaseLock() + : state.DatabaseLock.ReacquireIfNeeded(connectionOpened, useTransaction); + } PopulateMigrations( _historyRepository.GetAppliedMigrations().Select(t => t.MigrationId), @@ -241,6 +247,11 @@ public virtual async Task MigrateAsync( _logger.PendingModelChangesWarning(_currentContext.Context.GetType()); } + if (!useTransaction) + { + _logger.MigrationsUserTransactionWarning(); + } + _logger.MigrateUsingConnection(this, _connection); using var transactionScope = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled); @@ -254,7 +265,8 @@ public virtual async Task MigrateAsync( try { var state = new MigrationExecutionState(); - if (_historyRepository.LockReleaseBehavior != LockReleaseBehavior.Transaction) + if (_historyRepository.LockReleaseBehavior != LockReleaseBehavior.Transaction + && useTransaction) { state.DatabaseLock = await _historyRepository.AcquireDatabaseLockAsync(cancellationToken).ConfigureAwait(false); } @@ -306,12 +318,13 @@ private async Task MigrateImplementationAsync( state.Transaction = await (MigrationTransactionIsolationLevel == null ? context.Database.BeginTransactionAsync(cancellationToken) : context.Database.BeginTransactionAsync(MigrationTransactionIsolationLevel.Value, cancellationToken)) - .ConfigureAwait(false); - } + .ConfigureAwait(false); - state.DatabaseLock = state.DatabaseLock == null - ? await _historyRepository.AcquireDatabaseLockAsync(cancellationToken).ConfigureAwait(false) - : await state.DatabaseLock.ReacquireIfNeededAsync(connectionOpened, useTransaction, cancellationToken).ConfigureAwait(false); + state.DatabaseLock = state.DatabaseLock == null + ? await _historyRepository.AcquireDatabaseLockAsync(cancellationToken).ConfigureAwait(false) + : await state.DatabaseLock.ReacquireIfNeededAsync(connectionOpened, useTransaction, cancellationToken) + .ConfigureAwait(false); + } PopulateMigrations( (await _historyRepository.GetAppliedMigrationsAsync(cancellationToken).ConfigureAwait(false)).Select(t => t.MigrationId), diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index c2350480259..731c62c4ec2 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -2130,7 +2130,7 @@ public static string UnsupportedOperatorForSqlExpression(object? nodeType, objec nodeType, expressionType); /// - /// No relational type mapping can be found for property '{entity}.{property}' and the current provider doesn't specify a default store type for the properties of type '{clrType}'. + /// No relational type mapping can be found for property '{entity}.{property}' and the current provider doesn't specify a default store type for the properties of type '{clrType}'. /// public static string UnsupportedPropertyType(object? entity, object? property, object? clrType) => string.Format( @@ -2256,7 +2256,7 @@ private static readonly ResourceManager _resourceManager = new ResourceManager("Microsoft.EntityFrameworkCore.Properties.RelationalStrings", typeof(RelationalResources).Assembly); /// - /// Acquiring an exclusive lock for migration application. See https://aka.ms/efcore-docs-migrations for more information if this takes too long. + /// Acquiring an exclusive lock for migration application. See https://aka.ms/efcore-docs-migrations-lock for more information if this takes too long. /// public static EventDefinition LogAcquiringMigrationLock(IDiagnosticsLogger logger) { @@ -3424,6 +3424,31 @@ public static EventDefinition LogMigrationAttributeMissingWarning(IDiagn return (EventDefinition)definition; } + /// + /// A transaction was started before applying migrations. This prevents a database lock to be acquired and hence the database will not be protected from concurrent migration applications. The transactions and execution strategy are already managed by EF as needed. Remove the external transaction. + /// + public static EventDefinition LogMigrationsUserTransaction(IDiagnosticsLogger logger) + { + var definition = ((RelationalLoggingDefinitions)logger.Definitions).LogMigrationsUserTransactionWarning; + if (definition == null) + { + definition = NonCapturingLazyInitializer.EnsureInitialized( + ref ((RelationalLoggingDefinitions)logger.Definitions).LogMigrationsUserTransactionWarning, + logger, + static logger => new EventDefinition( + logger.Options, + RelationalEventId.MigrationsUserTransactionWarning, + LogLevel.Warning, + "RelationalEventId.MigrationsUserTransactionWarning", + level => LoggerMessage.Define( + level, + RelationalEventId.MigrationsUserTransactionWarning, + _resourceManager.GetString("LogMigrationsUserTransaction")!))); + } + + return (EventDefinition)definition; + } + /// /// Compiling a query which loads related collections for more than one collection navigation, either via 'Include' or through projection, but no 'QuerySplittingBehavior' has been configured. By default, Entity Framework will use 'QuerySplittingBehavior.SingleQuery', which can potentially result in slow query performance. See https://go.microsoft.com/fwlink/?linkid=2134277 for more information. To identify the query that's triggering this warning call 'ConfigureWarnings(w => w.Throw(RelationalEventId.MultipleCollectionIncludeWarning))'. /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index d6390b417fc..104daf9dafb 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -1,17 +1,17 @@  - @@ -786,6 +786,10 @@ A [Migration] attribute isn't specified on the '{class}' class. Warning RelationalEventId.MigrationAttributeMissingWarning string + + A transaction was started before applying migrations. This prevents a database lock to be acquired and hence the database will not be protected from concurrent migration applications. The transactions and execution strategy are already managed by EF as needed. Remove the external transaction. + Warning RelationalEventId.MigrationsUserTransactionWarning + Compiling a query which loads related collections for more than one collection navigation, either via 'Include' or through projection, but no 'QuerySplittingBehavior' has been configured. By default, Entity Framework will use 'QuerySplittingBehavior.SingleQuery', which can potentially result in slow query performance. See https://go.microsoft.com/fwlink/?linkid=2134277 for more information. To identify the query that's triggering this warning call 'ConfigureWarnings(w => w.Throw(RelationalEventId.MultipleCollectionIncludeWarning))'. Warning RelationalEventId.MultipleCollectionIncludeWarning