-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Need easier options to suspend / bypass the configured DbContext's IExecutionStrategy #24922
Comments
As you wrote, the code fragment you showed above (with the two explicit Database.BeginTransaction) isn't really safe - the first commit could succeed and the second could fail. If that's acceptable, then maybe it's also OK to simply do the two operations one after another: await using (var ctx1 = new MyContext1())
{
ctx1.Thing1s.Add(thing1);
await ctx1.SaveChangesAsync();
}
await using (var ctx2 = new MyContext1())
{
var thing2 = new Thing2
{
thing1ID = thing1.ID
};
ctx2.Thing2s.Add(thing2);
await ctx2.SaveChangesAsync();
} This has the advantage of retrying for the first operation. It's true that this is slightly more risky than your code sample above (with the two manual BeginTransctions) - since the second INSERT happens after the first COMMIT, as opposed to before in your code sample. However, that seems pretty negligible/theoretical to me, assuming you're willing to accept the risk that's already there anyway. Another option, is to specify whether you want resiliency when instantiating your DbContext, something like the following: public class MyContext1 : DbContext
{
private bool _enableRetryOnFailure;
public MyContext1()
=> _enableRetryOnFailure = true;
public MyContext1(bool enableRetryOnFailure)
=> _enableRetryOnFailure = enableRetryOnFailure;
protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
=> optionsBuilder
.UseSqlServer("...",
o =>
{
if (_enableRetryOnFailure)
o.EnableRetryOnFailure();
});
} This would allow you to selectively disable resiliency when you know you don't want it. Otherwise, yeah, you can simply extend SqlServerRetryingExecutionStrategy and override OnFirstExecution to not check for a transaction - that should be very trivial to do.
Note that EF Core has nothing to do with this - distributed transactions aren't supported by .NET Core, not EF Core. |
Hi @roji, not OP but also interested in this. Regarding selectively enabling the retry policy at in the config, what’s a good approach for that when using DI? i.e where typically you use AddDbContext once and can’t really control what happens for each instance you request via injection and it’s too late to configure it further once you get the instance. Would one approach be to create another DbContext class that inherits your base one, register that separately in DI with the different options and then inject the one you need as appropriate? Is there an alternative approach? Having a simple toggle on the context would be really nice but looking for the simplest alternatives that work now. Thanks! |
@roji thanks for your reply. My actual needs are of course a lot more complicated than this example. It wouldn't work to run the two operations separately, I need to be able to roll them both back if there's an error - typically in the First the good news. I found I was able to do the following in .NET 5 without two explicit
I'll admit I'm still slightly confused sometimes exactly what a distributed transaction is. I thought that communicating with two azure databases would qualify as one but it appears it does not. I'll need to read this https://docs.microsoft.com/en-us/azure/azure-sql/database/elastic-transactions-overview :-) I don't want to lose sight of my original suggestions that could be made either to the framework or documentation. It needs to be easier to opt-out of a configured strategy when a transaction is needed. I reported this as an issue on the repo and not on Stackoverflow for a reason. My goal is to make this easier and less confusing for others in the future!
I'm still thinking over what might be the impact of this. May be an option for me, but clearly not a solution for everyone.
@stevendarby I wondered about doing this when I first had this problem. I couldn't see an easy way to do this - I think you'd have to come up with your whole own way of creating a context. For the problem at hand it would be massively overcomplicated. |
@simeyla We can implement suggestion 2, as it's in-line with the current design. |
@simeyla a proper distribution transaction (not supported in .NET Core) ensures that both commits either succeed or fail on both databases, never committing one one database but not on the other. Without it, your first commit may succeed on your first database, but the second may fail, in which case you're left with an inconsistent state across your two databases. You can try to protect against this by manually undoing the effects of the first transaction, but... that could fail... This is basically what distributed transactions were created to solve. |
Do not throw an exception on the first execution of an execution strategy if a transaction is pending when MaxRetryCount is zero. Addresses dotnet#24922.
I am currently using the following execution strategy, the 'set it and forget it' approach. It usually works great!
I need to be able to suspend / replace the configured IExecutionStrategy for certain cases where I need to use an explicit transaction.
A solution therefore is needed to avoid the following exception:
The exception is described in Connection Resiliancy as being a problem if you're using an explicit
BeginTransaction
. The solution provided is to use this line of codedb.Database.CreateExecutionStrategy().Execute(() => ... )
instead of an explicit or ambient transaction.This solution can't be used if you need two active transactions for operations in two different databases. Here's a simplification of what I am logically trying to achieve:
Since we don't have distributed transactions yet in EF Core, I wanted to create two transaction scopes and then commit them together. The following code will work, but only if you do not have a retrying execution strategy defined.
For my needs this is an acceptable low-risk alternative to fully distributed transactions. I lose the auto-retry but I don't see any other alternative at this time.
The recommended solution (
db.Database.CreateExecutionStrategy().Execute(() => ... )
) unfortunately does not help in this situation since both transaction scopes need to remain active until they are committed.I've looked through some of the source to try to find solutions. Here are my attempts and the limitations I came across:
Attempt 1: Use
NonRetryingExecutionStrategy
I found NonRetryingExecutionStrategy which does exactly what I want, however there is no constructor taking a DbContext. The constructor takes
ExecutionStrategyDependencies
which is basically impossible for me to construct in my own code.Suggestion 1: Add a new constructor to
NonRetryingExecutionStrategy
This would allow me to do this in the scope of a preexisting transaction.
Attempt 2: Use SqlServerRetryingExecutionStrategy with 0 retries
Next I attempted to do the following:
This should work since it's no longer retrying anything (and that's the whole reason the exception is thrown).
This unfortunately does not work since the
OnFirstExecution
method inExecutionStrategy
(which is the base class) will ALWAYS throw the exception if there is an active transaction.Suggestion 2: Do not throw the exception if retries are disabled (RetriesOnFailure == false)
In other words update
ExecutionStrategy.OnFirstExecution
to the following:Then
RetriesOnFailure
would need to consider theMaxRetryCount
Attempt 3: Create my own custom execution strategy
This succeeded! My implementation is minimal, but with the following override.
While this worked it quite frankly took me all day to first be reminded (and annoyed!) that distributed transactions aren't yet supported, and then fail to find an existing solution.
PS: I'm pretty sure this succeeded in my brief testing! If there's some reason why this wouldn't work I'd love to hear it :-)
Attempt 4: Make other ways to create an execution strategy on the fly
The most discoverable way would be to add another way to create an execution strategy on the
DatabaseFacade
.Perhaps
DbContext.Database.CreateNonRetryingExecutionStrategy()
?This could then be documented and it would be completely obvious what it was.
Conclusions
It was simply too hard to disable the configured execution strategy and there needs to be an easier way.
My attempted solutions are not mutually exclusive.
Even with the best solution I came up with, I still need to wrap every group of database operation in
ExecuteAsync
. If there were an alternative way to disable the configured strategy for the whole context that would be preferable.Disclaimer:
Clearly things are always more complicated so I may have completely missed something.
The text was updated successfully, but these errors were encountered: