diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index a70dedb1688..07f1ec9ca97 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -287,7 +287,9 @@ protected virtual IReadOnlyList> TopologicalS AddUniqueValueEdges(modificationCommandGraph); - return modificationCommandGraph.BatchingTopologicalSort(FormatCycle); + AddSameTableEdges(modificationCommandGraph); + + return modificationCommandGraph.BatchingTopologicalSort(static (_, _, edges) => edges.All(e => e is IEntityType), FormatCycle); } private string FormatCycle(IReadOnlyList>> data) @@ -765,5 +767,32 @@ private void AddUniqueValueEdges(Multigraph modificationCommandGraph) + { + var deletedDictionary = new Dictionary<(string, string?), List>(); + + foreach (var command in modificationCommandGraph.Vertices) + { + if (command.EntityState == EntityState.Deleted) + { + deletedDictionary.GetOrAddNew((command.TableName, command.Schema)).Add(command); + } + } + + foreach (var command in modificationCommandGraph.Vertices) + { + if (command.EntityState == EntityState.Added) + { + if (deletedDictionary.TryGetValue((command.TableName, command.Schema), out var deletedList)) + { + foreach (var deleted in deletedList) + { + modificationCommandGraph.AddEdge(deleted, command, command.Entries[0].EntityType); + } + } + } + } + } } } diff --git a/src/Shared/Multigraph.cs b/src/Shared/Multigraph.cs index 93c69e95033..f3b6a0437bb 100644 --- a/src/Shared/Multigraph.cs +++ b/src/Shared/Multigraph.cs @@ -136,123 +136,123 @@ public IReadOnlyList TopologicalSort( Func>>, string>? formatCycle, Func? formatException = null) { - var sortedQueue = new List(); - var predecessorCounts = new Dictionary(); - - foreach (var vertex in _vertices) + var queue = new List(); + var predecessorCounts = new Dictionary(_predecessorMap.Count); + foreach (var predecessor in _predecessorMap) { - foreach (var outgoingNeighbor in GetOutgoingNeighbors(vertex)) - { - if (predecessorCounts.ContainsKey(outgoingNeighbor)) - { - predecessorCounts[outgoingNeighbor]++; - } - else - { - predecessorCounts[outgoingNeighbor] = 1; - } - } + predecessorCounts[predecessor.Key] = predecessor.Value.Count; } foreach (var vertex in _vertices) { if (!predecessorCounts.ContainsKey(vertex)) { - sortedQueue.Add(vertex); + queue.Add(vertex); } } var index = 0; - while (sortedQueue.Count < _vertices.Count) + while (queue.Count < _vertices.Count) { - while (index < sortedQueue.Count) + while (index < queue.Count) { - var currentRoot = sortedQueue[index]; + var currentRoot = queue[index]; + index++; - foreach (var successor in GetOutgoingNeighbors(currentRoot).Where(neighbor => predecessorCounts.ContainsKey(neighbor))) + foreach (var successor in GetOutgoingNeighbors(currentRoot)) { - // Decrement counts for edges from sorted vertices and append any vertices that no longer have predecessors predecessorCounts[successor]--; if (predecessorCounts[successor] == 0) { - sortedQueue.Add(successor); - predecessorCounts.Remove(successor); + queue.Add(successor); } } - - index++; } // Cycle breaking - if (sortedQueue.Count < _vertices.Count) + if (queue.Count < _vertices.Count) { var broken = false; var candidateVertices = predecessorCounts.Keys.ToList(); var candidateIndex = 0; - // Iterate over the unsorted vertices while ((candidateIndex < candidateVertices.Count) && !broken && tryBreakEdge != null) { var candidateVertex = candidateVertices[candidateIndex]; + if (predecessorCounts[candidateVertex] != 1) + { + candidateIndex++; + continue; + } - // Find vertices in the unsorted portion of the graph that have edges to the candidate - var incomingNeighbors = GetIncomingNeighbors(candidateVertex) - .Where(neighbor => predecessorCounts.ContainsKey(neighbor)).ToList(); + // Find a vertex in the unsorted portion of the graph that has edges to the candidate + var incomingNeighbor = GetIncomingNeighbors(candidateVertex) + .First(neighbor => predecessorCounts.TryGetValue(neighbor, out var neighborPredecessors) + && neighborPredecessors > 0); - foreach (var incomingNeighbor in incomingNeighbors) + if (tryBreakEdge(incomingNeighbor, candidateVertex, _successorMap[incomingNeighbor][candidateVertex])) { - // Check to see if the edge can be broken - if (tryBreakEdge(incomingNeighbor, candidateVertex, _successorMap[incomingNeighbor][candidateVertex])) - { - predecessorCounts[candidateVertex]--; - if (predecessorCounts[candidateVertex] == 0) - { - sortedQueue.Add(candidateVertex); - predecessorCounts.Remove(candidateVertex); - broken = true; - break; - } - } + _successorMap[incomingNeighbor].Remove(candidateVertex); + _predecessorMap[candidateVertex].Remove(incomingNeighbor); + predecessorCounts[candidateVertex]--; + queue.Add(candidateVertex); + broken = true; + break; } candidateIndex++; } - if (!broken) + if (broken) { - // Failed to break the cycle - var currentCycleVertex = _vertices.First(v => predecessorCounts.ContainsKey(v)); - var cycle = new List { currentCycleVertex }; - var finished = false; - while (!finished) + continue; + } + + var currentCycleVertex = _vertices.First( + v => predecessorCounts.TryGetValue(v, out var predecessorCount) && predecessorCount != 0); + var cycle = new List { currentCycleVertex }; + var finished = false; + while (!finished) + { + foreach (var predecessor in GetIncomingNeighbors(currentCycleVertex)) { - // Find a cycle - foreach (var predecessor in GetIncomingNeighbors(currentCycleVertex) - .Where(neighbor => predecessorCounts.ContainsKey(neighbor))) + if (!predecessorCounts.TryGetValue(predecessor, out var predecessorCount) + || predecessorCount == 0) { - if (predecessorCounts[predecessor] != 0) - { - predecessorCounts[currentCycleVertex] = -1; - - currentCycleVertex = predecessor; - cycle.Add(currentCycleVertex); - finished = predecessorCounts[predecessor] == -1; - break; - } + continue; } + + predecessorCounts[currentCycleVertex] = -1; + + currentCycleVertex = predecessor; + cycle.Add(currentCycleVertex); + finished = predecessorCounts[predecessor] == -1; + break; } + } + + cycle.Reverse(); - cycle.Reverse(); + // Remove any tail that's not part of the cycle + var startingVertex = cycle[0]; + for (var i = cycle.Count - 1; i >= 0; i--) + { + if (cycle[i].Equals(startingVertex)) + { + break; + } - ThrowCycle(cycle, formatCycle, formatException); + cycle.RemoveAt(i); } + + ThrowCycle(cycle, formatCycle, formatException); } } - return sortedQueue; + return queue; } private void ThrowCycle( @@ -287,27 +287,17 @@ private void ThrowCycle( => vertex.ToString(); public IReadOnlyList> BatchingTopologicalSort() - => BatchingTopologicalSort(null); + => BatchingTopologicalSort(null, null); public IReadOnlyList> BatchingTopologicalSort( + Func, bool>? tryBreakEdge, Func>>, string>? formatCycle) { var currentRootsQueue = new List(); - var predecessorCounts = new Dictionary(); - - foreach (var vertex in _vertices) + var predecessorCounts = new Dictionary(_predecessorMap.Count); + foreach (var predecessor in _predecessorMap) { - foreach (var outgoingNeighbor in GetOutgoingNeighbors(vertex)) - { - if (predecessorCounts.ContainsKey(outgoingNeighbor)) - { - predecessorCounts[outgoingNeighbor]++; - } - else - { - predecessorCounts[outgoingNeighbor] = 1; - } - } + predecessorCounts[predecessor.Key] = predecessor.Value.Count; } foreach (var vertex in _vertices) @@ -320,85 +310,121 @@ public IReadOnlyList> BatchingTopologicalSort( var result = new List>(); var nextRootsQueue = new List(); - var currentRootIndex = 0; - while (currentRootIndex < currentRootsQueue.Count) + while (result.Sum(b => b.Count) != _vertices.Count) { - var currentRoot = currentRootsQueue[currentRootIndex]; - currentRootIndex++; - - // Remove edges from current root and add any exposed vertices to the next batch - foreach (var successor in GetOutgoingNeighbors(currentRoot)) + var currentRootIndex = 0; + while (currentRootIndex < currentRootsQueue.Count) { - predecessorCounts[successor]--; - if (predecessorCounts[successor] == 0) + var currentRoot = currentRootsQueue[currentRootIndex]; + currentRootIndex++; + + foreach (var successor in GetOutgoingNeighbors(currentRoot)) { - nextRootsQueue.Add(successor); + predecessorCounts[successor]--; + if (predecessorCounts[successor] == 0) + { + nextRootsQueue.Add(successor); + } } - } - // Roll lists over for next batch - if (currentRootIndex == currentRootsQueue.Count) - { - result.Add(currentRootsQueue); + // Roll lists over for next batch + if (currentRootIndex == currentRootsQueue.Count) + { + result.Add(currentRootsQueue); - currentRootsQueue = nextRootsQueue; - currentRootIndex = 0; + currentRootsQueue = nextRootsQueue; + currentRootIndex = 0; - if (currentRootsQueue.Count != 0) - { - nextRootsQueue = new List(); + if (currentRootsQueue.Count != 0) + { + nextRootsQueue = new List(); + } } } - } - if (result.Sum(b => b.Count) != _vertices.Count) - { - var currentCycleVertex = _vertices.First( - v => predecessorCounts.TryGetValue(v, out var predecessorNumber) && predecessorNumber != 0); - var cyclicWalk = new List { currentCycleVertex }; - var finished = false; - while (!finished) + // Cycle breaking + if (result.Sum(b => b.Count) != _vertices.Count) { - foreach (var predecessor in GetIncomingNeighbors(currentCycleVertex)) + var broken = false; + + var candidateVertices = predecessorCounts.Keys.ToList(); + var candidateIndex = 0; + + while ((candidateIndex < candidateVertices.Count) + && !broken + && tryBreakEdge != null) { - if (!predecessorCounts.TryGetValue(predecessor, out var predecessorCount)) + var candidateVertex = candidateVertices[candidateIndex]; + if (predecessorCounts[candidateVertex] != 1) { + candidateIndex++; continue; } - if (predecessorCount != 0) + // Find a vertex in the unsorted portion of the graph that has edges to the candidate + var incomingNeighbor = GetIncomingNeighbors(candidateVertex) + .First(neighbor => predecessorCounts.TryGetValue(neighbor, out var neighborPredecessors) + && neighborPredecessors > 0); + + if (tryBreakEdge(incomingNeighbor, candidateVertex, _successorMap[incomingNeighbor][candidateVertex])) + { + _successorMap[incomingNeighbor].Remove(candidateVertex); + _predecessorMap[candidateVertex].Remove(incomingNeighbor); + predecessorCounts[candidateVertex]--; + currentRootsQueue.Add(candidateVertex); + nextRootsQueue = new List(); + broken = true; + break; + } + + candidateIndex++; + } + + if (broken) + { + continue; + } + + var currentCycleVertex = _vertices.First( + v => predecessorCounts.TryGetValue(v, out var predecessorCount) && predecessorCount != 0); + var cycle = new List { currentCycleVertex }; + var finished = false; + while (!finished) + { + foreach (var predecessor in GetIncomingNeighbors(currentCycleVertex)) { + if (!predecessorCounts.TryGetValue(predecessor, out var predecessorCount) + || predecessorCount == 0) + { + continue; + } + predecessorCounts[currentCycleVertex] = -1; currentCycleVertex = predecessor; - cyclicWalk.Add(currentCycleVertex); + cycle.Add(currentCycleVertex); finished = predecessorCounts[predecessor] == -1; break; } } - } - cyclicWalk.Reverse(); + cycle.Reverse(); - var cycle = new List(); - var startingVertex = cyclicWalk.First(); - cycle.Add(startingVertex); - foreach (var vertex in cyclicWalk.Skip(1)) - { - if (!vertex.Equals(startingVertex)) - { - cycle.Add(vertex); - } - else + // Remove any tail that's not part of the cycle + var startingVertex = cycle[0]; + for (var i = cycle.Count - 1; i >= 0; i--) { - break; - } - } + if (cycle[i].Equals(startingVertex)) + { + break; + } - cycle.Add(startingVertex); + cycle.RemoveAt(i); + } - ThrowCycle(cycle, formatCycle); + ThrowCycle(cycle, formatCycle); + } } return result; diff --git a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs index 69c96bd73e2..45aec7aaa3b 100644 --- a/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs +++ b/test/EFCore.Relational.Tests/Migrations/Internal/MigrationsModelDifferTest.cs @@ -333,13 +333,10 @@ public void Model_differ_breaks_double_foreign_key_cycles_in_create_table_operat }, result => { - Assert.Equal(14, result.Count); - - var createBankTableOperation = Assert.IsType(result[0]); - Assert.Equal("Banks", createBankTableOperation.Name); - Assert.Empty(createBankTableOperation.ForeignKeys); - - Assert.Equal(4, result.OfType().Count()); + Assert.Equal(3, result.OfType().Count()); + Assert.Equal(7, result.OfType().Count()); + Assert.Equal(7, result.OfType().SelectMany(t => t.ForeignKeys).Count() + + result.OfType().Count()); }); } diff --git a/test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs b/test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs index 4fa323a94c0..889e056dc92 100644 --- a/test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs +++ b/test/EFCore.SqlServer.FunctionalTests/BatchingTest.cs @@ -169,6 +169,73 @@ public void Insertion_order_is_preserved(int maxBatchSize) }); } + [ConditionalFact] + public void Deadlock_on_inserts_and_deletes_with_dependents_is_handled_correctly() + { + var blogs = new List(); + + using (var context = CreateContext()) + { + var owner1 = new Owner { Name = "0" }; + var owner2 = new Owner { Name = "1" }; + context.Owners.Add(owner1); + context.Owners.Add(owner2); + + blogs.Add(new Blog + { + Id = Guid.NewGuid(), + Owner = owner1, + Order = 1 + }); + blogs.Add(new Blog + { + Id = Guid.NewGuid(), + Owner = owner2, + Order = 2 + }); + blogs.Add(new Blog + { + Id = Guid.NewGuid(), + Owner = owner1, + Order = 3 + }); + blogs.Add(new Blog + { + Id = Guid.NewGuid(), + Owner = owner2, + Order = 4 + }); + + context.AddRange(blogs); + + context.SaveChanges(); + } + + for (var i = 0; i < 10; i++) + { + Parallel.ForEach(blogs, blog => + { + RemoveAndAddPosts(blog); + }); + } + + void RemoveAndAddPosts(Blog blog) + { + using var context = (BloggingContext)Fixture.CreateContext(useConnectionString: true); + + context.Attach(blog); + blog.Posts.Clear(); + + blog.Posts.Add(new Post { Comments = { new Comment() } }); + blog.Posts.Add(new Post { Comments = { new Comment() } }); + blog.Posts.Add(new Post { Comments = { new Comment() } }); + + context.SaveChanges(); + } + + Fixture.Reseed(); + } + [ConditionalFact] public void Deadlock_on_deletes_with_dependents_is_handled_correctly() { diff --git a/test/EFCore.Tests/Utilities/MultigraphTest.cs b/test/EFCore.Tests/Utilities/MultigraphTest.cs index 1ed04245ed3..3310ab14ffe 100644 --- a/test/EFCore.Tests/Utilities/MultigraphTest.cs +++ b/test/EFCore.Tests/Utilities/MultigraphTest.cs @@ -434,7 +434,7 @@ string formatter(IEnumerable>> data) Assert.Equal( CoreStrings.CircularDependency(message), - Assert.Throws(() => graph.BatchingTopologicalSort(formatter)).Message); + Assert.Throws(() => graph.BatchingTopologicalSort(null, formatter)).Message); Assert.Equal(3, cycleData.Count()); @@ -485,7 +485,7 @@ string formatter(IEnumerable>> data) Assert.Equal( CoreStrings.CircularDependency(message), - Assert.Throws(() => graph.BatchingTopologicalSort(formatter)).Message); + Assert.Throws(() => graph.BatchingTopologicalSort(null, formatter)).Message); Assert.Equal(2, cycleData.Count);