yasser777 / nettiers

Automatically exported from code.google.com/p/nettiers
0 stars 0 forks source link

Delete by PK (Integer Identity Column) causes MSDTC promotion #300

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
We had been chasing an issue where NetTiers code was causing MSDTC 
escalation within a TranascationScope even though there were no 
distributed transactions occurring.  

Normally EntLib DAAB manages the Connection based on participation in a 
TransactionScope... And netTiers in turn benefits from that self-
management down in EntLib.

As it turns out, in the 2.3.0 templates - in ConnectionScope.cst - we 
found the culprit...  The method [ValidateOrCreateTransaction] without any 
arguments was in turn calling [ValidateOrCreateTransaction] with a 
defaulted input argument of "true" (the arugment is named 
[createTransaction]).  By changing the default from "true" to "false" in 
the method override - this corrected the behavior.

What led us to make this change was that most of the data operations call 
the method signature with the argument and pass in "false"...  The delete 
operation for deleting by the PK rather than passing in the Entity was an 
exception and it called the method signature without an argument resulting 
in "true" being passed and a new connection/transaction of NetTier's own 
being created - hence the enlistment/promotion to a distributed 
transaction...

Before:

        /// <summary>
        /// Validates an existing <c cref="TransactionManager" /> 
if one exists,
        /// otherwise creates a new <c 
cref="TransactionManager" /> to use.
        /// </summary>
        public static TransactionManager 
ValidateOrCreateTransaction()
        {
            return ValidateOrCreateTransaction(true);
        }

After:

        /// <summary>
        /// Validates an existing <c cref="TransactionManager" /> 
if one exists,
        /// otherwise creates a new <c 
cref="TransactionManager" /> to use.
        /// </summary>
        public static TransactionManager 
ValidateOrCreateTransaction()
        {
            return ValidateOrCreateTransaction(false);
        }

In the Service layer - for a given Entity - here's the line in the Delete 
operation when passing in the Entity itself...

                transactionManager = 
ConnectionScope.ValidateOrCreateTransaction(noTranByDefault);

But when passing in the PK value (a single integer ID column in this 
case)... You find the following lines...

                //since this is a read operation, don't 
create a tran by default, only use tran if provided to us for custom 
isolation level
                transactionManager = 
ConnectionScope.ValidateOrCreateTransaction();

The comment is misleading in itself (imo) in that (A) it's a delete and 
(B) if they didn't want a transaction you'd expect false explicitly passed 
here too...

Original issue reported on code.google.com by bh...@yahoo.com on 27 Apr 2010 at 8:50

GoogleCodeExporter commented 9 years ago
In the actual [ValidateOrCreateTransaction(bool createTransaction)] 
operation... 
There is likely a simple fix if the change we made is not the right thing to 
do...

