Support opting out of wrapping database operations in a transaction during SaveChanges()

[ajcvickers]

Issue #6339

context.Database.AutoTransactionsEnabled = false;

Doesn't affect anything done with UseTransaction or BeginTransaction--only prevents EF from creating a transaction automatically.

Also some perf improvements for running the transaction tests--previously when running on my machine they would take about 70 seconds. Now they take about 3. But note that most of this is masked when running many tests in parallel.

[anpete]

Having this on Database makes some sense because we have other transaction APIs that live there. However, we could also consider an overload of SaveChanges that takes this as an argument. This approach could be nice because it makes the effect and scope of the flag very clear - Or, are there other places apart from SaveChanges where this flag has an effect?

[ajcvickers]

I was following the pattern of AutoDetectChangesEnabled, but I don't have any strong opinion on the API. @divega What do you want the API to look like?

[divega]

I had the same question when I reviewed the code yesterday, but I didn't comment because I couldn't come up with anything that felt fundamentally better that what @ajcvickers already put in the PR. FWIW my thinking went more ore less like this:

• Maybe we should put something in the name of the property that indicates that is for SaveChanges() only. But wait, is that what we really want? What if we later added another API that also has to wrap operations in transactions? It seems that the chances that we would want such API to follow this flag are very high.
• AutoTransactionsEnabled is also shorter because it doesn't say anything about SaveChanges(), which is nice
• Historically adding parameters to SaveChanges() hasn't scaled very well: Once you add a parameter like this, why not add AutoDetectChangesEnabled as well, or anything we could think of that is pertinent to SaveChanges(). But then, we already have some things that we didn't put there, and maybe we need an options type that we can pass to SaveChanges()! But wait, doesn't the DbContext already act as a big options object for SaveChanges()?

[anpete]

@divega Just to add a data point that we discussed: SaveChanges already has an overload with a parameter (acceptAllChangesOnSuccess). Do we consider obsoleting this and hanging this flag somewhere else?

[divega]

Interesting. To me that means there is no clear cut best way to go about this.

I tend to think about acceptAllChangesOnSuccess also as a mode of operation of the DbContext instance even if currently it is only applicable to SaveChanges(). E.g. I think in normal usage i would set it to false and then make several calls to SaveChanges(), but maybe I am missing something.

Anyway I personally don't feel strongly about obsoleting this overload. I just think that adding even more overloads with multiple flags would make things messy.

Let's discuss it in person.

[julielerman]

this is an interesting option, but what is your guidance around it especially for dealing with "some stuff got persisted, some didn't". I like being able to get as much in as possible and setting the failures aside. We can identify the failed entity but how can we identify what did and didn't get in so we can try those again later? Would we do something like use a log to identify which commands were executed and work from there? Thanks

[anpete]

@julielerman Guidance is that this is an advanced option that should only be used with extreme care 😄 We added it to support some specific high-throughput scenarios where data integrity was not an essential concern.

[julielerman]

@anpete oh like "it depends". ha! yeah extreme care ..for sure. So anyone using it is on their own wrt how to deal with the results . I guess as @divega mentioned above, have very limited things happening in a single call.