Skip to content

EF9: single transaction spanning all migrations causes transaction-incompatible SQL to fail #35096

New issue

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

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

Already on GitHub? Sign in to your account

Open
Tornhoof opened this issue Nov 13, 2024 · 29 comments
Assignees
Labels
area-migrations customer-reported priority-bug Issues which requires API breaks and have bigger impact hence should be fixed earlier in the release type-bug
Milestone

Comments

@Tornhoof
Copy link

Tornhoof commented Nov 13, 2024

Hi,
I tried to migrate my solution to .NET 9, incl. EF Core 9 with SQL Server as the provider.
Suddenly one of my migrations (old one from .NET 7 times) fails. I looked into the differences between the migration code and it appears that the transaction for migrations is now one large transaction per DbContext? and not one migration per .cs migration file.

If I modify my migration to use suppressTransaction: true from https://learn.microsoft.com/en-us/ef/core/managing-schemas/migrations/managing?tabs=dotnet-core-cli it works again.

Is that an intended breaking change or some config parameter I missed in the migration?

Log see below:

.NET 8:

2024-11-13 15:52:31.8230 - Info  - ThreadId: 16 - TraceId: na - Identity: System - Migrations: Applying migration '20230313093754_RemoveNodesMetadataId'.
2024-11-13 15:52:31.8230 - Debug - ThreadId: 16 - TraceId: na - Identity: System - Connection: Opening connection to database 'AnnotationsTestsDb' on server 'localhost'.
2024-11-13 15:52:31.8230 - Debug - ThreadId: 16 - TraceId: na - Identity: System - Connection: Opened connection to database 'AnnotationsTestsDb' on server 'localhost'.
2024-11-13 15:52:31.8230 - Debug - ThreadId: 16 - TraceId: na - Identity: System - Transaction: Beginning transaction with isolation level 'Unspecified'.
2024-11-13 15:52:31.8230 - Debug - ThreadId: 16 - TraceId: na - Identity: System - Transaction: Began transaction with isolation level 'ReadCommitted'.
2024-11-13 15:52:31.8230 - Debug - ThreadId: 16 - TraceId: na - Identity: System - Command: Creating DbCommand for 'ExecuteNonQuery'.
2024-11-13 15:52:31.8230 - Debug - ThreadId: 16 - TraceId: na - Identity: System - Command: Created DbCommand for 'ExecuteNonQuery' (0ms).
2024-11-13 15:52:31.8230 - Debug - ThreadId: 16 - TraceId: na - Identity: System - Command: Initialized DbCommand for 'ExecuteNonQuery' (0ms).
2024-11-13 15:52:31.8230 - Debug - ThreadId: 16 - TraceId: na - Identity: System - Command: Executing DbCommand [Parameters=[], CommandType='Text', CommandTimeout='30']

DECLARE @ToDeletePredecessor AS TVP_BIGINT;
INSERT INTO @ToDeletePredecessor SELECT DISTINCT e.Id FROM isPredecessorOf e LEFT OUTER JOIN Nodes n1 ON n1.$node_id = e.$from_id LEFT OUTER JOIN Nodes n2 ON n2.$node_id = e.$to_id WHERE n1.Id IS NULL OR n2.Id IS NULL;
DELETE FROM GraphPaths WHERE Id IN (SELECT [Value] FROM @ToDeletePredecessor);
DELETE FROM isPredecessorOf WHERE Id IN (SELECT [Value] FROM @ToDeletePredecessor);
2024-11-13 15:52:31.8230 - Info  - ThreadId: 19 - TraceId: na - Identity: System - Command: Executed DbCommand (10ms) [Parameters=[], CommandType='Text', CommandTimeout='30']

DECLARE @ToDeletePredecessor AS TVP_BIGINT;
INSERT INTO @ToDeletePredecessor SELECT DISTINCT e.Id FROM isPredecessorOf e LEFT OUTER JOIN Nodes n1 ON n1.$node_id = e.$from_id LEFT OUTER JOIN Nodes n2 ON n2.$node_id = e.$to_id WHERE n1.Id IS NULL OR n2.Id IS NULL;
DELETE FROM GraphPaths WHERE Id IN (SELECT [Value] FROM @ToDeletePredecessor);
DELETE FROM isPredecessorOf WHERE Id IN (SELECT [Value] FROM @ToDeletePredecessor);

