Remove migration lock timeout

Log migration lock acquisition

Fixes #34196
Fixes #17578

src/EFCore.Relational/Diagnostics/RelationalEventId.cs

@@ -79,6 +79,7 @@ private enum Id
         ColumnOrderIgnoredWarning,
         PendingModelChangesWarning,
         NonTransactionalMigrationOperationWarning,
+        MigrationLockCreating,
 
         // Query events
         QueryClientEvaluationWarning = CoreEventId.RelationalBaseId + 500,
@@ -749,6 +750,19 @@ private static EventId MakeMigrationsId(Id id)
     /// </remarks>
     public static readonly EventId NonTransactionalMigrationOperationWarning = MakeMigrationsId(Id.NonTransactionalMigrationOperationWarning);
 
+    /// <summary>
+    ///     A migration lock is being acquired.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         This event is in the <see cref="DbLoggerCategory.Migrations" /> category.
+    ///     </para>
+    ///     <para>
+    ///         This event uses the <see cref="EventData" /> payload when used with a <see cref="DiagnosticSource" />.
+    ///     </para>
+    /// </remarks>
+    public static readonly EventId MigrationLockCreating = MakeMigrationsId(Id.MigrationLockCreating);
+
     private static readonly string _queryPrefix = DbLoggerCategory.Query.Name + ".";
 
     private static EventId MakeQueryId(Id id)

src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs

@@ -2393,6 +2393,36 @@ private static string NonTransactionalMigrationOperationWarning(EventDefinitionB
         return d.GenerateMessage(commandText, p.Migration.GetType().ShortDisplayName());
     }
 
+    /// <summary>
+    ///     Logs for the <see cref="RelationalEventId.MigrationLockCreating" /> event.
+    /// </summary>
+    /// <param name="diagnostics">The diagnostics logger to use.</param>
+    public static void MigrationLockCreating(
+        this IDiagnosticsLogger<DbLoggerCategory.Migrations> diagnostics)
+    {
+        var definition = RelationalResources.LogCreatingMigrationLock(diagnostics);
+
+        if (diagnostics.ShouldLog(definition))
+        {
+            definition.Log(diagnostics);
+        }
+
+        if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
+        {
+            var eventData = new EventData(
+                definition,
+                MigrationLockCreating);
+
+            diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
+        }
+    }
+
+    private static string MigrationLockCreating(EventDefinitionBase definition, EventData payload)
+    {
+        var d = (EventDefinition)definition;
+        return d.GenerateMessage();
+    }
+
     /// <summary>
     ///     Logs for the <see cref="RelationalEventId.QueryPossibleUnintendedUseOfEqualsWarning" /> event.
     /// </summary>

src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs

@@ -358,6 +358,15 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions
     [EntityFrameworkInternal]
     public EventDefinitionBase? LogNoMigrationsFound;
 
+    /// <summary>
+    ///     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.
+    /// </summary>
+    [EntityFrameworkInternal]
+    public EventDefinitionBase? LogCreatingMigrationLock;
+
     /// <summary>
     ///     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

src/EFCore.Relational/Extensions/RelationalDatabaseFacadeExtensions.cs

@@ -126,10 +126,6 @@ public static void Migrate(this DatabaseFacade databaseFacade)
     /// <param name="seed">
     ///     The optional seed method to run after migrating the database. It will be invoked even if no migrations were applied.
     /// </param>
-    /// <param name="lockTimeout">
-    ///     The maximum amount of time that the migration lock should be held. Unless a catastrophic failure occurs, the
-    ///     lock is released when the migration operation completes.
-    /// </param>
     /// <remarks>
     ///     <para>
     ///         Note that this API is mutually exclusive with <see cref="DatabaseFacade.EnsureCreated" />. EnsureCreated does not use migrations
@@ -146,9 +142,8 @@ public static void Migrate(this DatabaseFacade databaseFacade)
     public static void Migrate(
         this DatabaseFacade databaseFacade,
         Action<DbContext, IMigratorData>? seed,
-        string? targetMigration = null,
-        TimeSpan? lockTimeout = null)
-        => databaseFacade.GetRelationalService<IMigrator>().Migrate(seed, targetMigration, lockTimeout);
+        string? targetMigration = null)
+        => databaseFacade.GetRelationalService<IMigrator>().Migrate(seed, targetMigration);
 
     /// <summary>
     ///     Asynchronously applies any pending migrations for the context to the database. Will create the database
