Skip to content
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

Stop wrapping single changes in transactions where possible #27500

Merged
merged 7 commits into from
Mar 4, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2189,7 +2189,7 @@ private IEnumerable<MigrationOperation> GetDataOperations(
var commandBatches = new CommandBatchPreparer(CommandBatchPreparerDependencies)
.BatchCommands(entries, updateAdapter);

foreach (var commandBatch in commandBatches)
foreach (var (commandBatch, _) in commandBatches)
{
InsertDataOperation? batchInsertOperation = null;
foreach (var command in commandBatch.ModificationCommands)
Expand Down
12 changes: 12 additions & 0 deletions src/EFCore.Relational/Properties/RelationalStrings.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions src/EFCore.Relational/Properties/RelationalStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,12 @@
<data name="MissingParameterValue" xml:space="preserve">
<value>No value was provided for the required parameter '{parameter}'.</value>
</data>
<data name="ModificationCommandBatchAlreadyComplete" xml:space="preserve">
<value>Cannot add commands to a completed ModificationCommandBatch.</value>
</data>
<data name="ModificationCommandBatchNotComplete" xml:space="preserve">
<value>Cannot execute an ModificationCommandBatch which hasn't been completed.</value>
</data>
<data name="ModificationCommandInvalidEntityState" xml:space="preserve">
<value>Cannot save changes for an entity of type '{entityType}' in state '{entityState}'. This may indicate a bug in Entity Framework, please open an issue at https://go.microsoft.com/fwlink/?linkid=2142044. Consider using 'DbContextOptionsBuilder.EnableSensitiveDataLogging' to see the key values of the entity.</value>
</data>
Expand Down
12 changes: 8 additions & 4 deletions src/EFCore.Relational/Update/IBatchExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,17 +28,21 @@ public interface IBatchExecutor
/// <summary>
/// Executes the commands in the batches against the given database connection.
/// </summary>
/// <param name="commandBatches">The batches to execute.</param>
/// <param name="commandBatches">
/// A list of value tuples, each of which contains a batch to execute, and whether more batches are available.
/// </param>
/// <param name="connection">The database connection to use.</param>
/// <returns>The total number of rows affected.</returns>
int Execute(
IEnumerable<ModificationCommandBatch> commandBatches,
IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> commandBatches,
IRelationalConnection connection);

/// <summary>
/// Executes the commands in the batches against the given database connection.
/// </summary>
/// <param name="commandBatches">The batches to execute.</param>
/// <param name="commandBatches">
/// A list of value tuples, each of which contains a batch to execute, and whether more batches are available.
/// </param>
/// <param name="connection">The database connection to use.</param>
/// <param name="cancellationToken">A <see cref="CancellationToken" /> to observe while waiting for the task to complete.</param>
/// <returns>
Expand All @@ -47,7 +51,7 @@ int Execute(
/// </returns>
/// <exception cref="OperationCanceledException">If the <see cref="CancellationToken" /> is canceled.</exception>
Task<int> ExecuteAsync(
IEnumerable<ModificationCommandBatch> commandBatches,
IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> commandBatches,
IRelationalConnection connection,
CancellationToken cancellationToken = default);
}
6 changes: 2 additions & 4 deletions src/EFCore.Relational/Update/ICommandBatchPreparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ public interface ICommandBatchPreparer
/// </summary>
/// <param name="entries">The entries that represent the entities to be modified.</param>
/// <param name="updateAdapter">The model data.</param>
/// <returns>The list of batches to execute.</returns>
IEnumerable<ModificationCommandBatch> BatchCommands(
IList<IUpdateEntry> entries,
IUpdateAdapter updateAdapter);
/// <returns>A list of value tuples, each of which contains a batch to execute, and whether more batches are available.</returns>
IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> BatchCommands(IList<IUpdateEntry> entries, IUpdateAdapter updateAdapter);
}
57 changes: 54 additions & 3 deletions src/EFCore.Relational/Update/IUpdateSqlGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,26 @@ void AppendNextSequenceValueOperation(
/// <param name="commandStringBuilder">The builder to which the SQL fragment should be appended.</param>
void AppendBatchHeader(StringBuilder commandStringBuilder);

/// <summary>
/// Prepends a SQL command for turning on autocommit mode in the database, in case it is off.
/// </summary>
/// <param name="commandStringBuilder">The builder to which the SQL should be prepended.</param>
void PrependEnsureAutocommit(StringBuilder commandStringBuilder);

/// <summary>
/// Appends a SQL command for deleting a row to the commands being built.
/// </summary>
/// <param name="commandStringBuilder">The builder to which the SQL should be appended.</param>
/// <param name="command">The command that represents the delete operation.</param>
/// <param name="commandPosition">The ordinal of this command in the batch.</param>
/// <param name="requiresTransaction">Returns whether the SQL appended must be executed in a transaction to work correctly.</param>
/// <returns>The <see cref="ResultSetMapping" /> for the command.</returns>
ResultSetMapping AppendDeleteOperation(
StringBuilder commandStringBuilder,
IReadOnlyModificationCommand command,
int commandPosition,
out bool requiresTransaction);

/// <summary>
/// Appends a SQL command for deleting a row to the commands being built.
/// </summary>
Expand All @@ -64,7 +84,22 @@ void AppendNextSequenceValueOperation(
ResultSetMapping AppendDeleteOperation(
StringBuilder commandStringBuilder,
IReadOnlyModificationCommand command,
int commandPosition);
int commandPosition)
=> AppendDeleteOperation(commandStringBuilder, command, commandPosition, out _);

/// <summary>
/// Appends a SQL command for inserting a row to the commands being built.
/// </summary>
/// <param name="commandStringBuilder">The builder to which the SQL should be appended.</param>
/// <param name="command">The command that represents the delete operation.</param>
/// <param name="commandPosition">The ordinal of this command in the batch.</param>
/// <param name="requiresTransaction">Returns whether the SQL appended must be executed in a transaction to work correctly.</param>
/// <returns>The <see cref="ResultSetMapping" /> for the command.</returns>
ResultSetMapping AppendInsertOperation(
StringBuilder commandStringBuilder,
IReadOnlyModificationCommand command,
int commandPosition,
out bool requiresTransaction);

/// <summary>
/// Appends a SQL command for inserting a row to the commands being built.
Expand All @@ -76,7 +111,22 @@ ResultSetMapping AppendDeleteOperation(
ResultSetMapping AppendInsertOperation(
StringBuilder commandStringBuilder,
IReadOnlyModificationCommand command,
int commandPosition);
int commandPosition)
=> AppendInsertOperation(commandStringBuilder, command, commandPosition, out _);

/// <summary>
/// Appends a SQL command for updating a row to the commands being built.
/// </summary>
/// <param name="commandStringBuilder">The builder to which the SQL should be appended.</param>
/// <param name="command">The command that represents the delete operation.</param>
/// <param name="commandPosition">The ordinal of this command in the batch.</param>
/// <param name="requiresTransaction">Returns whether the SQL appended must be executed in a transaction to work correctly.</param>
/// <returns>The <see cref="ResultSetMapping" /> for the command.</returns>
ResultSetMapping AppendUpdateOperation(
StringBuilder commandStringBuilder,
IReadOnlyModificationCommand command,
int commandPosition,
out bool requiresTransaction);

/// <summary>
/// Appends a SQL command for updating a row to the commands being built.
Expand All @@ -88,5 +138,6 @@ ResultSetMapping AppendInsertOperation(
ResultSetMapping AppendUpdateOperation(
StringBuilder commandStringBuilder,
IReadOnlyModificationCommand command,
int commandPosition);
int commandPosition)
=> AppendUpdateOperation(commandStringBuilder, command, commandPosition, out _);
}
38 changes: 32 additions & 6 deletions src/EFCore.Relational/Update/Internal/BatchExecutor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,18 @@ public BatchExecutor(
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual int Execute(
IEnumerable<ModificationCommandBatch> commandBatches,
IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> commandBatches,
IRelationalConnection connection)
{
using var batchEnumerator = commandBatches.GetEnumerator();

if (!batchEnumerator.MoveNext())
{
return 0;
}

var (batch, hasMoreBatches) = batchEnumerator.Current;

var rowsAffected = 0;
var transaction = connection.CurrentTransaction;
var beganTransaction = false;
Expand All @@ -62,7 +71,9 @@ public virtual int Execute(
if (transaction == null
&& transactionEnlistManager?.EnlistedTransaction is null
&& transactionEnlistManager?.CurrentAmbientTransaction is null
&& CurrentContext.Context.Database.AutoTransactionsEnabled)
&& CurrentContext.Context.Database.AutoTransactionsEnabled
// Don't start a transaction if we have a single batch which doesn't require a transaction (single command), for perf.
&& (hasMoreBatches || batch.RequiresTransaction))
{
transaction = connection.BeginTransaction();
beganTransaction = true;
Expand All @@ -79,11 +90,13 @@ public virtual int Execute(
}
}

foreach (var batch in commandBatches)
do
{
batch = batchEnumerator.Current.Batch;
batch.Execute(connection);
rowsAffected += batch.ModificationCommands.Count;
}
while (batchEnumerator.MoveNext());

if (beganTransaction)
{
Expand Down Expand Up @@ -143,10 +156,19 @@ public virtual int Execute(
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
public virtual async Task<int> ExecuteAsync(
IEnumerable<ModificationCommandBatch> commandBatches,
IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> commandBatches,
IRelationalConnection connection,
CancellationToken cancellationToken = default)
{
using var batchEnumerator = commandBatches.GetEnumerator();

if (!batchEnumerator.MoveNext())
{
return 0;
}

var (batch, hasMoreBatches) = batchEnumerator.Current;

var rowsAffected = 0;
var transaction = connection.CurrentTransaction;
var beganTransaction = false;
Expand All @@ -157,7 +179,9 @@ public virtual async Task<int> ExecuteAsync(
if (transaction == null
&& transactionEnlistManager?.EnlistedTransaction is null
&& transactionEnlistManager?.CurrentAmbientTransaction is null
&& CurrentContext.Context.Database.AutoTransactionsEnabled)
&& CurrentContext.Context.Database.AutoTransactionsEnabled
// Don't start a transaction if we have a single batch which doesn't require a transaction (single command), for perf.
&& (hasMoreBatches || batch.RequiresTransaction))
{
transaction = await connection.BeginTransactionAsync(cancellationToken).ConfigureAwait(false);
beganTransaction = true;
Expand All @@ -174,11 +198,13 @@ public virtual async Task<int> ExecuteAsync(
}
}

foreach (var batch in commandBatches)
do
{
batch = batchEnumerator.Current.Batch;
await batch.ExecuteAsync(connection, cancellationToken).ConfigureAwait(false);
rowsAffected += batch.ModificationCommands.Count;
}
while (batchEnumerator.MoveNext());

if (beganTransaction)
{
Expand Down
28 changes: 21 additions & 7 deletions src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,18 @@ public CommandBatchPreparer(CommandBatchPreparerDependencies dependencies)
/// 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 virtual IEnumerable<ModificationCommandBatch> BatchCommands(
public virtual IEnumerable<(ModificationCommandBatch Batch, bool HasMore)> BatchCommands(
IList<IUpdateEntry> entries,
IUpdateAdapter updateAdapter)
{
var parameterNameGenerator = Dependencies.ParameterNameGeneratorFactory.Create();
var commands = CreateModificationCommands(entries, updateAdapter, parameterNameGenerator.GenerateNext);
var sortedCommandSets = TopologicalSort(commands);

foreach (var independentCommandSet in sortedCommandSets)
for (var commandSetIndex = 0; commandSetIndex < sortedCommandSets.Count; commandSetIndex++)
{
var independentCommandSet = sortedCommandSets[commandSetIndex];

independentCommandSet.Sort(Dependencies.ModificationCommandComparer);

var batch = Dependencies.ModificationCommandBatchFactory.Create();
Expand All @@ -83,7 +85,9 @@ public virtual IEnumerable<ModificationCommandBatch> BatchCommands(
batch.ModificationCommands.SelectMany(c => c.Entries), batch.ModificationCommands.Count);
}

yield return batch;
batch.Complete();

yield return (batch, true);
}
else
{
Expand All @@ -92,14 +96,19 @@ public virtual IEnumerable<ModificationCommandBatch> BatchCommands(

foreach (var command in batch.ModificationCommands)
{
yield return StartNewBatch(parameterNameGenerator, command);
batch = StartNewBatch(parameterNameGenerator, command);
batch.Complete();

yield return (batch, true);
}
}

batch = StartNewBatch(parameterNameGenerator, modificationCommand);
}
}

var hasMoreCommandSets = commandSetIndex < sortedCommandSets.Count - 1;

if (batch.ModificationCommands.Count == 1
|| batch.ModificationCommands.Count >= _minBatchSize)
{
Expand All @@ -109,16 +118,21 @@ public virtual IEnumerable<ModificationCommandBatch> BatchCommands(
batch.ModificationCommands.SelectMany(c => c.Entries), batch.ModificationCommands.Count);
}

yield return batch;
batch.Complete();

yield return (batch, hasMoreCommandSets);
}
else
{
Dependencies.UpdateLogger.BatchSmallerThanMinBatchSize(
batch.ModificationCommands.SelectMany(c => c.Entries), batch.ModificationCommands.Count, _minBatchSize);

foreach (var command in batch.ModificationCommands)
for (var commandIndex = 0; commandIndex < batch.ModificationCommands.Count; commandIndex++)
{
yield return StartNewBatch(parameterNameGenerator, command);
var singleCommandBatch = StartNewBatch(parameterNameGenerator, batch.ModificationCommands[commandIndex]);
singleCommandBatch.Complete();

yield return (singleCommandBatch, hasMoreCommandSets || commandIndex < batch.ModificationCommands.Count - 1);
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions src/EFCore.Relational/Update/ModificationCommandBatch.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@ public abstract class ModificationCommandBatch
/// </returns>
public abstract bool AddCommand(IReadOnlyModificationCommand modificationCommand);

/// <summary>
/// Indicates that no more commands will be added to this batch, and prepares it for execution.
/// </summary>
public abstract void Complete();

/// <summary>
/// Indicates whether the batch requires a transaction in order to execute correctly.
/// </summary>
public abstract bool RequiresTransaction { get; }

/// <summary>
/// Sends insert/update/delete commands to the database.
/// </summary>
Expand Down
Loading