.NET9:

2024-11-13 16:05:04.0882 - Info  - ThreadId: 15 - TraceId: na - Identity: System - Migrations: Applying migration '20230313093754_RemoveNodesMetadataId'.
2024-11-13 16:05:04.0882 - Debug - ThreadId: 15 - TraceId: na - Identity: System - Command: Creating DbCommand for 'ExecuteNonQuery'.
2024-11-13 16:05:04.0882 - Debug - ThreadId: 15 - TraceId: na - Identity: System - Command: Created DbCommand for 'ExecuteNonQuery' (0ms).
2024-11-13 16:05:04.0882 - Debug - ThreadId: 15 - TraceId: na - Identity: System - Command: Initialized DbCommand for 'ExecuteNonQuery' (0ms).
2024-11-13 16:05:04.0882 - Debug - ThreadId: 15 - TraceId: na - Identity: System - Command: Executing DbCommand [Parameters=[], CommandType='Text', CommandTimeout='30']

DECLARE @ToDeletePredecessor AS TVP_BIGINT;
INSERT INTO @ToDeletePredecessor SELECT DISTINCT e.Id FROM isPredecessorOf e LEFT OUTER JOIN Nodes n1 ON n1.$node_id = e.$from_id LEFT OUTER JOIN Nodes n2 ON n2.$node_id = e.$to_id WHERE n1.Id IS NULL OR n2.Id IS NULL;
DELETE FROM GraphPaths WHERE Id IN (SELECT [Value] FROM @ToDeletePredecessor);
DELETE FROM isPredecessorOf WHERE Id IN (SELECT [Value] FROM @ToDeletePredecessor)
2024-11-13 16:05:05.1854 - Error - ThreadId: 15 - TraceId: na - Identity: System - Command: Failed executing DbCommand (1,091ms) [Parameters=[], CommandType='Text', CommandTimeout='30']

DECLARE @ToDeletePredecessor AS TVP_BIGINT;
INSERT INTO @ToDeletePredecessor SELECT DISTINCT e.Id FROM isPredecessorOf e LEFT OUTER JOIN Nodes n1 ON n1.$node_id = e.$from_id LEFT OUTER JOIN Nodes n2 ON n2.$node_id = e.$to_id WHERE n1.Id IS NULL OR n2.Id IS NULL;
DELETE FROM GraphPaths WHERE Id IN (SELECT [Value] FROM @ToDeletePredecessor);
DELETE FROM isPredecessorOf WHERE Id IN (SELECT [Value] FROM @ToDeletePredecessor)
2024-11-13 16:05:05.1854 - Debug - ThreadId: 15 - TraceId: na - Identity: System - Transaction: Disposing transaction.

(I don't really know why this specific sql snippet fails in the existing migration and I have not really looked into that in any detail, I guess it might be related to the TVP not being available in that transaction, but that's just a wild guess).

@roji
Copy link
Member

roji commented Nov 13, 2024

@Tornhoof can you please share the part of the migration which causes the error when executing in a transaction?

@Tornhoof
Copy link
Author

I'm not sure how much you need, what specifically fails are the 4 sql statements in the log file, the first first one in that file fails.
This is directly the first call to migrationBuilder.Sql in this specific migration source file. If I comment it or use the suppressTransaction arg everything works. It's not the first migration file, nor the last, nor is it the first or last using sql statements or even the aforementioned TVP_BigInt.

The full migration file, which includes these sql statements is below in details. The migration is basically three different sql scripts, first one cleans some SQL graph tables up, then removes a column in a SQL Graph tables and changes a few constrains and the third one recreates an unrelated table-valued-parameter. The migrations are just used to run the scripts, there are no EF core model changes involved.

namespace WebApp.Viewing.Graph.Migrations
{
    public static class GraphTableMigrationExtension
    {

       // THIS ONE FAILS
        private const string CleanupDanglingPredecessorsSql = @"
DECLARE @ToDeletePredecessor AS TVP_BIGINT;
INSERT INTO @ToDeletePredecessor SELECT DISTINCT e.Id FROM isPredecessorOf e LEFT OUTER JOIN Nodes n1 ON n1.$node_id = e.$from_id LEFT OUTER JOIN Nodes n2 ON n2.$node_id = e.$to_id WHERE n1.Id IS NULL OR n2.Id IS NULL;
DELETE FROM GraphPaths WHERE Id IN (SELECT [Value] FROM @ToDeletePredecessor);
DELETE FROM isPredecessorOf WHERE Id IN (SELECT [Value] FROM @ToDeletePredecessor)
";

	
		public static void CleanupDanglingPredecessors(this MigrationBuilder migrationBuilder)
        {
            migrationBuilder.Sql(CleanupDanglingPredecessorsSql, suppressTransaction:true); // requires suppressTransaction in .NET 9 atm
        }

