Deadlock when adding multiple entities

[sam-wheat]

I don't know if this is a problem with EF or I am just doing it wrong (more often the latter).

If I am doing it wrong I would like to know why the code works when I save each entity individually as opposed to saving all of them at once outside the loop. As is shown in the attached project, setting the isolation level appears to have no effect, nor does removing the lookup that happens for each row.

// Save the Releases to the database
using (Db db = new Db(dbContextOptions))
{
	// does not work:
	// db.Database.ExecuteSqlCommand("SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;");

	foreach (Release rel in releases)
	{
		// omitting the following line has no effect WRT error
		await db.Releases.AnyAsync(x => x.NativeID == rel.NativeID && x.DataProviderID == rel.DataProviderID && x.ID != rel.ID);

		db.Entry(rel).State = EntityState.Added;

		foreach (SourceRelease sr in rel.SourceReleases)
			db.Entry(sr).State = EntityState.Added;

	
		//await db.SaveChangesAsync();  // WORKAROUND: works if we save each entity here
		
	}

	// ...
	// Inner Exception 3:
	// SqlException: Transaction(Process ID 55) was deadlocked on lock resources with another process 
	// and has been chosen as the deadlock victim. Rerun the transaction.
	
	await db.SaveChangesAsync();  // error here.   WORKAROUND:  comment this out and save inside the loop
}

InsertTest.zip

[antonioortizpola]

I do not know why is behaving that way, just some notes:
You can avoid the executeSqlCommand with

using (new TransactionScope(
                    TransactionScopeOption.Required, 
                    new TransactionOptions 
                    { 
                         IsolationLevel = IsolationLevel.ReadUncommitted 
                    })) 
{
        using (var db = new MyDbContext()) { 
            // query
        }
}

Second, you should check if you can do snapshot instead uncommited.

And third, EF does not get along very well with batch inserts and updates, i would recommend to include EF Plus to handle this specific inserts.

[sam-wheat]

Hi Antonio, thanks for your tip on TransactionScope. As stated in the code, I don't think that statement has any effect on the problem being reported.

And third, EF does not get along very well with batch inserts and updates

I think EF should be able to perform well in the code I have provided.

[antonioortizpola]

I know, it should be capable, how many elements are you trying to add? remember that EF will add the entity, then request the last inserted id and keep the entity tracked until the context is closed, making the context more slow after each insert, like here.

On the other hand, this should result in a very slow insertion rather than the deadlock you are experiencing (or, if you are trying to insert a lot of objects).

So, if your list is large, or if is very important that the operation is made quick, i still recommend to try EF Plus. If you list is small nor you care about performance your solution should work or I can not see anything that causes the problem, any help from the people who really know EF that help us could be appreciated!

[sam-wheat]

how many elements are you trying to add?

550 per task x 6 tasks.
Look at the code above you see the context is short lived.

[antonioortizpola]

Ok, i can say for sure, while that is a short lived context in terms of code, is still a very long lived context, since is used for 550 insertions (i guess, one context per thread), where each insertion will cause EF to:

• Save the entity
• Cache the entity and create tracking for the database and entity
• Ask for the saved id

The cache and the entity tracking will cost you a lot, and from what i see not necessary. No matter where you put that SaveChanges, this will cause many performance problems, you should add the instruction to set the context AutoDetectChangesEnabled in false.

Bulk inset is a hard problem, you can check an implementation a little more complete that yours in stackoverflow.

I would not recommend to follow your approach (only if you are doing a very few insertions) since will cause a lot overhead on your app and on your DB. But still, i repeat, i have no idea why your code works with the workaround and breaks without it, but i almost sure both approaches are inefficient and will lead to further problems under heavy or even normal load.

Take what I say with care, I'm no expert, just a user trying to help, I hope someone with better credentials could help us clear this out.

[smitpatel]

@divega - To write about lock escalation.

[divega]

@sam-wheat @antonioortizpola, @smitpatel and I played a bit with the repro and realized what you are seeing is the effect of a feature in SQL Server called lock escalation, which is the process of converting many fine-grained locks (e.g. row locks) into less fine grained locks, e.g. page locks or table locks.

