Skip to content

Commit

Permalink
Disable savepoints on SQL Server when MARS is enabled
Browse files Browse the repository at this point in the history
Fixes #23269
  • Loading branch information
roji committed Nov 13, 2020
1 parent 641d553 commit 43f34bf
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 0 deletions.
1 change: 1 addition & 0 deletions All.sln.DotSettings
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ Licensed under the Apache License, Version 2.0. See License.txt in the project r
<s:String x:Key="/Default/CodeStyle/Naming/CppNaming/UserRules/=PUBLIC_005FSTRUCT_005FFIELD/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="" Suffix="" Style="aa_bb" /&gt;</s:String>
<s:Boolean x:Key="/Default/CodeStyle/Naming/CSharpAutoNaming/IsNotificationDisabled/@EntryValue">True</s:Boolean>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=FK/@EntryIndexedValue">FK</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=MARS/@EntryIndexedValue">MARS</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/EventHandlerPatternLong/@EntryValue">$object$_On$event$</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/PredefinedNamingRules/=PrivateStaticReadonly/@EntryIndexedValue">&lt;Policy Inspect="True" Prefix="_" Suffix="" Style="aaBb" /&gt;</s:String>
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/UserRules/=042e7460_002D3ec6_002D4f1c_002D87c3_002Dfc91240d4c90/@EntryIndexedValue">&lt;Policy&gt;&lt;Descriptor Staticness="Static, Instance" AccessRightKinds="Public" Description="Test Methods"&gt;&lt;ElementKinds&gt;&lt;Kind Name="TEST_MEMBER" /&gt;&lt;/ElementKinds&gt;&lt;/Descriptor&gt;&lt;Policy Inspect="True" Prefix="" Suffix="" Style="Aa_bb" /&gt;&lt;/Policy&gt;</s:String>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,14 @@ public class SqlServerLoggingDefinitions : RelationalLoggingDefinitions
/// </summary>
public EventDefinitionBase LogReflexiveConstraintIgnored;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// 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 EventDefinitionBase LogSavepointsDisabledBecauseOfMARS;

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down
12 changes: 12 additions & 0 deletions src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ private enum Id
ByteIdentityColumnWarning,
ConflictingValueGenerationStrategiesWarning,
DecimalTypeKeyWarning,
SavepointsDisabledBecauseOfMARS,

// Scaffolding events
ColumnFound = CoreEventId.ProviderDesignBaseId,
Expand Down Expand Up @@ -211,5 +212,16 @@ private static EventId MakeScaffoldingId(Id id)
/// This event is in the <see cref="DbLoggerCategory.Scaffolding" /> category.
/// </summary>
public static readonly EventId ReflexiveConstraintIgnored = MakeScaffoldingId(Id.ReflexiveConstraintIgnored);

/// <summary>
/// <para>
/// Savepoints have been disabled when saving changes with an external transaction, because Multiple Active Result Sets is
/// enabled.
/// </para>
/// <para>
/// This event is in the <see cref="DbLoggerCategory.Database.Transaction" /> category.
/// </para>
/// </summary>
public static readonly EventId SavepointsDisabledBecauseOfMARS = MakeValidationId(Id.SavepointsDisabledBecauseOfMARS);
}
}
24 changes: 24 additions & 0 deletions src/EFCore.SqlServer/Internal/SqlServerLoggerExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -510,5 +510,29 @@ public static void ReflexiveConstraintIgnored(

// No DiagnosticsSource events because these are purely design-time messages
}

/// <summary>
/// Logs for the <see cref="RelationalEventId.MultipleCollectionIncludeWarning" /> event.
/// </summary>
/// <param name="diagnostics"> The diagnostics logger to use. </param>
public static void SavepointsDisabledBecauseOfMARS(
[NotNull] this IDiagnosticsLogger<DbLoggerCategory.Database.Transaction> diagnostics)
{
var definition = SqlServerResources.LogSavepointsDisabledBecauseOfMARS(diagnostics);

if (diagnostics.ShouldLog(definition))
{
definition.Log(diagnostics);
}

if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
{
var eventData = new EventData(
definition,
(d, p) => ((EventDefinition)d).GenerateMessage());

diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
}
}
}
}
24 changes: 24 additions & 0 deletions src/EFCore.SqlServer/Properties/SqlServerStrings.Designer.cs

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