Given this orignial code...

        public static TransactionManager ValidateOrCreateTransaction(bool 
createTransaction)
        {
            TransactionManager transactionManager = 
Current.TransactionManager;
            bool isBorrowedTransaction = (transactionManager != null && 
transactionManager.IsOpen);
            NetTiersProvider dataProvider = Current.DataProvider;

            if (isBorrowedTransaction && !
dataProvider.IsTransactionSupported)
            {
                if (transactionManager != null)
                    throw new Exception("Transaction Support is 
not included with the current DataRepository provider.  If using a provider 
that 
doesn't support transactions, such as a webservice, you should turn off 
transaction 
management.");
            }
            else if (isBorrowedTransaction && 
dataProvider.IsTransactionSupported)
            {
                if (transactionManager == null || !
transactionManager.IsOpen )
                    throw new ArgumentException("The 
transactionManager is in an invalid state for this method.  \nYou must begin 
the 
tranasction prior to using this method.");

            }
            else if (createTransaction && !isBorrowedTransaction && 
dataProvider.IsTransactionSupported)
            {
                transactionManager = CreateTransaction();
            }
            else if ( !createTransaction && !isBorrowedTransaction )
            {
                // if current transaction is not valid, but we don't 
require one, return null
                transactionManager = null;
            }

            return transactionManager;

You could in theory change the following line to look at 
TransactionScope.Current to 
be not null and infer that it is a "borrowed transaction"...

            bool isBorrowedTransaction = (transactionManager != null && 
transactionManager.IsOpen) || (TransactionScope.Current != null);

Or something of that nature...

Original comment by bh...@yahoo.com on 27 Apr 2010 at 9:20

GoogleCodeExporter commented 9 years ago
On further review - this whole block needs evaluated to take the 
TransactionScope 
into consideration ( i.e. I didn't notice the "check" to ensure the 
TransactionManager was open - same would apply to TransactionScope ).

Does the NetTiers "transaction manager" even serve a real role now that we have 
the 
TransactionScope?  Should it be deprecated in favor of using TransactionScope?

Original comment by bh...@yahoo.com on 27 Apr 2010 at 9:22

GoogleCodeExporter commented 9 years ago
Just following up since I've seen activity from the team on other issues... 
Please eyeball this one.. it's a true bug that can cause some serious headaches 
as well as significant performance degregation (enlisting a distributed 
transaction)...

Original comment by bh...@questis.com on 21 Sep 2010 at 5:52

GoogleCodeExporter commented 9 years ago
Hello,

Jeff, have you come across this issue before? I haven't seen this one in the 
forums.

Thanks
-Blake Niemyjski

Original comment by bniemyjski on 21 Sep 2010 at 8:33

GoogleCodeExporter commented 9 years ago
We discovered it because code we had used previously was in an environment 
where we had 60+ databases plus a centralized one... so obviously MSDTC was in 
place and configured correclty....

On a following project we were challenged to deploy the app to the cloud with 
Azure... They found Azure had no MSDTC support so to test if we could move to 
SQL Azure or not, we disabled MSDTC on our local machines....   During testing 
we found a specific scenario where it was trying to escalate to a distributed 
transaction and failing (as would be expected with MSDTC disabled)...

After digging into the offending code, it was found that the code causing 
escalation was the Delete method on the Service layer... and when we compared 
it to other Delete operations that were NOT causing the escalation - we found 
the different to be that in the one causing escalation we were passing the 
integer Primar Key where the other NOT causing escalation was being passed the 
Entity itself...

Once we stepped into the NetTiers code we found what was reported above...

Original comment by bh...@questis.com on 21 Sep 2010 at 9:07

GoogleCodeExporter commented 9 years ago
It seems the 'fix' here would be to simply pass in the 'noTranByDefault' value 
as the other operations do... I'm not sure which template file this would occur 
in...

Therfore our hack to the [ValidateOrCreateTransaction] operation to change the 
hard-coded 'true' to 'false' (which on 2nd thought wasn't the 'right' fix but 
it let us make 1 strategic centralized change to the templates with a lower 
risk/turn-around time since we aren't that familiar with the templates) - is 
un-needed and should not happen...

Additionally - I'm not sure what sets the actual value of 'noTranByDefault' 
since it seems to be a constant or something set by perhaps the properties 
file...

Original comment by bh...@questis.com on 21 Sep 2010 at 9:34

GoogleCodeExporter commented 9 years ago
I think I see it... In ComponentDataAcces.cst --- compare the Delete by PK 
below with the others... for the "transactionManager = 
ConnectionScope.ValidateOrCreateTransaction();" statement... almost all others 
call it with the 'noTranByDefault' argument... On searching I see DeepSave 
isn't doing it as well...  (here at the client-site I've got the 2.3.0 Beta 1 
templates on hand)...

        /// <summary>
        ///     Deletes a row from the DataSource based on the PK'S <%= GetFunctionHeaderParameters(keys) %>
        /// </summary>
        <% foreach(string item in GetFunctionCallParameters(keys).Split(',')) { %>
        /// <param name="<%= item.Trim() %>"><%=className%> pk id.</param>
        <% } %>
        <% if (RowVersion != null){ %>
        /// <param name="<%=GetFieldName(RowVersion)%>">The timestamp field used for concurrency check.</param>
        <% }  %>
        /// <remarks>Deletes based on primary key(s).</remarks>
        /// <returns>Returns true if operation suceeded.</returns>
        [DataObjectMethod(DataObjectMethodType.Delete)]
        <%= constructorAccessModifierOnMembers %> bool <%=partialClassInternalPrefix%><%= MethodNames.Delete %>(<%= GetFunctionHeaderParameters(keys) %><% if(RowVersion != null) {Response.Write(", byte[] " + GetFieldName(RowVersion));}%>)
        {
            #region Security check
            // throws security exception if not authorized
            SecurityContext.IsAuthorized("<%= MethodNames.Delete %>");
            #endregion Security check

            #region Initialisation
            bool result = false;
            bool isBorrowedTransaction = false;
            TransactionManager transactionManager = null; 
            NetTiersProvider dataProvider;
            #endregion Initialisation

            try
            {
                isBorrowedTransaction = ConnectionScope.Current.HasTransaction;

                //since this is a read operation, don't create a tran by default, only use tran if provided to us for custom isolation level
                transactionManager = ConnectionScope.ValidateOrCreateTransaction();
                dataProvider = ConnectionScope.Current.DataProvider;

                result = dataProvider.<%= GetClassName(SourceTable, ClassNameFormat.Provider) %>.<%= MethodNames.Delete %>(transactionManager, <%= GetFunctionCallParameters(keys) %><% if(RowVersion != null) {Response.Write(", " + GetFieldName(RowVersion));}%>);

                if (!isBorrowedTransaction && transactionManager != null && transactionManager.IsOpen)
                    transactionManager.Commit();
            }
            catch (Exception exc)
            {
                #region Handle transaction rollback and exception
                if (transactionManager != null && transactionManager.IsOpen) 
                    transactionManager.Rollback();

                //Handle exception based on policy
                if (DomainUtil.HandleException(exc, layerExceptionPolicy)) 
                    throw;
                #endregion Handle transaction rollback and exception
            }

            return result;

        }
        #endregion 
        <% } //endif(includedelete) %>

Original comment by bh...@questis.com on 21 Sep 2010 at 9:40

GoogleCodeExporter commented 9 years ago
Hello,

Thanks for updating this, it sure saves us a lot of time setting this up and 
investigating!

Thanks
-Blake Niemyjski

Original comment by bniemyjski on 21 Sep 2010 at 11:04