@@ -187,10 +182,6 @@ public static Task MigrateAsync(
     /// <param name="seed">
     ///     The optional seed method to run after migrating the database. It will be invoked even if no migrations were applied.
     /// </param>
-    /// <param name="lockTimeout">
-    ///     The maximum amount of time that the migration lock should be held. Unless a catastrophic failure occurs, the
-    ///     lock is released when the migration operation completes.
-    /// </param>
     /// <param name="cancellationToken">A <see cref="CancellationToken" /> to observe while waiting for the task to complete.</param>
     /// <remarks>
     ///     <para>
@@ -211,9 +202,8 @@ public static Task MigrateAsync(
         this DatabaseFacade databaseFacade,
         Func<DbContext, IMigratorData, CancellationToken, Task>? seed,
         string? targetMigration = null,
-        TimeSpan? lockTimeout = null,
         CancellationToken cancellationToken = default)
-        => databaseFacade.GetRelationalService<IMigrator>().MigrateAsync(seed, targetMigration, lockTimeout, cancellationToken);
+        => databaseFacade.GetRelationalService<IMigrator>().MigrateAsync(seed, targetMigration, cancellationToken);
 
     /// <summary>
     ///     Executes the given SQL against the database and returns the number of rows affected.

src/EFCore.Relational/Migrations/HistoryRepository.cs

@@ -197,18 +197,16 @@ protected virtual IReadOnlyList<MigrationCommand> GetCreateCommands()
     /// <summary>
     ///     Gets an exclusive lock on the database.
     /// </summary>
-    /// <param name="timeout">The time to wait for the lock before an exception is thrown.</param>
     /// <returns>An object that can be disposed to release the lock.</returns>
-    public abstract IDisposable GetDatabaseLock(TimeSpan timeout);
+    public abstract IDisposable GetDatabaseLock();
 
     /// <summary>
     ///     Gets an exclusive lock on the database.
     /// </summary>
-    /// <param name="timeout">The time to wait for the lock before an exception is thrown.</param>
     /// <param name="cancellationToken">A <see cref="CancellationToken" /> to observe while waiting for the task to complete.</param>
     /// <returns>An object that can be disposed to release the lock.</returns>
     /// <exception cref="OperationCanceledException">If the <see cref="CancellationToken" /> is canceled.</exception>
-    public abstract Task<IAsyncDisposable> GetDatabaseLockAsync(TimeSpan timeout, CancellationToken cancellationToken = default);
+    public abstract Task<IAsyncDisposable> GetDatabaseLockAsync(CancellationToken cancellationToken = default);
 
     /// <summary>
     ///     Configures the entity type mapped to the history table.

src/EFCore.Relational/Migrations/IHistoryRepository.cs

@@ -77,18 +77,16 @@ Task<IReadOnlyList<HistoryRow>> GetAppliedMigrationsAsync(
     /// <summary>
     ///     Gets an exclusive lock on the database.
     /// </summary>
-    /// <param name="timeout">The time to wait for the lock before an exception is thrown.</param>
     /// <returns>An object that can be disposed to release the lock.</returns>
-    IDisposable GetDatabaseLock(TimeSpan timeout);
+    IDisposable GetDatabaseLock();
 
     /// <summary>
     ///     Gets an exclusive lock on the database.
     /// </summary>
-    /// <param name="timeout">The time to wait for the lock before an exception is thrown.</param>
     /// <param name="cancellationToken">A <see cref="CancellationToken" /> to observe while waiting for the task to complete.</param>
     /// <returns>An object that can be disposed to release the lock.</returns>
     /// <exception cref="OperationCanceledException">If the <see cref="CancellationToken" /> is canceled.</exception>
-    Task<IAsyncDisposable> GetDatabaseLockAsync(TimeSpan timeout, CancellationToken cancellationToken = default);
+    Task<IAsyncDisposable> GetDatabaseLockAsync(CancellationToken cancellationToken = default);
 
     /// <summary>
     ///     Generates a SQL script that will create the history table.

src/EFCore.Relational/Migrations/IMigrator.cs

@@ -32,16 +32,12 @@ public interface IMigrator
     /// <param name="targetMigration">
     ///     The target migration to migrate the database to, or <see langword="null" /> to migrate to the latest.
     /// </param>
-    /// <param name="lockTimeout">
-    ///     The maximum amount of time that the migration lock should be held. Unless a catastrophic failure occurs, the
-    ///     lock is released when the migration operation completes.
-    /// </param>
     /// <remarks>
     ///     See <see href="https://aka.ms/efcore-docs-migrations">Database migrations</see> for more information and examples.
     /// </remarks>
     [RequiresUnreferencedCode("Migration generation currently isn't compatible with trimming")]
     [RequiresDynamicCode("Migrations operations are not supported with NativeAOT")]
-    void Migrate(Action<DbContext, IMigratorData>? seed = null, string? targetMigration = null, TimeSpan? lockTimeout = null);
+    void Migrate(Action<DbContext, IMigratorData>? seed = null, string? targetMigration = null);
 
     /// <summary>
     ///     Migrates the database to either a specified target migration or up to the latest
@@ -53,10 +49,6 @@ public interface IMigrator
     /// <param name="targetMigration">
     ///     The target migration to migrate the database to, or <see langword="null" /> to migrate to the latest.
     /// </param>
-    /// <param name="lockTimeout">
-    ///     The maximum amount of time that the migration lock should be held. Unless a catastrophic failure occurs, the
-    ///     lock is released when the migration operation completes.
-    /// </param>
     /// <param name="cancellationToken">A <see cref="CancellationToken" /> to observe while waiting for the task to complete.</param>
     /// <returns>A task that represents the asynchronous operation</returns>
     /// <remarks>
@@ -68,7 +60,6 @@ public interface IMigrator
     Task MigrateAsync(
         Func<DbContext, IMigratorData, CancellationToken, Task>? seed = null,
         string? targetMigration = null,
-        TimeSpan? lockTimeout = null,
         CancellationToken cancellationToken = default);
 
     /// <summary>

src/EFCore.Relational/Migrations/IMigratorPlugin.cs

@@ -6,7 +6,7 @@ namespace Microsoft.EntityFrameworkCore.Migrations;
 /// <summary>
 ///     <para>
 ///         A service on the EF internal service provider that allows providers or extensions to execute logic
-///         after <see cref="IMigrator.Migrate(Action{DbContext, IMigratorData}?, string?, TimeSpan?)"/> is called.
+///         after <see cref="IMigrator.Migrate(Action{DbContext, IMigratorData}?, string?)"/> is called.
 ///     </para>
 ///     <para>
 ///         This type is typically used by providers or extensions. It is generally not used in application code.
@@ -20,7 +20,7 @@ namespace Microsoft.EntityFrameworkCore.Migrations;
 public interface IMigratorPlugin
 {
     /// <summary>
-    ///     Called by <see cref="IMigrator.Migrate(Action{DbContext, IMigratorData}?, string?, TimeSpan?)"/> before applying the migrations.
+    ///     Called by <see cref="IMigrator.Migrate(Action{DbContext, IMigratorData}?, string?)"/> before applying the migrations.
     /// </summary>
     /// <param name="context">The <see cref="DbContext" /> that is being migrated.</param>
     /// <param name="data">The <see cref="IMigratorData" /> that contains the result of the migrations application.</param>
@@ -30,7 +30,7 @@ public interface IMigratorPlugin
     void Migrating(DbContext context, IMigratorData data);
 
     /// <summary>
-    ///     Called by <see cref="IMigrator.MigrateAsync(Func{DbContext, IMigratorData, CancellationToken, Task}?, string?, TimeSpan?, CancellationToken)"/> before applying the migrations.
+    ///     Called by <see cref="IMigrator.MigrateAsync(Func{DbContext, IMigratorData, CancellationToken, Task}?, string?, CancellationToken)"/> before applying the migrations.
     /// </summary>
     /// <param name="context">The <see cref="DbContext" /> that is being migrated.</param>
     /// <param name="data">The <see cref="IMigratorData" /> that contains the result of the migrations application.</param>
@@ -46,7 +46,7 @@ Task MigratingAsync(
         CancellationToken cancellationToken = default);
 
     /// <summary>
-    ///     Called by <see cref="IMigrator.Migrate(Action{DbContext, IMigratorData}?, string?, TimeSpan?)"/> after applying the migrations, but before the seeding action.
+    ///     Called by <see cref="IMigrator.Migrate(Action{DbContext, IMigratorData}?, string?)"/> after applying the migrations, but before the seeding action.
     /// </summary>
     /// <param name="context">The <see cref="DbContext" /> that is being migrated.</param>
     /// <param name="data">The <see cref="IMigratorData" /> that contains the result of the migrations application.</param>
@@ -56,7 +56,7 @@ Task MigratingAsync(
     void Migrated(DbContext context, IMigratorData data);
 
     /// <summary>
-    ///     Called by <see cref="IMigrator.MigrateAsync(Func{DbContext, IMigratorData, CancellationToken, Task}?, string?, TimeSpan?, CancellationToken)"/> after applying the migrations, but before the seeding action.
+    ///     Called by <see cref="IMigrator.MigrateAsync(Func{DbContext, IMigratorData, CancellationToken, Task}?, string?, CancellationToken)"/> after applying the migrations, but before the seeding action.
     /// </summary>
     /// <param name="context">The <see cref="DbContext" /> that is being migrated.</param>
     /// <param name="data">The <see cref="IMigratorData" /> that contains the result of the migrations application.</param>

src/EFCore.Relational/Migrations/Internal/MigrationCommandExecutor.cs

@@ -11,21 +11,14 @@ 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.
 /// </summary>
-public class MigrationCommandExecutor : IMigrationCommandExecutor
+/// <remarks>
+///     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.
+/// </remarks>
+public class MigrationCommandExecutor(IExecutionStrategy executionStrategy) : IMigrationCommandExecutor
 {
-    private readonly IExecutionStrategy _executionStrategy;
-
-    /// <summary>
-    ///     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.
-    /// </summary>
-    public MigrationCommandExecutor(IExecutionStrategy executionStrategy)
-    {
-        _executionStrategy = executionStrategy;
-    }
-
     /// <summary>
     ///     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
@@ -36,19 +29,20 @@ public virtual void ExecuteNonQuery(
         IEnumerable<MigrationCommand> migrationCommands,
         IRelationalConnection connection)
     {
+        var commands = migrationCommands.ToList();
         var userTransaction = connection.CurrentTransaction;
         if (userTransaction is not null
-            && (migrationCommands.Any(x => x.TransactionSuppressed) || _executionStrategy.RetriesOnFailure))
+            && (commands.Any(x => x.TransactionSuppressed) || executionStrategy.RetriesOnFailure))
         {
             throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction);
         }
 
         using (new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled))
         {
-            var parameters = new ExecuteParameters(migrationCommands.ToList(), connection);
+            var parameters = new ExecuteParameters(commands, connection);
             if (userTransaction is null)
             {
-                _executionStrategy.Execute(parameters, static (_, p) => Execute(p, beginTransaction: true), verifySucceeded: null);
+                executionStrategy.Execute(parameters, static (_, p) => Execute(p, beginTransaction: true), verifySucceeded: null);
             }
             else
             {
@@ -114,20 +108,21 @@ public virtual async Task ExecuteNonQueryAsync(
         IRelationalConnection connection,
         CancellationToken cancellationToken = default)
     {
+        var commands = migrationCommands.ToList();
         var userTransaction = connection.CurrentTransaction;
         if (userTransaction is not null
-            && (migrationCommands.Any(x => x.TransactionSuppressed) || _executionStrategy.RetriesOnFailure))
+            && (commands.Any(x => x.TransactionSuppressed) || executionStrategy.RetriesOnFailure))
         {
             throw new NotSupportedException(RelationalStrings.TransactionSuppressedMigrationInUserTransaction);
         }
 
         var transactionScope = new TransactionScope(TransactionScopeOption.Suppress, TransactionScopeAsyncFlowOption.Enabled);
         try
         {
-            var parameters = new ExecuteParameters(migrationCommands.ToList(), connection);
+            var parameters = new ExecuteParameters(commands, connection);
             if (userTransaction is null)
             {
-                await _executionStrategy.ExecuteAsync(
+                await executionStrategy.ExecuteAsync(
                     parameters,
                     static (_, p, ct) => ExecuteAsync(p, beginTransaction: true, ct),
                     verifySucceeded: null,