4 changes: 4 additions & 0 deletions src/EFCore.SqlServer/Properties/SqlServerStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,10 @@
<value>Skipping foreign key '{foreignKeyName}' on table '{tableName}' since all of its columns reference themselves.</value>
<comment>Debug SqlServerEventId.ReflexiveConstraintIgnored string string</comment>
</data>
<data name="LogSavepointsDisabledBecauseOfMARS" xml:space="preserve">
<value>Savepoints are disabled because Multiple Active Result Sets is enabled. When saving changes, the transaction will not be rolled back if error occurs, and may be left in an unknown state. See https://docs.microsoft.com/en-us/ef/core/saving/transactions#savepoints for more information. To identify the code which triggers this warning, call 'ConfigureWarnings(w =&gt; w.Throw(RelationalEventId.SavepointsDisabledBecauseOfMARS))'.</value>
<comment>Warning SqlServerEventId.SavepointsDisabledBecauseOfMARS</comment>
</data>
<data name="MultipleIdentityColumns" xml:space="preserve">
<value>The properties {properties} are configured to use 'Identity' value generator and are mapped to the same table '{table}'. Only one column per table can be configured as 'Identity'. Call 'ValueGeneratedNever' for properties that should not use 'Identity'.</value>
</data>
Expand Down
19 changes: 19 additions & 0 deletions src/EFCore.SqlServer/Storage/Internal/SqlServerTransaction.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Threading.Tasks;
using JetBrains.Annotations;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.SqlServer.Internal;
using Microsoft.EntityFrameworkCore.Storage;

namespace Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal
Expand Down Expand Up @@ -43,6 +44,24 @@ protected override string GetCreateSavepointSql(string name)
protected override string GetRollbackToSavepointSql(string name)
=> "ROLLBACK TRANSACTION " + name;

/// <inheritdoc />
public override bool SupportsSavepoints
{
get
{
if (Connection is ISqlServerConnection sqlServerConnection && sqlServerConnection.IsMultipleActiveResultSetsEnabled)
{
Logger.SavepointsDisabledBecauseOfMARS();

return false;
}

return true;
}
}

// SQL Server doesn't support releasing savepoints. Override to do nothing.

/// <inheritdoc />
public override void ReleaseSavepoint(string name) { }

Expand Down
38 changes: 38 additions & 0 deletions test/EFCore.SqlServer.FunctionalTests/TransactionSqlServerTest.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Threading.Tasks;
using Microsoft.Data.SqlClient;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Xunit;

// ReSharper disable MethodHasAsyncOverload

namespace Microsoft.EntityFrameworkCore
{
Expand All @@ -14,6 +20,37 @@ public TransactionSqlServerTest(TransactionSqlServerFixture fixture)
{
}

[ConditionalTheory]
[InlineData(true)]
[InlineData(false)]
public virtual async Task Savepoints_are_disabled_with_MARS(bool async)
{
await using var context = CreateContext();

context.Database.SetDbConnection(
new SqlConnection(TestStore.ConnectionString + ";MultipleActiveResultSets=True"));

var transaction = await context.Database.BeginTransactionAsync();

var orderId = 300;
foreach (var _ in context.Set<TransactionCustomer>())
{
context.Add(new TransactionOrder { Id = orderId++, Name = "Order " + orderId });
if (async)
{
await context.SaveChangesAsync();
}
else
{
context.SaveChanges();
}
}

await transaction.CommitAsync();

Assert.Contains(Fixture.ListLoggerFactory.Log, t => t.Id == SqlServerEventId.SavepointsDisabledBecauseOfMARS);
}

protected override bool SnapshotSupported
=> true;

Expand Down Expand Up @@ -60,6 +97,7 @@ public override DbContextOptionsBuilder AddOptions(DbContextOptionsBuilder build
new SqlServerDbContextOptionsBuilder(
base.AddOptions(builder))
.ExecutionStrategy(c => new SqlServerExecutionStrategy(c));
builder.ConfigureWarnings(b => b.Log(SqlServerEventId.SavepointsDisabledBecauseOfMARS));
return builder;
}
}
Expand Down

0 comments on commit 43f34bf

Please sign in to comment.