views:

424

answers:

4

Hi, i am trying to insert the contents of a CSV file into a database table using linq2SQL.

I want to be able to rollback the transaction if ANY of the inserts fail but when i try with this code i get the following error at - db.Transaction.Commit()

System.InvalidOperationException was unhandled: This SqlTransaction has completed; it is no longer usable.

Does anyone know what i am doing wrong?

using (DataContext db = new DataContext())
{
    db.Connection.Open();
    db.Transaction = db.Connection.BeginTransaction();

    try
    {
     foreach (string entry in entries)
     {
      XXX xxx = new XXX()
      {
       P1 = "something",
       P2 = "something"
      };

      db.XXXX.InsertOnSubmit(xxx);
      db.SubmitChanges();
     }
    }
    catch (Exception)
    {
     db.Transaction.Rollback();
    }
    finally
    {
     db.Connection.Close();
    }

    db.Transaction.Commit();
}
+2  A: 

Is it because you do the commit after doing the rollback?

You should place the commit last inside the try block, so either rollback or commit are called. Never both...

UPDATE: As Peter mentions in his answer, I expect that neither the Close or Rollback statements are necessary, as the using block will Dispose (thus also Close) the connection, and a transaction that isn't comitted should automatically be rolled back.

Arjan Einbu
+2  A: 

Well, the ordering is wrong - you are calling db.Transaction.Commit() after the whole big block, so it'll be called even when an exception occured and you already called db.Transaction.Rollback();

Change your code to:

using (DataContext db = new DataContext())
{
    db.Connection.Open();
    db.Transaction = db.Connection.BeginTransaction();

    try
    {
        foreach (string entry in entries)
        {
            ....
            db.XXXX.InsertOnSubmit(xxx);
            db.SubmitChanges();
        }

        db.Transaction.Commit(); <== CALL HERE !!
    }
    catch (Exception)
    {
        db.Transaction.Rollback();
    }
    finally
    {
        db.Connection.Close();
    }
}

In this case, your Commit is called after the foreach, but it will NOT be called if you run into an exception and do a rollback.

Marc

marc_s
+2  A: 

Based on the fact that "using datacontext" will ensure that the current transaction and connection will be closed, I will assume that the following block should be sufficient:

01.    using (DataContext db = new DataContext())
02.    {    
03.        db.Connection.Open();    
04.        db.Transaction = db.Connection.BeginTransaction();    
05.
06.        foreach (string entry in entries)        
07.        {                
08.            XXX xxx = new XXX()                
09.            {                        
10.                P1 = "something",                        
11.                P2 = "something"                
12.            };                
13.            db.XXXX.InsertOnSubmit(xxx);                
14.        }    
15.        db.SubmitChanges();        
16.
17.        db.Transaction.Commit();
18.    }

If an exception occurs between line 05 and 16 the transaction will never be marked with Commit and thus rolled back as soon as the transaction and connetion is finalized at line 18.

Note: there is a difference in behavior here which I'm not sure is intentional or not: in addition to rolling back the transaction, your catch block swallows the exception and thus hides the fact that an error have occured.

Update: I would also move the SubmitChanges call out of the inner loop. You should be able to first do your inserts and then the submit changes once for all changes.

Peter Lillevold
is that actually the case? will a transaction be automatically rolled back if its not explicitly committed? is there a performance hit in removing the Close() call?
Grant
That is the case when doing transactions in ADO.NET (SqlTransaction), so I would expect the same behaviour in L2S. (Trasactions are by default rolled back unless explicitly comitted.)
Arjan Einbu
DataContext.Connection uses DbConnection. The Close method will automatically rollback pending transactions that have not been committed. http://msdn.microsoft.com/en-us/library/system.data.common.dbconnection.close.aspx
Peter Lillevold
Regarding performance: the same work is being done (rolling back transactions and closing the connection) so I would expect the same performance with our without an explicit Close call. In this case I will prefer readability vs saving a few cpu cycles :)
Peter Lillevold
A: 

With regard to the code posted in "Peter Lillevold"'s answer : I can see that if an error occurs at line 15 e.g. db.SubmitChanges(), it does not close the database connection. Therefore, the correct solution is :

enter code here
      using (DataContext db = new DataContext())
        {
            // The dispose method of DbConnection will close any open connection
            // and will rollback any uncommitted transactions
            using (DbConnection dbConnection = db.Connection)
            {
                dbConnection.Open();
                db.Transaction = dbConnection.BeginTransaction();
                foreach (string entry in entries)
                {
                    XXX xxx = new XXX()
                    {
                        P1 = "something",
                        P2 = "something"
                    };
                    db.XXXX.InsertOnSubmit(xxx);
                }
                db.SubmitChanges();
                db.Transaction.Commit();
            }
        } 

PS: For further explaination, please refer to http://msdn.microsoft.com/en-us/library/bb292288.aspx, which says "If you provide an open connection, the DataContext will not close it. "

akapoor