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 17, 2020
1 parent 641d553 commit 2d0a835
Show file tree
Hide file tree
Showing 11 changed files with 156 additions and 6 deletions.
3 changes: 3 additions & 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 Expand Up @@ -219,6 +220,8 @@ Licensed under the Apache License, Version 2.0. See License.txt in the project r
<s:Boolean x:Key="/Default/UserDictionary/Words/=remapper/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=requiredness/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=retriable/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=savepoint/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=savepoints/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=shaper/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=spatialite/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=sproc/@EntryIndexedValue">True</s:Boolean>
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
19 changes: 19 additions & 0 deletions src/EFCore.SqlServer/Diagnostics/SqlServerEventId.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ private enum Id
ConflictingValueGenerationStrategiesWarning,
DecimalTypeKeyWarning,

// Transaction events
SavepointsDisabledBecauseOfMARS,

// Scaffolding events
ColumnFound = CoreEventId.ProviderDesignBaseId,
ColumnNotNamedWarning,
Expand Down Expand Up @@ -121,6 +124,22 @@ private static EventId MakeValidationId(Id id)
public static readonly EventId ConflictingValueGenerationStrategiesWarning =
MakeValidationId(Id.ConflictingValueGenerationStrategiesWarning);

private static readonly string _transactionPrefix = DbLoggerCategory.Database.Transaction.Name + ".";

private static EventId MakeTransactionId(Id id)
=> new EventId((int)id, _transactionPrefix + id);

/// <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 = MakeTransactionId(Id.SavepointsDisabledBecauseOfMARS);

private static readonly string _scaffoldingPrefix = DbLoggerCategory.Scaffolding.Name + ".";

private static EventId MakeScaffoldingId(Id id)
Expand Down
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 (MARS) is enabled. If 'SaveChanges' fails, then the transaction cannot be automatically rolled back to a known clean state. Instead, the transaction should be rolled back by the application before retrying 'SaveChanges'. See https://go.microsoft.com/fwlink/?linkid=2149338 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
2 changes: 1 addition & 1 deletion src/EFCore/Diagnostics/WarningsConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ public virtual long GetServiceProviderHashCode()

if (_explicitBehaviors != null)
{
hashCode = _explicitBehaviors.Aggregate(
hashCode = _explicitBehaviors.OrderBy(b => b.Key).Aggregate(
hashCode,
(t, e) => (t * 397) ^ (((long)e.Value.GetHashCode() * 3163) ^ (long)e.Key.GetHashCode()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Threading.Tasks;
using Identity30.Data;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Migrations;
Expand Down Expand Up @@ -1410,9 +1411,8 @@ IF EXISTS(select * from sys.databases where name='TransactionSuppressed')

public override MigrationsContext CreateContext()
{
var options = AddOptions(
new DbContextOptionsBuilder()
.UseSqlServer(TestStore.ConnectionString, b => b.ApplyConfiguration()))
var options = AddOptions(TestStore.AddProviderOptions(new DbContextOptionsBuilder()))
.UseSqlServer(TestStore.ConnectionString, b => b.ApplyConfiguration())
.UseInternalServiceProvider(ServiceProvider)
.Options;
return new MigrationsContext(options);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Data.SqlClient;
using Microsoft.EntityFrameworkCore.Diagnostics;

#pragma warning disable IDE0022 // Use block body for methods
// ReSharper disable SuggestBaseTypeForParameter
Expand Down Expand Up @@ -98,7 +99,9 @@ protected override void Initialize(Func<DbContext> createContext, Action<DbConte
}

public override DbContextOptionsBuilder AddProviderOptions(DbContextOptionsBuilder builder)
=> builder.UseSqlServer(Connection, b => b.ApplyConfiguration());
=> builder
.UseSqlServer(Connection, b => b.ApplyConfiguration())
.ConfigureWarnings(b => b.Ignore(SqlServerEventId.SavepointsDisabledBecauseOfMARS));

private bool CreateDatabase(Action<DbContext> clean)
{
Expand Down
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,19 +20,58 @@ public TransactionSqlServerTest(TransactionSqlServerFixture fixture)
{
}

[ConditionalTheory]
[InlineData(true)]
[InlineData(false)]
public virtual async Task Savepoints_are_disabled_with_MARS(bool async)
{
await using var context = CreateContextWithConnectionString(
SqlServerTestStore.CreateConnectionString(TestStore.Name, multipleActiveResultSets: true));

await using 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);
}

// Test relies on savepoints, which are disabled when MARS is enabled
public override Task SaveChanges_uses_explicit_transaction_with_failure_behavior(bool async, bool autoTransaction)
=> new SqlConnectionStringBuilder(TestStore.ConnectionString).MultipleActiveResultSets
? Task.CompletedTask
: base.SaveChanges_uses_explicit_transaction_with_failure_behavior(async, autoTransaction);

protected override bool SnapshotSupported
=> true;

protected override bool AmbientTransactionsSupported
=> true;

protected override DbContext CreateContextWithConnectionString()
=> CreateContextWithConnectionString(null);

protected DbContext CreateContextWithConnectionString(string connectionString)
{
var options = Fixture.AddOptions(
new DbContextOptionsBuilder()
.UseSqlServer(
TestStore.ConnectionString,
connectionString ?? TestStore.ConnectionString,
b => b.ApplyConfiguration().ExecutionStrategy(c => new SqlServerExecutionStrategy(c))))
.ConfigureWarnings(b => b.Log(SqlServerEventId.SavepointsDisabledBecauseOfMARS))
.UseInternalServiceProvider(Fixture.ServiceProvider);

return new DbContext(options.Options);
Expand Down Expand Up @@ -60,6 +105,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 2d0a835

Please sign in to comment.