        private const string RemoveMetadataIdColumn = @"
IF COL_LENGTH('dbo.Nodes', 'MetadataId') IS NOT NULL
BEGIN
    ALTER TABLE Nodes ADD CONSTRAINT FK_Nodes_Metadata_Id FOREIGN KEY (Id) REFERENCES Metadata (Id) ON DELETE CASCADE;
    ALTER TABLE Nodes DROP CONSTRAINT C_Nodes_Id_MetadataId;
    DROP INDEX IX_Nodes_MetadataId ON Nodes;
    ALTER TABLE Nodes DROP COLUMN MetadataId;
    ALTER TABLE isParentOf ADD CONSTRAINT EC_isParentOf CONNECTION (Nodes TO Nodes) ON DELETE CASCADE;
    ALTER TABLE isFollowerOf ADD CONSTRAINT EC_isFollowerOf CONNECTION (Nodes TO Nodes) ON DELETE CASCADE;
    ALTER TABLE isPredecessorOf ADD CONSTRAINT EC_isPredecessorOf CONNECTION (Nodes TO Nodes) ON DELETE CASCADE;
END
";
        public static void RemoveMetadataId(this MigrationBuilder migrationBuilder)
        {
            migrationBuilder.Sql(RemoveMetadataIdColumn);
        }
		
		private const string DropNodeTvp = "DROP TYPE IF EXISTS TVP_Nodes";
		
        private const string CreateNodeTvp = @"
IF TYPE_ID(N'TVP_Nodes') IS NULL
CREATE TYPE [TVP_Nodes] AS TABLE(
	[Id] BIGINT NOT NULL,
	[NodeType] INT NOT NULL,
	[Text] NVARCHAR(523),
	INDEX IX_TVP_Nodes_Id ([Id])
);";		
		
        public static void RecreateNodeTvpStatement(this MigrationBuilder migrationBuilder)
        {
            migrationBuilder.DropNodeTvpStatement();
            migrationBuilder.CreateNodeTvpStatement();
        }
	
	}
}	  


using WebApp.Viewing.Graph.Extensions;
using Microsoft.EntityFrameworkCore.Migrations;

#nullable disable

namespace WebApp.Viewing.Graph.Migrations
{
    /// <inheritdoc />
    public partial class RemoveNodesMetadataId : Migration
    {
        /// <inheritdoc />
        protected override void Up(MigrationBuilder migrationBuilder)
        {
            // this is to add the later node constraints
            migrationBuilder.CleanupDanglingPredecessors();
            migrationBuilder.RemoveMetadataId();
            migrationBuilder.RecreateNodeTvpStatement();
        }

        /// <inheritdoc />
        protected override void Down(MigrationBuilder migrationBuilder)
        {
            // not supported
        }
    }
}


// <auto-generated />
using System;
using WebApp.Viewing.Graph.Infrastructure;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Migrations;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;

#nullable disable

namespace WebApp.Viewing.Graph.Migrations
{
    [DbContext(typeof(GraphContext))]
    [Migration("20230313093754_RemoveNodesMetadataId")]
    partial class RemoveNodesMetadataId
    {
        /// <inheritdoc />
        protected override void BuildTargetModel(ModelBuilder modelBuilder)
        {
#pragma warning disable 612, 618
            modelBuilder
                .HasAnnotation("ProductVersion", "7.0.3")
                .HasAnnotation("Relational:MaxIdentifierLength", 128);

            SqlServerModelBuilderExtensions.UseIdentityColumns(modelBuilder);

            modelBuilder.Entity("WebApp.Viewing.Graph.Data.CustomAttribute", b =>
                {
                    b.Property<long>("Id")
                        .ValueGeneratedOnAdd()
                        .HasColumnType("bigint");

                    SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property<long>("Id"));

                    b.Property<string>("Key")
                        .HasMaxLength(100)
                        .HasColumnType("nvarchar(100)");

                    b.Property<long>("MetadataId")
                        .HasColumnType("bigint");

                    b.Property<int>("Order")
                        .HasColumnType("int");

                    b.Property<byte[]>("RowVersion")
                        .IsConcurrencyToken()
                        .IsRequired()
                        .ValueGeneratedOnAddOrUpdate()
                        .HasColumnType("rowversion");

                    b.Property<int>("Type")
                        .HasColumnType("int");

                    b.Property<string>("Value")
                        .IsRequired()
                        .HasColumnType("nvarchar(max)");

                    b.HasKey("Id");

                    b.HasIndex("Type");

                    b.HasIndex("MetadataId", "Order")
                        .IsUnique();

                    b.ToTable("Attributes", (string)null);
                });

            modelBuilder.Entity("WebApp.Viewing.Graph.Data.GraphPath", b =>
                {
                    b.Property<long>("Id")
                        .ValueGeneratedOnAdd()
                        .HasColumnType("bigint");

                    SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property<long>("Id"));

                    b.Property<DateTimeOffset>("Created")
                        .HasColumnType("datetimeoffset");

                    b.Property<string>("Key")
                        .IsRequired()
                        .HasMaxLength(128)
                        .HasColumnType("nvarchar(128)");

                    b.Property<long>("MetadataId")
                        .HasColumnType("bigint");

                    b.Property<byte[]>("RowVersion")
                        .IsConcurrencyToken()
                        .IsRequired()
                        .ValueGeneratedOnAddOrUpdate()
                        .HasColumnType("rowversion");

                    b.Property<int>("Type")
                        .HasColumnType("int");

                    b.HasKey("Id");

                    b.HasIndex("Key")
                        .IsUnique();

                    b.HasIndex("MetadataId");

                    b.HasIndex("Type");

                    b.ToTable("GraphPaths", (string)null);
                });

            modelBuilder.Entity("WebApp.Viewing.Graph.Data.Metadata", b =>
                {
                    b.Property<long>("Id")
                        .ValueGeneratedOnAdd()
                        .HasColumnType("bigint");

                    SqlServerPropertyBuilderExtensions.UseIdentityColumn(b.Property<long>("Id"));

                    b.Property<bool>("Extended")
                        .HasColumnType("bit");

                    b.Property<byte?>("Operation")
                        .HasColumnType("tinyint");

                    b.Property<string>("Path")
                        .HasMaxLength(500)
                        .HasColumnType("nvarchar(500)");

                    b.Property<byte[]>("RowVersion")
                        .IsConcurrencyToken()
                        .IsRequired()
                        .ValueGeneratedOnAddOrUpdate()
                        .HasColumnType("rowversion");

                    b.Property<string>("SourceGroupId")
                        .HasMaxLength(100)
                        .HasColumnType("nvarchar(100)");

                    b.Property<string>("SourceNodeId")
                        .HasMaxLength(100)
                        .HasColumnType("nvarchar(100)");

                    b.Property<string>("SourceOperationReference")
                        .HasMaxLength(200)
                        .HasColumnType("nvarchar(200)");

                    b.Property<int>("Type")
                        .HasColumnType("int");

                    b.HasKey("Id");

                    b.HasIndex("Path")
                        .HasFilter("[Path] IS NOT NULL AND [Type] != 105");

                    b.HasIndex("SourceNodeId");

                    b.HasIndex("Type");

                    b.ToTable("Metadata", (string)null);
                });

            modelBuilder.Entity("WebApp.Viewing.Graph.Data.VdmIndexedAttributeType", b =>
                {
                    b.Property<int>("Type")
                        .HasColumnType("int");

                    b.Property<bool>("Mode")
                        .HasColumnType("bit");

                    b.HasKey("Type");

                    b.ToTable("VdmIndexedAttributeTypes", (string)null);
                });

            modelBuilder.Entity("WebApp.Viewing.Graph.Data.CustomAttribute", b =>
                {
                    b.HasOne("WebApp.Viewing.Graph.Data.Metadata", null)
                        .WithMany("CustomAttributes")
                        .HasForeignKey("MetadataId")
                        .OnDelete(DeleteBehavior.Cascade)
                        .IsRequired();
                });

            modelBuilder.Entity("WebApp.Viewing.Graph.Data.Metadata", b =>
                {
                    b.Navigation("CustomAttributes");
                });
#pragma warning restore 612, 618
        }
    }
}

@roji
Copy link
Member

roji commented Nov 13, 2024

How does it fail, what's the error? We need some details in order to investigate this.

@Tornhoof
Copy link
Author

It happens during the unit tests which create a db via graphContext.Database.MigrateAsync on an non-existing db.
It also happens if I run the affected migration against an existing db which has old enough state.
The exception is the following:

  System.InvalidOperationException : An exception has been raised that is likely due to a transient failure. Consider enabling transient error resiliency by adding 'EnableRetryOnFailure' to the 'UseSqlServer' call.
  ---- Microsoft.Data.SqlClient.SqlException : Transaction (Process ID 54) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
  Stack Trace:
WebApp.Annotation.LongTests.AnnotationHandlerTests.FindRangeMultipleMetadataIdsConsecutive(count: 100, extraIds: True) [FAIL]
       at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)
       at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationCommandExecutor.ExecuteNonQueryAsync(IReadOnlyList`1 migrationCommands, IRelationalConnection connection, MigrationExecutionState executionState, Boolean commitTransaction, Nullable`1 isolationLevel, CancellationToken cancellationToken)
       at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.MigrateImplementationAsync(DbContext context, String targetMigration, MigrationExecutionState state, Boolean useTransaction, CancellationToken cancellationToken)
       at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.MigrateImplementationAsync(DbContext context, String targetMigration, MigrationExecutionState state, Boolean useTransaction, CancellationToken cancellationToken)
       at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.<>c.<<MigrateAsync>b__22_1>d.MoveNext()
    --- End of stack trace from previous location ---
       at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)
       at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.MigrateAsync(String targetMigration, CancellationToken cancellationToken)
       at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.MigrateAsync(String targetMigration, CancellationToken cancellationToken)
    C:\agent\_work\8\s\Backend\NGP\QA\WebApp.Annotation.LongTests\AnnotationLongTestFixture.cs(45,0): at WebApp.Annotation.LongTests.AnnotationLongTestFixture.GetGraphContext()
    C:\agent\_work\8\s\Backend\NGP\QA\WebApp.Annotation.LongTests\AnnotationLongTestFixture.cs(34,0): at WebApp.Annotation.LongTests.AnnotationLongTestFixture.InitializeAsync()
    ----- Inner Stack Trace -----
       at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
       at Microsoft.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
       at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, SqlCommand command, Boolean callerHasConnectionLock, Boolean asyncClose)
       at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
       at Microsoft.Data.SqlClient.SqlCommand.InternalEndExecuteNonQuery(IAsyncResult asyncResult, Boolean isInternal, String endMethod)
       at Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryInternal(IAsyncResult asyncResult)
       at Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryAsync(IAsyncResult asyncResult)
       at Microsoft.Data.SqlClient.SqlCommand.<>c.<InternalExecuteNonQueryAsync>b__193_1(IAsyncResult asyncResult)
       at System.Threading.Tasks.TaskFactory`1.FromAsyncCoreLogic(IAsyncResult iar, Func`2 endFunction, Action`1 endAction, Task`1 promise, Boolean requiresSynchronization)
    --- End of stack trace from previous location ---
       at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteNonQueryAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
       at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteNonQueryAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
       at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteNonQueryAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
       at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationCommandExecutor.ExecuteAsync(IReadOnlyList`1 migrationCommands, IRelationalConnection connection, MigrationExecutionState executionState, Boolean beginTransaction, Boolean commitTransaction, Nullable`1 isolationLevel, CancellationToken cancellationToken)
       at Microsoft.EntityFrameworkCore.Migrations.Internal.MigrationCommandExecutor.ExecuteAsync(IReadOnlyList`1 migrationCommands, IRelationalConnection connection, MigrationExecutionState executionState, Boolean beginTransaction, Boolean commitTransaction, Nullable`1 isolationLevel, CancellationToken cancellationToken)
       at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)

If I read the What's new Document correctly, EF Core 9 now tries to block concurrent migrations, I don't know if Resource deadlock is the expected error if that happens, but I'm really sure that there are no concurrent migrations here and commenting out a specific sql statement should have no impact on concurrent migrations anyway, so I did put that into the topic.

@roji
Copy link
Member

roji commented Nov 13, 2024

