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

Some design problems in current TransactionScope implemention #513

Closed
skykiker opened this issue Mar 2, 2015 · 3 comments
Closed

Some design problems in current TransactionScope implemention #513

skykiker opened this issue Mar 2, 2015 · 3 comments

Comments

@skykiker
Copy link
Contributor

skykiker commented Mar 2, 2015

Hi,all

In my view there's some design problems in current TransactionScope implemention.
I wish these problems will not appear in the future Npgsql V3.1 .

1)it's not needed to implement the IPromotableSinglePhaseNotification interface

Promotable Single Phase Enlistment(PSPE) is used to optimize performance for an "remote" durable resource.

https://msdn.microsoft.com/en-us/library/ms229980.aspx

To optimize performance, the System.Transactions infrastructure provides the Promotable Single Phase Enlistment (PSPE) that allows a single remote durable resource, located in a different application domain, process or machine, to participate in a System.Transactions transaction without causing it to be escalated to an MSDTC transaction. This resource manager (RM) can host and "own" a transaction that can later be escalated to a distributed transaction (or MSDTC transaction) if necessary. This reduces the chance of using the MSDTC.

But Npgsql is not a "remote" durable resource,and not need to to implement the PSPE.
In other words,the class NpgsqlPromotableSinglePhaseNotification is not useful(except making code more complex).

2)it's not necessity to implement a "Durable Resource Manager"

Aaccording to MSDN "The durability (or conversely the volatility) of a resource manager refers to whether the resource manager supports failure recovery".

https://msdn.microsoft.com/en-us/library/ms172153.aspx

Enlisting Resources in a Transaction

In order for a resource to participate in a transaction, it must enlist in the transaction. The Transaction class defines a set of methods whose names begin with Enlist that provide this functionality. The different Enlist methods correspond to the different types of enlistment that a resource manager may have. Specifically, you use the EnlistVolatile methods for volatile resources, and the EnlistDurable method for durable resources. The durability (or conversely the volatility) of a resource manager refers to whether the resource manager supports failure recovery. If a resource manager supports failure recovery, it persists data to durable storage during Phase1 (prepare) such that if the resource manager goes down, it can re-enlist in the transaction upon recovery and perform the proper actions based on the notifications received from the TM. In general, volatile resource managers manage volatile resources such as an in-memory data structure (for example, an in-memory transacted-hashtable), and durable resource managers manage resources that have a more persistent backing store (for example, a database whose backing store is disk).

Npsql currently does not support failure recovery,and so is not a durable resource indead.
So Npsql should use the EnlistVolatile method to enlist in the transaction, but now Npgsql use the EnlistDurable method.
Using of the EnlistDurable method means more tend to use MSDTC other than System.Transactions,and performance is poor.

https://msdn.microsoft.com/en-us/library/ms229980.aspx

Promotable Single Phase Enlistment

The System.Transactions infrastructure administrates a transaction inside a single application domain that involves at most a single durable resource or multiple volatile resources. Since the System.Transactions infrastructure uses only intra-application domain calls, it yields the best throughput and performance.

And you will find some .NET Data Providers(Firebird,Mysql?) which do not support "failure recovery" just implement TransactionScope as an "Volatile Resource"(call EnlistVolatile method to enlist in the transaction).I think that maybe the correct strategy to implementing TransactionScope.

3)Consideration of mutithread is not sufficient
The member methods from the IEnlistmentNotification interface(or other interfaces for TransactionScope) will be called by a backgroud thread but not the main thread.But the current TransactionScope implementation does not careful to deal with that.
Such as should use "cancle" but not "rollback" to deal with timeout(a bug report by #495) ,
,should not throw execption in these interface's methods directly(the backgroud thread can not deal with the excption correctly),and so on.

If your agree with me about these problems,i propose create a new TransactionScope implementation.
Because the current TransactionScope implementation is too complex for supporting of PSPE.

best regarts,
Chen Huajun

@roji roji added this to the 3.1 milestone Mar 2, 2015
@roji
Copy link
Member

roji commented Mar 2, 2015

@skykiker, thanks for all this. There's a good chance you're right - I've been wanting to take a look at our ambient/distributed transaction support for a while now. There's no way we can handle this for 3.0 (too much is going on), but it's definitely a major goal for 3.1. I promise to take a serious look at all of the above.

@roji
Copy link
Member

roji commented Jun 5, 2015

@skykiker, note PR #634 submitted on some of these issues.

@roji
Copy link
Member

roji commented Dec 11, 2016

@skykiker, as you may have seen in #122, I've been rewriting the support for System.Transactions these past few days. I think I've taken all of your comments into account - the current implementation is volatile (at least until we implement recovery in #1378), it has a mechanism for dealing with timeout-triggered rollbacks, and it's enlistment (hopefully) never propagates exceptions into the calling thread. Am going to submit a PR soon for review, I'd really appreciate if you could take a look and comment.

@roji roji closed this as completed Dec 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants