Skip to content

Commit

Permalink
Sort inserts to preserve tracking order when possible.
Browse files Browse the repository at this point in the history 
Add a test for deadlocks.

Fixes #15180
Fixes #25228
  • Loading branch information
@AndriySvyryd
AndriySvyryd committed Sep 4, 2021
1 parent 7741131 commit ad6263b
Show file tree
Hide file tree
Showing 4 changed files with 156 additions and 25 deletions.
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ protected virtual IEnumerable<IReadOnlyModificationCommand> CreateModificationCo
foreach (var mapping in mappings)
{
var table = mapping.Table;
var tableKey = (table.Name, table.Schema);

IModificationCommand command;
var isMainEntry = true;
Expand All @@ -189,6 +188,7 @@ protected virtual IEnumerable<IReadOnlyModificationCommand> CreateModificationCo
sharedTablesCommandsMap = new Dictionary<(string, string?), SharedTableEntryMap<IModificationCommand>>();
}

var tableKey = (table.Name, table.Schema);
if (!sharedTablesCommandsMap.TryGetValue(tableKey, out var sharedCommandsMap))
{
sharedCommandsMap = new SharedTableEntryMap<IModificationCommand>(table, updateAdapter);
Expand Down
21 changes: 9 additions & 12 deletions src/EFCore.Relational/Update/Internal/ModificationCommandComparer.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ public virtual int Compare(IReadOnlyModificationCommand? x, IReadOnlyModificatio
return 1;
}

result = StringComparer.Ordinal.Compare(x.Schema, y.Schema);
result = StringComparer.Ordinal.Compare(x.TableName, y.TableName);
if (result != 0)
{
return result;
}

result = StringComparer.Ordinal.Compare(x.TableName, y.TableName);
result = StringComparer.Ordinal.Compare(x.Schema, y.Schema);
if (result != 0)
{
return result;
Expand Down Expand Up @@ -82,18 +82,15 @@ public virtual int Compare(IReadOnlyModificationCommand? x, IReadOnlyModificatio
}
}

if (xState != EntityState.Added)
var xKey = xEntry.EntityType.FindPrimaryKey()!;
for (var i = 0; i < xKey.Properties.Count; i++)
{
var xKey = xEntry.EntityType.FindPrimaryKey()!;
for (var i = 0; i < xKey.Properties.Count; i++)
{
var xKeyProperty = xKey.Properties[i];
var xKeyProperty = xKey.Properties[i];

result = xKeyProperty.GetCurrentValueComparer().Compare(xEntry, yEntry);
if (result != 0)
{
return result;
}
result = xKeyProperty.GetCurrentValueComparer().Compare(xEntry, yEntry);
if (result != 0)
{
return result;
}
}
}
Expand Down
18 changes: 9 additions & 9 deletions test/EFCore.Relational.Specification.Tests/TransactionTestBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public virtual async Task SaveChanges_can_be_used_with_no_transaction(bool async
context.Database.AutoTransactionsEnabled = false;

context.Add(
new TransactionCustomer { Id = 77, Name = "Bobble" });
new TransactionCustomer { Id = -77, Name = "Bobble" });

context.Entry(context.Set<TransactionCustomer>().OrderBy(c => c.Id).Last()).State = EntityState.Added;

Expand All @@ -69,9 +69,9 @@ public virtual async Task SaveChanges_can_be_used_with_no_transaction(bool async
Assert.Equal(
new List<int>
{
-77,
1,
2,
77
},
context.Set<TransactionCustomer>().OrderBy(c => c.Id).Select(e => e.Id).ToList());
}
Expand Down Expand Up @@ -119,7 +119,7 @@ public virtual async Task SaveChanges_uses_enlisted_transaction(bool async, bool
context.Database.AutoTransactionsEnabled = autoTransactionsEnabled;

context.Add(
new TransactionCustomer { Id = 77, Name = "Bobble" });
new TransactionCustomer { Id = -77, Name = "Bobble" });

context.Entry(context.Set<TransactionCustomer>().OrderBy(c => c.Id).Last()).State = EntityState.Added;

Expand Down Expand Up @@ -151,7 +151,7 @@ public virtual async Task SaveChanges_uses_enlisted_transaction(bool async, bool
if (!autoTransactionsEnabled)
{
using var context = CreateContext();
context.Entry(context.Set<TransactionCustomer>().Single(c => c.Id == 77)).State = EntityState.Deleted;
context.Entry(context.Set<TransactionCustomer>().Single(c => c.Id == -77)).State = EntityState.Deleted;

if (async)
{
Expand Down Expand Up @@ -275,7 +275,7 @@ public virtual async Task SaveChanges_uses_ambient_transaction(bool async, bool
context.Database.AutoTransactionsEnabled = autoTransactionsEnabled;

context.Add(
new TransactionCustomer { Id = 77, Name = "Bobble" });
new TransactionCustomer { Id = -77, Name = "Bobble" });

context.Entry(context.Set<TransactionCustomer>().OrderBy(c => c.Id).Last()).State = EntityState.Added;

Expand Down Expand Up @@ -305,7 +305,7 @@ public virtual async Task SaveChanges_uses_ambient_transaction(bool async, bool
Fixture.ListLoggerFactory.Log.Skip(2).First().Message);

using var context = CreateContext();
context.Entry(context.Set<TransactionCustomer>().Single(c => c.Id == 77)).State = EntityState.Deleted;
context.Entry(context.Set<TransactionCustomer>().Single(c => c.Id == -77)).State = EntityState.Deleted;

if (async)
{
Expand Down Expand Up @@ -1268,7 +1268,7 @@ public virtual async Task SaveChanges_can_be_used_with_no_savepoint(bool async)
? await context.Database.BeginTransactionAsync()
: context.Database.BeginTransaction();

context.Add(new TransactionCustomer { Id = 77, Name = "Bobble" });
context.Add(new TransactionCustomer { Id = -77, Name = "Bobble" });

if (async)
{
Expand All @@ -1279,7 +1279,7 @@ public virtual async Task SaveChanges_can_be_used_with_no_savepoint(bool async)
context.SaveChanges();
}

context.Add(new TransactionCustomer { Id = 78, Name = "Hobble" });
context.Add(new TransactionCustomer { Id = -78, Name = "Hobble" });
context.Add(new TransactionCustomer { Id = 1, Name = "Gobble" }); // Cause SaveChanges failure

if (async)
Expand All @@ -1298,7 +1298,7 @@ public virtual async Task SaveChanges_can_be_used_with_no_savepoint(bool async)

using (var context = CreateContext())
{
Assert.Equal(78, context.Set<TransactionCustomer>().Max(c => c.Id));
Assert.Equal(-78, context.Set<TransactionCustomer>().Min(c => c.Id));
}
}

Expand Down
140 changes: 137 additions & 3 deletions test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading.Tasks;
using Microsoft.EntityFrameworkCore.Diagnostics.Internal;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.SqlServer.Diagnostics.Internal;
using Microsoft.EntityFrameworkCore.SqlServer.Infrastructure.Internal;
using Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;
Expand Down Expand Up @@ -125,6 +128,93 @@ public void Inserts_and_updates_are_batched_correctly()
context => AssertDatabaseState(context, true, expectedBlogs));
}

[ConditionalTheory]
[InlineData(1)]
[InlineData(3)]
[InlineData(4)]
[InlineData(100)]
public void Insertion_order_is_preserved(int maxBatchSize)
{
var blogId = new Guid();

TestHelpers.ExecuteWithStrategyInTransaction(
() => (BloggingContext)Fixture.CreateContext(maxBatchSize: maxBatchSize),
UseTransaction,
context =>
{
var owner = new Owner();
var blog = new Blog
{
Owner = owner
};
for (var i = 0; i < 20; i++)
{
context.Add(new Post { Order = i, Blog = blog });
}
context.SaveChanges();
blogId = blog.Id;
},
context =>
{
var posts = context.Set<Post>().Where(p => p.BlogId == blogId).OrderBy(p => p.Order);
var lastId = 0;
foreach (var post in posts)
{
Assert.True(post.PostId > lastId, $"Last ID: {lastId}, current ID: {post.PostId}");
lastId = post.PostId;
}
});
}

[ConditionalFact]
public void Deadlock_on_deletes_with_dependents_is_handled_correctly()
{
var owners = new[] { new Owner { Name = "0" }, new Owner { Name = "1" } };
using (var context = CreateContext())
{
context.Owners.AddRange(owners);

for (var h = 0; h <= 40; h++)
{
var owner = owners[h % 2];
var blog = new Blog
{
Id = Guid.NewGuid(),
Owner = owner,
Order = h
};

for (var i = 0; i <= 40; i++)
{
blog.Posts.Add(new Post { Comments = { new Comment() } });
}

context.Add(blog);
}

context.SaveChanges();
}

Parallel.ForEach(owners, owner =>
{
using var context = (BloggingContext)Fixture.CreateContext(useConnectionString: true);
context.RemoveRange(context.Blogs.Where(b => b.OwnerId == owner.Id));
context.SaveChanges();
});

using (var context = CreateContext())
{
Assert.Empty(context.Blogs);
}

Fixture.Reseed();
}

[ConditionalFact]
public void Inserts_when_database_type_is_different()
{
Expand Down Expand Up @@ -251,6 +341,7 @@ private class Blog
public string OwnerId { get; set; }
public Owner Owner { get; set; }
public byte[] Version { get; set; }
public ICollection<Post> Posts { get; } = new HashSet<Post>();
}

private class Owner
Expand All @@ -260,6 +351,22 @@ private class Owner
public byte[] Version { get; set; }
}

private class Post
{
public int PostId { get; set; }
public int? Order { get; set; }
public Guid BlogId { get; set; }
public Blog Blog { get; set; }
public ICollection<Comment> Comments { get; } = new HashSet<Comment>();
}

private class Comment
{
public int CommentId { get; set; }
public int PostId { get; set; }
public Post Post { get; set; }
}

public class BatchingTestFixture : SharedStoreFixtureBase<PoolableDbContext>
{
protected override string StoreName { get; } = "BatchingTest";
Expand All @@ -284,10 +391,37 @@ ALTER TABLE dbo.Owners
ALTER COLUMN Name nvarchar(MAX);");
}

public DbContext CreateContext(int minBatchSize)
public DbContext CreateContext(
int? minBatchSize = null,
int? maxBatchSize = null,
bool useConnectionString = false,
bool disableConnectionResiliency = false)
{
var optionsBuilder = new DbContextOptionsBuilder(CreateOptions());
new SqlServerDbContextOptionsBuilder(optionsBuilder).MinBatchSize(minBatchSize);
var options = CreateOptions();
var optionsBuilder = new DbContextOptionsBuilder(options);
if (useConnectionString)
{
RelationalOptionsExtension extension = options.FindExtension<SqlServerOptionsExtension>()
?? new SqlServerOptionsExtension();

extension = extension.WithConnection(null).WithConnectionString(((SqlServerTestStore)TestStore).ConnectionString);
((IDbContextOptionsBuilderInfrastructure)optionsBuilder).AddOrUpdateExtension(extension);
}

if (minBatchSize.HasValue)
{
new SqlServerDbContextOptionsBuilder(optionsBuilder).MinBatchSize(minBatchSize.Value);
}

if (maxBatchSize.HasValue)
{
new SqlServerDbContextOptionsBuilder(optionsBuilder).MinBatchSize(maxBatchSize.Value);
}

if (disableConnectionResiliency)
{
new SqlServerDbContextOptionsBuilder(optionsBuilder).ExecutionStrategy(d => new SqlServerExecutionStrategy(d));
}
return new BloggingContext(optionsBuilder.Options);
}
}
Expand Down

0 comments on commit ad6263b

Please sign in to comment.