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

Reuse some IExecutionStrategy instances. #25836

Merged
merged 3 commits into from
Sep 3, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
242 changes: 114 additions & 128 deletions src/EFCore.Cosmos/Storage/Internal/CosmosClientWrapper.cs

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/EFCore.Cosmos/Storage/Internal/CosmosExecutionStrategy.cs
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
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
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
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
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
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
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
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
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