There is some information about how to diagnose deadlocks and the effects of lock escalation at https://docs.microsoft.com/en-us/sql/relational-databases/sql-server-transaction-locking-and-row-versioning-guide#deadlocking.

But the short answer is that you can avoid this lock escalation in two ways:

1. Disable lock escalation in the table causing the deadlock, in this case:

   ALTER TABLE dbo.Releases SET (LOCK_ESCALATION = DISABLE);

2. Tunnig down the size of batches used by EF Core until the escalation goes away. In our testing, a batch size of 128 made it go away:

   optionsBuilder.UseSqlServer(connectionString, sso => sso.MaxBatchSize(128));

[divega]

Note for triage: I am leaving this open to have a chance to investigate/discuss if there is anything we should do to prevent lock escalation from happening by default.

[smitpatel]

Removing milestone so that it shows up during triage.

[divega]

@smitpatel and @divega to do a binary search to find the magic number for max batch size that allows this repro to work.

[smitpatel]

The magic number moves away as the size of table grows.
With empty table the magic numbers comes in the range of 100-112. Due to intermittent nature of the issue, cannot get exact value.

[sam-wheat]

I modified my original upload to do a more formal test of batch size versus task count.
This new code iterates over a range of batch sizes and task counts and saves the result as a .csv file.
Results from three runs I've done are attached.
It appears sql server is happiest with a very small batch size however the data I've gathered does not seem conclusive about task count.
Additional comments and insights are welcome and appreciated.

InsertTest.zip

[divega]

@smitpatel @AndriySvyryd can you check with the data from @sam-wheat if there is a number that makes sense or at least enough information to decide some other action plan? I am not going to be able to check myself for at least a week.

[ajcvickers]

Discussed this in triage and it is looking like 128 could be a reasonable value to use, but we don't think this change needs to be made for 2.1, so moving to the backlog.

[sam-wheat]

What is the current default value please?

[AndriySvyryd]

@sam-wheat There is no default value, we just create the biggest batches possible.

[divega]

Related to #9270.

[gopal-k]

I have a multi threading application that needs to insert bulk of records in the same table from different places. After continuously running 2 Hrs my application gets connection time out issue

Stack Trace
System.Data.SqlClient.SqlException (0x80131904): Timeout expired. The timeout period elapsed prior to completion of the operation or the server is not responding. ---> System.ComponentModel.Win32Exception (258): The wait operation timed out
at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
at System.Data.SqlClient.SqlDataReader.TryConsumeMetaData()
at System.Data.SqlClient.SqlDataReader.get_MetaData()

I suspect this may be the cause of deadlock in a particular table. I verified in SQL that table is on deadlock.

I have tried the following solution

1. using (var scope = new TransactionScope(TransactionScopeOption.Required, new TransactionOptions { IsolationLevel = IsolationLevel.ReadUncommitted }))
2. I tried with SERIALIZABLE and SNAPSHOT isolation levels also

When running the multi threading application it says MSDTC Server on unavailble . Note: I started the Distributed Transaction Coordinator service but still i get your environment not support distributed transactions

I tried with SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED but still i am getting the same issue after 2Hrs running continuously

I added this SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED between the BeginTransaction and Commit transaction in my C# code but no luck.

Could you please someone can help me to overcome from this ?

[roji]

@gopal-k this doesn't seem to belong here - I see you already opened #14743, discussion can continue there.

[SimonCropp]

What version was this fixed in?

[AndriySvyryd]

@SimonCropp It hasn't been fixed yet, you need to set MaxBatchSize manually.

[SimonCropp]

@AndriySvyryd thanks. in that case should this issue be re-opened?

[AndriySvyryd]

@SimonCropp No, this is a duplicate of #9270

[SimonCropp]

@AndriySvyryd thanks again. would you mine placcing the " this is a duplicate of #9270" at the top of the issue description?

[ajcvickers]

@SimonCropp We get a lot of duplicates. They are closed and tagged with the closed-duplicate label, which should make it fairly obvious that the issued is closed as being a duplicate.