Additional ExecutionStrategy extension point: OnOperationRetrying(Async)
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 35/100
Research direction
Start in src/EFCore/Storage/ExecutionStrategy.cs, reading OnRetry and the ExecuteImplementation and ExecuteImplementationAsync entry points. Trace the existing retry and delay flow, including how ShouldRetryOn handles exceptions. Done means the proposed synchronous and asynchronous retry extension points fit both flows without disrupting existing behavior, with coverage for their timing and exception handling.
Written by the indexing model from the issue text.
Description
In ExecutionStrategy, there is a convenient template method OnRetry that will be called after an operation failure. However, there are a few limitations to this extension point:
- It is called before the delay, so you cannot do something after the delay (i.e. allowing the database time to recover)
- Any exceptions thrown by
OnRetryare not caught and handled viaShouldRetryOnlike operation exceptions are - It does not have an
asyncversion
I'd like to propose an additional extension point, which for the sake of this issue I'll call OnOperationRetrying (feel free to rename). My primary use case is to be able to execute some SQL queries before the operation in the case of a retry, but I could imagine other use cases such as logging or consistency validation.
It would be a similar template-method signature to OnRetry but with an additional async version:
protected virtual void OnOperationRetrying()
{
}
protected virtual Task OnOperationRetryingAsync(CancellationToken cancellationToken)
{
return Task.CompletedTask;
}
This would be called from ExecuteImplementation and ExecuteImplementationAsync inside the try block, immediately before the call to operation. For example:
private TResult ExecuteImplementation<TState, TResult>(
Func<DbContext, TState, TResult> operation,
Func<DbContext, TState, ExecutionResult<TResult>> verifySucceeded,
TState state)
{
bool isRetrying = false; // This is new
while (true)
{
TimeSpan? delay;
try
{
Suspended = true;
if (isRetrying) {
OnOperationRetrying(); // new extension point
}
var result = operation(Dependencies.CurrentDbContext.Context, state);
Suspended = false;
return result;
}
catch (Exception ex)
{
... // existing catch code unmodified
OnRetry(); // this will still exist
isRetrying = true; // this is new
}
using (var waitEvent = new ManualResetEventSlim(false))
{
waitEvent.WaitHandle.WaitOne(delay.Value);
}
}
}
And of course there would be similar logic in ExecuteImplementationAsync just using await OnOperationRetryingAsync() instead.
This addresses all 3 of the limitations above:
- It is called right before the next operation, so it is after the delay
- It is inside the
tryblock, so any exceptions that occur thatShouldRetryOnsays should be retried will be retried - There is an
asyncversion
In addition, this should not impose a significant performance penalty in the common (non-retry) case, as only an if check of a boolean variable is added on the first execution of the loop. And because of the delay logic, the additional cost of a non-overridden virtual method call on a retry is negligible.
The only caveat that would be needed for the documentation would be that if OnOperationRetrying throws, that would count as a retry and operation would not be called. But by its nature, that is the expected behavior. (If OnOperationRetrying fails, the expectation is that operation would likely fail anyways.)
I'd be happy to submit a PR for this, but I wanted to get feedback on it first to see if it's acceptable, or if someone has an alternative of a better way of doing this (without having to copy/paste reimplement ExecutionStrategy).
- Dominant language
- C#
- Stars
- 14.8k
- Forks
- 3.4k
- Avg merge
- 2d 5h
- Merged PRs (30d)
- 134
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from dotnet/efcore
-
Difficulty 4/5 3-5 days Newbie friendliness 55/100
-
customer-reported
Difficulty 5/5 Over a week Newbie friendliness 38/100
-
area-cosmos area-vector-search
Difficulty 5/5 Over a week Newbie friendliness 25/100
-
area-cosmos
Difficulty 5/5 Over a week Newbie friendliness 25/100
-
area-tools needs-design
Difficulty 4/5 3-5 days Newbie friendliness 25/100
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 86/100
-
:watch: Not Triaged 11.0 fundamentals/subsvc
Difficulty 2/5 1-3 hours Newbie friendliness 92/100
dotnet/AspNetCore.Docs#37699 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
SubtitleEdit/subtitleedit#15108 · 1 comment ·
-
area/docs-content Bug pulumi/docs
Difficulty 1/5 1-3 hours Newbie friendliness 94/100
-
agentic-workflows untriaged
Difficulty 2/5 1-3 hours Newbie friendliness 76/100