That's very odd indeed. Are you able to put together a minimal repro that shows this happening? That would be the best way to help us investigate and fix this.

/cc @AndriySvyryd

@Tornhoof
Copy link
Author

I'll see what I can cobble together

@Tornhoof
Copy link
Author

I wrote a repro here: https://github.com/Tornhoof/MigrationRepro, just clone it and run it (see program.cs for connection string).
It contains two migrations, one creates a tvp, the other one uses it.
As expected above it is the TVP.
Apparently I was wrong and it's the first use of any TVP in any migration, every other migration just creates them.

@roji
Copy link
Member

roji commented Nov 14, 2024

Confirmed, see below for a minimal repro without EF: trying to create a TVP and to declare a variable with that TVP in a transaction fails on SQL Server because of a deadlock.

Note that this was a problem in previous versions of EF; the following migration triggers a deadlock also with EF 8.0:

protected override void Up(MigrationBuilder migrationBuilder)
{
    migrationBuilder.Sql("""
CREATE TYPE TVP_BigInt AS TABLE  (
[Value] BIGINT,
PRIMARY KEY ([Value])
);
""");
    migrationBuilder.Sql("DECLARE @ToDeletePredecessor AS TVP_BIGINT");
}

The difference in 9.0 is that the transaction extends across migrations, so the deadlock occurs even when the creation and the declaration are spread across different migrations.

I don't think there's anything we can do here really - the workaround for now would be to suppress the transaction as @Tornhoof said above. We can also reach out to the SQL Server people and discuss this.

Minimal repro without EF
await using var connection = new SqlConnection("Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0;Encrypt=false");
await connection.OpenAsync();

await using var command = connection.CreateCommand();
command.CommandText = "DROP TYPE IF EXISTS TVP_BigInt";
await command.ExecuteNonQueryAsync();

await using var transaction = (SqlTransaction)await connection.BeginTransactionAsync();
command.Transaction = transaction;

command.CommandText = """
CREATE TYPE TVP_BigInt AS TABLE  (
    [Value] BIGINT,
    PRIMARY KEY ([Value])
)
""";
await command.ExecuteNonQueryAsync();

command.CommandText = "DECLARE @ToDeletePredecessor AS TVP_BIGINT";
await command.ExecuteNonQueryAsync();

await transaction.CommitAsync();

@roji roji changed the title EF Core 9 Migration Transaction change? EF9: single transaction spanning all migrations causes transaction-incompatible SQL to fail Nov 16, 2024
@roji
Copy link
Member

roji commented Nov 16, 2024

Thanks for bringing to our attention @Tornhoof. I unfortunately don't think there's anything actionable for us here, plus the problem existed before as well, if you happened to perform both actions in a single migration...

