eShop icon indicating copy to clipboard operation
eShop copied to clipboard

Retries in the TransactionBehavior and use of the EntityFramework Core DbContext

Open bytefish opened this issue 1 year ago • 4 comments

The unusual use of transactions in the TransactionBehavior is a work item mentioned in issue #23 and it should be reviewed. Sadly, there is no activity in the issue. What I am trying to make sense of in the TransactionBehavior is, that the DbContext has a Scoped Lifetime and it is passed through the entire application (which already is a highly questionable decision to me).

So the DbContext has all changes in the ChangeTracker, that have been made along the way:

  • https://github.com/dotnet/eShop/blob/0618455473bcd8783597e203ceb6354bf994db45/src/Ordering.API/Application/Behaviors/TransactionBehavior.cs

What's going to happen with the Retry in there? The DbContext will travel through all Services again, all Entities will be added again. I don't see the ChangeTracker being cleared, the DbContext being renewed, entities being detached or any kind of compensation. It looks like you are going to end up with a poisoned DbContext and a Retry is going to fail anyways.

@ajcvickers @bricelam I am sorry to add you directly, but I would love to get some idea from the EntityFramework Core team. I would love to add a Pull Request, but it's my impression there are more things to be done. More generally I feel the entire "Domain Driven Design" approach here is problematic. Every "DomainEvent Handler" can directly call into DbContext#SaveChangesAsync (through some IUnitOfWork indirection), and finding out who is the first to do so, is like trying to understand SQL Triggers (fun).

bytefish avatar Apr 12 '24 04:04 bytefish

@bytefish Unfortunately, the EF team does not have any time available to spend on this at the moment. /cc @SamMonoRT

ajcvickers avatar Apr 12 '24 10:04 ajcvickers

At the moment this is below EF/Data teams' cut line for 9.0 and we may not get to it anytime soon.

SamMonoRT avatar Apr 12 '24 18:04 SamMonoRT

@SamMonoRT @ajcvickers Understood. But could use please explain, if usage of the DbContext, as suggested in this TransactionBehavior, is best practice?

If I read the discussion in the EntityFramework Core Issue Tracker correctly, the DbContext will not reset:

  • https://github.com/dotnet/efcore/issues/30097

bytefish avatar Apr 12 '24 23:04 bytefish

if usage of the DbContext, as suggested in this TransactionBehavior, is best practice?

No, in general this repo should not be used for guidance on how to use EF Core. There are lot of things in this code that the team considers anti-patterns.

ajcvickers avatar Apr 13 '24 09:04 ajcvickers