Skip to content

Commit

Permalink
Reuse some IExecutionStrategy instances.
Browse files Browse the repository at this point in the history 
Don't pass null to ExecutionStrategy.ShouldRetryOn.
Use the currently executing strategy to determine whether buffering is needed.

Fixes #21350
Fixes #23019
Fixes #21463
  • Loading branch information
@AndriySvyryd
AndriySvyryd authored Sep 3, 2021
1 parent 16ef6fd commit d82352e
Show file tree
Hide file tree
Showing 28 changed files with 346 additions and 283 deletions.
242 changes: 114 additions & 128 deletions src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs
View file

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/EFCore.Cosmos/Storage/Internal/CosmosExecutionStrategy.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public CosmosExecutionStrategy(ExecutionStrategyDependencies dependencies, int m
/// 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>
protected override bool ShouldRetryOn(Exception? exception)
protected override bool ShouldRetryOn(Exception exception)
{
return exception switch
{
Expand Down Expand Up @@ -126,7 +126,7 @@ static bool IsTransient(HttpStatusCode statusCode)
?? baseDelay;
}

private static TimeSpan? GetDelayFromException(Exception? exception)
private static TimeSpan? GetDelayFromException(Exception exception)
{
switch (exception)
{
Expand Down
45 changes: 15 additions & 30 deletions src/EFCore.Relational/Extensions/RelationalDatabaseFacadeExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,9 +502,8 @@ public static void SetConnectionString(this DatabaseFacade databaseFacade, strin
/// </summary>
/// <param name="databaseFacade"> The <see cref="DatabaseFacade" /> for the context. </param>
public static void OpenConnection(this DatabaseFacade databaseFacade)
=> databaseFacade.CreateExecutionStrategy().Execute(
databaseFacade, database
=> GetFacadeDependencies(database).RelationalConnection.Open());
=> ((IDatabaseFacadeDependenciesAccessor)databaseFacade).Dependencies.ExecutionStrategy
.Execute(databaseFacade, database => GetFacadeDependencies(database).RelationalConnection.Open(), null);

/// <summary>
/// Opens the underlying <see cref="DbConnection" />.
Expand All @@ -516,9 +515,8 @@ public static void OpenConnection(this DatabaseFacade databaseFacade)
public static Task OpenConnectionAsync(
this DatabaseFacade databaseFacade,
CancellationToken cancellationToken = default)
=> databaseFacade.CreateExecutionStrategy().ExecuteAsync(
databaseFacade, (database, ct) =>
GetFacadeDependencies(database).RelationalConnection.OpenAsync(ct), cancellationToken);
=> ((IDatabaseFacadeDependenciesAccessor)databaseFacade).Dependencies.ExecutionStrategy
.ExecuteAsync(databaseFacade, (database, ct) => GetFacadeDependencies(database).RelationalConnection.OpenAsync(ct), null, cancellationToken);

/// <summary>
/// Closes the underlying <see cref="DbConnection" />.
Expand All @@ -542,15 +540,16 @@ public static Task CloseConnectionAsync(this DatabaseFacade databaseFacade)
/// <param name="isolationLevel"> The <see cref="IsolationLevel" /> to use. </param>
/// <returns> A <see cref="IDbContextTransaction" /> that represents the started transaction. </returns>
public static IDbContextTransaction BeginTransaction(this DatabaseFacade databaseFacade, IsolationLevel isolationLevel)
=> databaseFacade.CreateExecutionStrategy().Execute(
=> ((IDatabaseFacadeDependenciesAccessor)databaseFacade).Dependencies.ExecutionStrategy.Execute(
databaseFacade, database =>
{
var transactionManager = database.GetTransactionManager();
return transactionManager is IRelationalTransactionManager relationalTransactionManager
? relationalTransactionManager.BeginTransaction(isolationLevel)
: transactionManager.BeginTransaction();
});
},
null);

/// <summary>
/// Asynchronously starts a new transaction with a given <see cref="IsolationLevel" />.
Expand All @@ -567,15 +566,15 @@ public static Task<IDbContextTransaction> BeginTransactionAsync(
this DatabaseFacade databaseFacade,
IsolationLevel isolationLevel,
CancellationToken cancellationToken = default)
=> databaseFacade.CreateExecutionStrategy().ExecuteAsync(
=> ((IDatabaseFacadeDependenciesAccessor)databaseFacade).Dependencies.ExecutionStrategy.ExecuteAsync(
databaseFacade, (database, ct) =>
{
var transactionManager = database.GetTransactionManager();
return transactionManager is IRelationalTransactionManager relationalTransactionManager
? relationalTransactionManager.BeginTransactionAsync(isolationLevel, ct)
: transactionManager.BeginTransactionAsync(ct);
}, cancellationToken);
}, null, cancellationToken);

/// <summary>
/// Sets the <see cref="DbTransaction" /> to be used by database operations on the <see cref="DbContext" />.
Expand All @@ -599,16 +598,9 @@ public static Task<IDbContextTransaction> BeginTransactionAsync(
this DatabaseFacade databaseFacade,
DbTransaction? transaction,
Guid transactionId)
{
var transactionManager = GetTransactionManager(databaseFacade);

if (!(transactionManager is IRelationalTransactionManager relationalTransactionManager))
{
throw new InvalidOperationException(RelationalStrings.RelationalNotInUse);
}

return relationalTransactionManager.UseTransaction(transaction, transactionId);
}
=> GetTransactionManager(databaseFacade) is IRelationalTransactionManager relationalTransactionManager
? relationalTransactionManager.UseTransaction(transaction, transactionId)
: throw new InvalidOperationException(RelationalStrings.RelationalNotInUse);

/// <summary>
/// Sets the <see cref="DbTransaction" /> to be used by database operations on the <see cref="DbContext" />.
Expand Down Expand Up @@ -638,16 +630,9 @@ public static Task<IDbContextTransaction> BeginTransactionAsync(
DbTransaction? transaction,
Guid transactionId,
CancellationToken cancellationToken = default)
{
var transactionManager = GetTransactionManager(databaseFacade);

if (!(transactionManager is IRelationalTransactionManager relationalTransactionManager))
{
throw new InvalidOperationException(RelationalStrings.RelationalNotInUse);
}

return relationalTransactionManager.UseTransactionAsync(transaction, transactionId, cancellationToken);
}
=> GetTransactionManager(databaseFacade) is IRelationalTransactionManager relationalTransactionManager
? relationalTransactionManager.UseTransactionAsync(transaction, transactionId, cancellationToken)
: throw new InvalidOperationException(RelationalStrings.RelationalNotInUse);

/// <summary>
/// <para>
Expand Down
6 changes: 2 additions & 4 deletions src/EFCore.Relational/Query/Internal/FromSqlQueryingEnumerable.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -197,8 +197,7 @@ public bool MoveNext()
{
if (_dataReader == null)
{
_relationalQueryContext.ExecutionStrategyFactory.Create()
.Execute(this, (_, enumerator) => InitializeReader(enumerator), null);
_relationalQueryContext.ExecutionStrategy.Execute(this, (_, enumerator) => InitializeReader(enumerator), null);
}

var hasNext = _dataReader!.Read();
Expand Down Expand Up @@ -303,8 +302,7 @@ public async ValueTask<bool> MoveNextAsync()
{
if (_dataReader == null)
{
await _relationalQueryContext.ExecutionStrategyFactory.Create()
.ExecuteAsync(
await _relationalQueryContext.ExecutionStrategy.ExecuteAsync(
this,
(_, enumerator, cancellationToken) => InitializeReaderAsync(enumerator, cancellationToken),
null,
Expand Down
6 changes: 2 additions & 4 deletions src/EFCore.Relational/Query/Internal/SingleQueryingEnumerable.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,7 @@ public bool MoveNext()
{
if (_dataReader == null)
{
_relationalQueryContext.ExecutionStrategyFactory.Create()
.Execute(this, static (_, enumerator) => InitializeReader(enumerator), null);
_relationalQueryContext.ExecutionStrategy.Execute(this, static (_, enumerator) => InitializeReader(enumerator), null);
}

var hasNext = _resultCoordinator!.HasNext ?? _dataReader!.Read();
Expand Down Expand Up @@ -303,8 +302,7 @@ public async ValueTask<bool> MoveNextAsync()
{
if (_dataReader == null)
{
await _relationalQueryContext.ExecutionStrategyFactory.Create()
.ExecuteAsync(
await _relationalQueryContext.ExecutionStrategy.ExecuteAsync(
this,
static (_, enumerator, cancellationToken) => InitializeReaderAsync(enumerator, cancellationToken),
null,
Expand Down
21 changes: 5 additions & 16 deletions src/EFCore.Relational/Query/Internal/SplitQueryingEnumerable.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ private sealed class Enumerator : IEnumerator<T>
private RelationalDataReader? _dataReader;
private DbDataReader? _dbDataReader;
private SplitQueryResultCoordinator? _resultCoordinator;
private IExecutionStrategy? _executionStrategy;

public Enumerator(SplitQueryingEnumerable<T> queryingEnumerable)
{
Expand Down Expand Up @@ -175,12 +174,7 @@ public bool MoveNext()
{
if (_dataReader == null)
{
if (_executionStrategy == null)
{
_executionStrategy = _relationalQueryContext.ExecutionStrategyFactory.Create();
}

_executionStrategy.Execute(this, static (_, enumerator) => InitializeReader(enumerator), null);
_relationalQueryContext.ExecutionStrategy.Execute(this, static (_, enumerator) => InitializeReader(enumerator), null);
}

var hasNext = _dataReader!.Read();
Expand All @@ -192,7 +186,7 @@ public bool MoveNext()
_relationalQueryContext, _dbDataReader!, _resultCoordinator.ResultContext, _resultCoordinator);
if (_relatedDataLoaders != null)
{
_relatedDataLoaders.Invoke(_relationalQueryContext, _executionStrategy!, _resultCoordinator);
_relatedDataLoaders.Invoke(_relationalQueryContext, _relationalQueryContext.ExecutionStrategy, _resultCoordinator);
Current = _shaper(
_relationalQueryContext, _dbDataReader!, _resultCoordinator.ResultContext, _resultCoordinator);
}
Expand Down Expand Up @@ -285,7 +279,6 @@ private sealed class AsyncEnumerator : IAsyncEnumerator<T>
private RelationalDataReader? _dataReader;
private DbDataReader? _dbDataReader;
private SplitQueryResultCoordinator? _resultCoordinator;
private IExecutionStrategy? _executionStrategy;

public AsyncEnumerator(SplitQueryingEnumerable<T> queryingEnumerable)
{
Expand Down Expand Up @@ -317,12 +310,7 @@ public async ValueTask<bool> MoveNextAsync()
{
if (_dataReader == null)
{
if (_executionStrategy == null)
{
_executionStrategy = _relationalQueryContext.ExecutionStrategyFactory.Create();
}

await _executionStrategy.ExecuteAsync(
await _relationalQueryContext.ExecutionStrategy.ExecuteAsync(
this,
static (_, enumerator, cancellationToken) => InitializeReaderAsync(enumerator, cancellationToken),
null,
Expand All @@ -338,7 +326,7 @@ await _executionStrategy.ExecuteAsync(
Current = _shaper(_relationalQueryContext, _dbDataReader!, _resultCoordinator.ResultContext, _resultCoordinator);
if (_relatedDataLoaders != null)
{
await _relatedDataLoaders(_relationalQueryContext, _executionStrategy!, _resultCoordinator)
await _relatedDataLoaders(_relationalQueryContext, _relationalQueryContext.ExecutionStrategy, _resultCoordinator)
.ConfigureAwait(false);
Current =
_shaper(_relationalQueryContext, _dbDataReader!, _resultCoordinator.ResultContext, _resultCoordinator);
Expand Down Expand Up @@ -408,6 +396,7 @@ public async ValueTask DisposeAsync()
_resultCoordinator.DataReaders.Clear();
_resultCoordinator = null;
}

_dataReader = null;
_dbDataReader = null;
}
Expand Down
3 changes: 2 additions & 1 deletion src/EFCore.Relational/Query/RelationalCompiledQueryCacheKeyGenerator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Linq.Expressions;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Storage;
using Microsoft.EntityFrameworkCore.Utilities;
using Microsoft.Extensions.DependencyInjection;

Expand Down Expand Up @@ -71,7 +72,7 @@ public override object GenerateCacheKey(Expression query, bool async)
base.GenerateCacheKeyCore(query, async),
relationalOptions.UseRelationalNulls,
relationalOptions.QuerySplittingBehavior,
shouldBuffer: Dependencies.IsRetryingExecutionStrategy);
shouldBuffer: ExecutionStrategy.Current?.RetriesOnFailure ?? Dependencies.IsRetryingExecutionStrategy);
}

/// <summary>
Expand Down
10 changes: 10 additions & 0 deletions src/EFCore.Relational/Storage/Internal/RelationalDatabaseFacadeDependencies.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ public record RelationalDatabaseFacadeDependencies : IRelationalDatabaseFacadeDe
public RelationalDatabaseFacadeDependencies(
IDbContextTransactionManager transactionManager,
IDatabaseCreator databaseCreator,
IExecutionStrategy executionStrategy,
IExecutionStrategyFactory executionStrategyFactory,
IEnumerable<IDatabaseProvider> databaseProviders,
IRelationalCommandDiagnosticsLogger commandLogger,
Expand All @@ -34,6 +35,7 @@ public RelationalDatabaseFacadeDependencies(
{
TransactionManager = transactionManager;
DatabaseCreator = databaseCreator;
ExecutionStrategy = executionStrategy;
ExecutionStrategyFactory = executionStrategyFactory;
DatabaseProviders = databaseProviders;
CommandLogger = commandLogger;
Expand All @@ -59,6 +61,14 @@ public RelationalDatabaseFacadeDependencies(
/// </summary>
public virtual IDatabaseCreator DatabaseCreator { get; init; }

/// <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 virtual IExecutionStrategy ExecutionStrategy { get; init; }

/// <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
24 changes: 12 additions & 12 deletions src/EFCore.Relational/Storage/RelationalDatabaseCreatorDependencies.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,20 +69,11 @@ public RelationalDatabaseCreatorDependencies(
IMigrationsSqlGenerator migrationsSqlGenerator,
IMigrationCommandExecutor migrationCommandExecutor,
ISqlGenerationHelper sqlGenerationHelper,
IExecutionStrategy executionStrategy,
IExecutionStrategyFactory executionStrategyFactory,
ICurrentDbContext currentContext,
IRelationalCommandDiagnosticsLogger commandLogger)
{
Check.NotNull(model, nameof(model));
Check.NotNull(connection, nameof(connection));
Check.NotNull(modelDiffer, nameof(modelDiffer));
Check.NotNull(migrationsSqlGenerator, nameof(migrationsSqlGenerator));
Check.NotNull(migrationCommandExecutor, nameof(migrationCommandExecutor));
Check.NotNull(sqlGenerationHelper, nameof(sqlGenerationHelper));
Check.NotNull(executionStrategyFactory, nameof(executionStrategyFactory));
Check.NotNull(currentContext, nameof(currentContext));
Check.NotNull(commandLogger, nameof(commandLogger));

#pragma warning disable CS0618 // Type or member is obsolete
Model = model;
#pragma warning restore CS0618 // Type or member is obsolete
Expand All @@ -91,7 +82,10 @@ public RelationalDatabaseCreatorDependencies(
MigrationsSqlGenerator = migrationsSqlGenerator;
MigrationCommandExecutor = migrationCommandExecutor;
SqlGenerationHelper = sqlGenerationHelper;
ExecutionStrategy = executionStrategy;
#pragma warning disable CS0618 // Type or member is obsolete
ExecutionStrategyFactory = executionStrategyFactory;
#pragma warning restore CS0618 // Type or member is obsolete
CurrentContext = currentContext;
CommandLogger = commandLogger;
}
Expand Down Expand Up @@ -128,12 +122,18 @@ public RelationalDatabaseCreatorDependencies(
public ISqlGenerationHelper SqlGenerationHelper { get; init; }

/// <summary>
/// Gets the <see cref="IExecutionStrategyFactory" /> to be used.
/// Gets the execution strategy.
/// </summary>
public IExecutionStrategy ExecutionStrategy { get; }

/// <summary>
/// Gets the execution strategy factory to be used.
/// </summary>
[Obsolete("Use ExecutionStrategy instead")]
public IExecutionStrategyFactory ExecutionStrategyFactory { get; init; }

/// <summary>
/// The command logger.
/// Gets the command logger.
/// </summary>
public IRelationalCommandDiagnosticsLogger CommandLogger { get; init; }

Expand Down
4 changes: 2 additions & 2 deletions src/EFCore.SqlServer/SqlServerRetryingExecutionStrategy.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ public SqlServerRetryingExecutionStrategy(
/// <returns>
/// <see langword="true" /> if the specified exception is considered as transient, otherwise <see langword="false" />.
/// </returns>
protected override bool ShouldRetryOn(Exception? exception)
protected override bool ShouldRetryOn(Exception exception)
{
if (_additionalErrorNumbers != null
&& exception is SqlException sqlException)
Expand Down Expand Up @@ -166,7 +166,7 @@ protected override bool ShouldRetryOn(Exception? exception)
: baseDelay;
}

private static bool IsMemoryOptimizedError(Exception? exception)
private static bool IsMemoryOptimizedError(Exception exception)
{
if (exception is SqlException sqlException)
{
Expand Down
Loading

0 comments on commit d82352e

Please sign in to comment.