In addition to the workaround of suppressing the transaction, you can also simply apply the two migrations separately, by explicitly specifying on the command-line which migration state you want to migrate to (although you'd have to remember to do that every time).

Other than that, I'll send a note to the SQL Server people about this so that they're aware - you can also follow up with them yourself.

@akamor
Copy link

akamor commented Nov 19, 2024

@roji We are seeing perhaps similar behavior and its odd.

Prior to the dotnet9 upgrade the SQL generated by our migrations wrapped each migration in its own BEGIN...COMMIT block. So each migration had its own transaction. In dotnet9 that is gone and the entire migration history is a single transaction.

One of our migrations modifies an enum value and then a subsequent migration that references the enum errors with the PG message:

    MessageText: unsafe use of new value "ParseFiles" of enum type job_type
    Hint: New enum values must be committed before they can be used.

This didn't happen before because altering the ENUM in a previous migration was committed prior to the latter migration being run.

Is this expected behavior in dotnet9? If so, this seems like it could potentially break a lot of databases unless I am missing something (which is entirely possible).

In terms of workarounds I could go modify the old migration and explicitly commit the ALTER TYPE command. Any other ideas?

@akamor
Copy link

akamor commented Nov 19, 2024

Ok for those hitting same issue I have a potentially better workaround. The Migration created for modifying the enum gave me no way to call into suppressTransaction since it wasn't using migrationBuilder.Sql(). Luckily, the later migration was built using raw sql so I set suppressTransaction=true there and things are working now. By doing that, it commits all previous migration scripts and starts a new transaction AFTER the raw sql runs. Its a bit risky if something goes wrong because that one migration wont be rolled back but I dont think there are any other options here.

@roji
Copy link
Member

roji commented Nov 19, 2024

@akamor can you provide a bit more context, ideally a sample? I'm assuming you're using PostgreSQL (which version)?

@akamor
Copy link

akamor commented Nov 19, 2024

Certainly, I am on Postgres v14.7. The issue happened when we migrated from net8 to net9. On dotnet8 it appears that each migration happens inside its own transaction. We had a migration that modified an enum by adding a new value. In a subsequent migration that enum value was referenced in some custom sql. This worked fine in net8. when we moved to net9 we saw that each migration no longer had its own transaction and instead there is a single transaction for all of the migrations. This lead to the problem that an enum was being modified in the same transaction block where it was later referenced. Postgres didnt like this because to reference a changed enum value that change must first be committed but it no longer was as of net9.

The real issue here appears to be the change in behavior where each migration no longer gets its own transaction. Does this make sense?

@AnthonyDewhirst

This comment has been minimized.

@roji

This comment has been minimized.

@AnthonyDewhirst

This comment has been minimized.

@roji

This comment has been minimized.

@roji
Copy link
Member

roji commented Nov 26, 2024

@akamor thanks for the info, I can indeed reproduce this problem on PostgreSQL with the following SQL:

CREATE TYPE some_enum AS ENUM ('one', 'two');
CREATE TABLE foo (some_enum some_enum);

BEGIN;
ALTER TYPE some_enum ADD VALUE 'three';
INSERT INTO foo (some_enum) VALUES ('three');
COMMIT;

Error:

[55P04] ERROR: unsafe use of new value "three" of enum type some_enum
Hint: New enum values must be committed before they can be used.

So yes, if you're adding an enum label in one migration, and then using it in another, the change in EF 9 would make that start failing. For now, you can work around this by enabling suppressTransaction: true on the raw SQL where you attempt to use the enum - we'll discuss this internally to see how we want to proceed.

@AnthonyDewhirst
Copy link

I can confirm that the workaround for my drop table issue was resolved by the workaround in the linked issue.

Apologise for my tone earlier. Way too much work going on and jumped on the first thing I saw as the problem instead of properly investigating. Something I try to get my team to not do, and I jumped head first in. Sorry.

@roji
Copy link
Member

roji commented Nov 26, 2024

@AnthonyDewhirst no worries, I understand how upgrading and getting a sudden unexpected break is frustrating. Thanks for engaging and constructively looking for a fix.

@blastrock
Copy link

Hi,

We just got hit by that same issue. You said that you'll be discussing this internally, is there an open issue that we can track to have updates on this?

Thank you

@rburnham-def
Copy link

We just tried upgrading to EF9 as well and looks like we are running into the same problem. We have a situation where we could be connecting to a new Database so we have to reapply the entire migration history. EF8 was fine but we noticed several migrations that use migration.Sql() to do some sort of data conversion fail with the same deadlock error. For example we have one that converts an a bool prop into an Enum. The migration Adds a new int column, then updates all the rows to the correct enum value based on the old bool and then drops the bool column.

            migrationBuilder.AddColumn<int>(
                name: "MyTableDefaultMode",
                schema: "dbo",
                table: "MyTable",
                type: "int",
                nullable: false,
                defaultValue: 0);

            migrationBuilder.Sql("UPDATE MyTable SET MyTableDefaultMode = 1 WHERE IsDefault = 1");

            migrationBuilder.DropColumn(
                name: "IsDefault",
                schema: "dbo",
                table: "MyTable");

The update statement is where it's failing. I'll try the suppress option as suggested but was this mentioned anywhere in breaking changes?

@rburnham-def
Copy link

suppressTransaction:true fixes that migration, it looks like it commits all migration up until that one. But now it's failing on a new migration because we try to drop a table that was created in a previous migration but not committed yet.

            migrationBuilder.DropTable(
                name: "FileMigrationLogs",
                schema: "dbo");

// ----------
System.Collections.Generic.KeyNotFoundException: The given key '(FileMigrationLogs, dbo)' was not present in the dictionary.
   at Microsoft.EntityFrameworkCore.Migrations.SqlServerMigrationsSqlGenerator.RewriteOperations(IReadOnlyList`1 migrationOperations, IModel model, MigrationsSqlGenerationOptions options)
   at Microsoft.EntityFrameworkCore.Migrations.SqlServerMigrationsSqlGenerator.Generate(IReadOnlyList`1 operations, IModel model, MigrationsSqlGenerationOptions options)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.GenerateUpSql(Migration migration, MigrationsSqlGenerationOptions options)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.<>c__DisplayClass24_2.<GetMigrationCommandLists>b__2()
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.MigrateImplementation(DbContext context, String targetMigration, MigrationExecutionState state, Boolean useTransaction)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.<>c.<Migrate>b__20_1(DbContext c, ValueTuple`4 s)
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.Migrate(String targetMigration)
   at Microsoft.EntityFrameworkCore.RelationalDatabaseFacadeExtensions.Migrate(DatabaseFacade databaseFacade)
   at MyApp.MigrationManager.MigrateDatabase(IHost host) in D:\Source\MyApp\App_Start\MigrationManager.cs:line 55
   at MyApp.Program.ExecuteApp(String[] args, CommandOption migrateOnly) in D:\Source\MyAp\Program.cs:line 61

If i comment out this migration it applies the rest ok.

@roji
Copy link
Member

roji commented Jan 13, 2025

Reopening, for EF 10 we'll be changing the behavior to wrap each individual migration with a transaction, rather than all migrations. This is similar to how migrations functioned in EF 8 and below, but we'll be keeping the locking behavior introduced in 9.0, and was the main point of the exercise. Implementation details are TBD, but for cases where the lock is disconnected from the transaction (e.g. SQL Server), we should be able to still keep the lock around all migrations; for other cases (e.g. PG) where the locking mechanism is bound to the transaction, the lock will have to be retaken after every migration, and the migration history table reloaded.

If we see significant evidence that the current situation is disruptive to a large number of users, we may consider backporting this change to 9.0 as well - but that's not the current plan.

@AndriySvyryd tentatively assigning to you as you're the expert and did the work here, but we can discuss.

@roji roji reopened this Jan 13, 2025
@roji roji added this to the 10.0.0 milestone Jan 13, 2025
@matthiaslischka

This comment has been minimized.

@roji

This comment has been minimized.

@roji
Copy link
Member

roji commented Feb 4, 2025

Note another case in #35576.

Summary of known problems so far with having the same migration spanning all migrations:

  • SQL Server: Trying to create a TVP and to declare a variable with that TVP in a transaction fails on SQL Server because of a deadlock (link).
  • PostgreSQL: Altering an enum to add a label, and then use that label within the same transaction (link).
  • The same variable names can't be used across migrations, since variable names need to be unique within the transaction (link).
  • Adding a column in an earlier migration and trying to update it in a later one (link).

@AndriySvyryd AndriySvyryd marked this as a duplicate of #35662 Feb 24, 2025
@leonardochaia
Copy link

If we see significant evidence that the current situation is disruptive to a large number of users, we may consider backporting this change to 9.0 as well - but that's not the current plan.

Hi all, @roji , just wanted to add that we are facing a similar issue which we think is related to this issue.
We have a migration that adds a column to a table, and then updates the new column with values. This kind of behavior used to work on net8 but is failing on net9 due to missing GO statements.

migrationBuilder.AddColumn<Guid>(
                name: "AuditOperationCorrelationId",
                schema: "AuditTrails",
                table: "AuditLine",
                type: "uniqueidentifier",
                nullable: false,
                defaultValue: Guid.Empty);

            migrationBuilder.Sql("UPDATE <redacted>");

Is there any workaround we can do either on the efcore cli invocation or in the application's source code?
We have ~100 microservices for which we generate migrations and having a solid workaround would be very useful until net10 fix is available.

Thank you.
Leo

@roji
Copy link
Member

roji commented Mar 5, 2025

@leonardochaia thanks, I've added your case to the list just above. We're already intending to change this behavior for EF 10 (linkj), see above for working around this in 9 with suppressTransaction.

@AndriySvyryd AndriySvyryd added the priority-bug Issues which requires API breaks and have bigger impact hence should be fixed earlier in the release label Mar 17, 2025
@maumar maumar marked this as a duplicate of #35731 Mar 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-migrations customer-reported priority-bug Issues which requires API breaks and have bigger impact hence should be fixed earlier in the release type-bug
Projects
None yet
Development

No branches or pull requests

10 participants