3

I'm completely frustrated and confused about the behavior of EF6.1 and the SQL Server.

I want to have a table with (at the moment) one row which contains a value I want to use for other tasks. Every time a user is executing this function, I want to get the content of the value - for simplicity just an int - and set a new value. Because I want to use this value also as a primary key for other tables, it has to be unique and it should be gap-less.

My idea was to create a transaction to get the value, calculate a new one and update it in the db. I want to block the table as long as the new value is commited. Then the next one can get and set the new value.

But after 5 hours of testing it does not work yet. To simulate (just 3) simultaneous accesses, I wanted to create a console app and start it 3 times with a build-in Thread.Sleep(200).

Here is my code I am testing with:

[Table("Tests")]
public class Test
{
    public int Id { get; set; }
    public int Value { get; set; }
}

The table already exists and it has one row with { Id = 1, Value = 0 } in it.

In my console app I have:

for( int i = 1; i <= 200; i++ )
{
    Thread.Sleep( 200 );
    using( var db = new MyDbContext( connectionString ) )
    {
        using( var trans = db.Database.BeginTransaction( IsolationLevel.RepeatableRead ) )
        {

            var entry = db.Tests.Find( 1 );
            db.Entry( entry ).Entity.Value += 1;

            // Just for testing
            Console.WriteLine( string.Format( "{0,5}", i ) + "   " + string.Format( "{0,7}", db.Entry( entry ).Entity.Value ) );

            db.SaveChanges();
            trans.Commit();
        }
    }
}

My problem: Sometimes it is enough to start the second app and I get deadlocks. I tried to set the isolation level to IsolationLevel.ReadCommited, but after doing that I got duplicates. How I have got absolutely no idea what I am doing wrong. Can someone help me, please?



UPDATE

When I take the for into the transaction, the second started app will wait as long as the first one runs (about 20 seconds) and then start to read and update the value. This works as expected, but why is the "simulation" above not working?

using( var db = new MyDbContext( connectionString ) )
{
    using( var trans = db.Database.BeginTransaction( IsolationLevel.Serializable ) )
    {
        for( int i = 1; i <= 2000; i++ )
        {
            var entry = db.Tests.Find( 1 );
            db.Entry( entry ).Entity.Value += 1;

            // Just for testing
            Console.WriteLine( string.Format( "{0,5}", i ) + "   " + string.Format( "{0,7}", db.Entry( entry ).Entity.Value ) );

            db.SaveChanges();
        }
        trans.Commit();
    }
}




SOLUTION

Thanks to Travis J here the solution:

for( int i = 1; i <= 2000; i++ )
{
    using( var db = new MyDbContext( connectionString ) )
    {
        using( var trans = db.Database.BeginTransaction( IsolationLevel.Serializable ) )
        {

            db.Database.ExecuteSqlCommand( "SELECT TOP 1 *FROM Tests WITH (TABLOCKX, HOLDLOCK)" );

            var entry = db.Tests.Find( 1 );
            db.Entry( entry ).Entity.Value += 1;

            // Just for testing
            Console.WriteLine( string.Format( "{0,5}", i ) + "   " + string.Format( "{0,7}", db.Entry( entry ).Entity.Value ) );

            db.SaveChanges();
            trans.Commit();
        }
    }
}

... and let me add a comment:
In my case it also works with IsolationLevel.RepeatableRead.
db.Database.ExecuteSqlCommand( "SELECT TOP 1 *FROM Tests WITH (TABLOCKX, HOLDLOCK)" ); only works inside the transaction. As I turned out I had tried this code without any transaction and the result was identically to use a transaction with IsolationLevel.ReadCommited.

Travis, thank you!

Ronin
  • 179
  • 11
  • How is the key defined for your test table? Is it a composite of id and value? – Travis J Aug 21 '15 at 21:41
  • @TravisJ No, it is like shown above. I use Code First with the code above and there were no changes made to the database afterwards. Is there a problem? – Ronin Aug 22 '15 at 06:47

3 Answers3

3

The phenomenon that you are experiencing could be caused by one of two factors. It is hard to tell without seeing the table design itself in MSSQL. Those factors are either a dirty read, or a phantom read. The value that is being changed is being read just prior to the change taking effect. This results in the value being incremented twice but both times were from x -> y, x-> y as opposed to going from x -> y, y -> z (which is why you observe a number less than the expected total.

RepeatableRead:
Locks are placed on all data that is used in a query, preventing other users from updating the data. Prevents non-repeatable reads but phantom rows are still possible. IsolationLevel Enumeration MDN (emphasis mine)

If the primary key is changing, the lock will be released and will act similar to a new row. This row can then be read, and the value would be the one prior to the update. This is possibly happening, although I believe it is most likely a dirty read.

A dirty read can occur here because the row is only locked while the query is executing, which is to say that during the fetch and during the save, the query is executed. So in between that when the actual value is being modified, it is possible that the value in the database is read and results in 10 becoming 11 twice.

In order to prevent this, an even stricter lock must be used. Serializable seems like the natural choice here, as it states that the lock is held until the transaction is complete, and that no shared locks are issued. However, dirty reads are still possible when using the serializable scope. The only real option here is going to be to lock the entire table.

https://stackoverflow.com/a/22721849/1026459 agrees as well, and gives the example code:

entities.Database.ExecuteSqlCommand(
    "SELECT TOP 1 KeyColumn FROM MyTable WITH (TABLOCKX, HOLDLOCK)"
);

Which would translate into your situation like this (note, I also changed the isolation to serializable with the hopes of further strengthening the lock)

for( int i = 1; i <= 200; i++ )
{
    Thread.Sleep( 200 );
    using( var db = new MyDbContext( connectionString ) )
    {
        using( var trans = db.Database.BeginTransaction( IsolationLevel.Serializable ) )
        {

            db.Database.ExecuteSqlCommand(
                "SELECT TOP 1 FROM Test WITH (TABLOCKX, HOLDLOCK)"
            );

            var entry = db.Tests.Find( 1 );
            db.Entry( entry ).Entity.Value += 1;

            // Just for testing
            Console.WriteLine( string.Format( "{0,5}", i ) + "   " + string.Format( "{0,7}", db.Entry( entry ).Entity.Value ) );

            db.SaveChanges();
            trans.Commit();
        }
    }
}
Community
  • 1
  • 1
Travis J
  • 81,153
  • 41
  • 202
  • 273
  • Just a little correction for the next readers: In my case the sql command is `"SELECT TOP 1 * FROM Tests WITH (TABLOCKX, HOLDLOCK)"`. **THANK YOU so much!!** The bad news for me: I had this line already tested, but it had not worked ... obviously at that time I had also another problem. Last but not least: Can you tell my, why the code in my _Update_ works without the tablelock? – Ronin Aug 22 '15 at 07:30
  • I'm thinking where's `rollback`?! Since `DbContextTransaction` surrounded by `using` block, there's no need to call `trans.Rollback()`? Is right? – ABS Aug 07 '17 at 18:40
  • 1
    @AliBagheriShakib since he's using `using`, there is no need for calling `trans.Rollback();` in a try...catch-phrase. It's automatically called in the `Dispose` of the `using`. See [this answer](https://stackoverflow.com/a/24592536/140059) for a more detailed explanation. – RickardN Apr 11 '18 at 09:35
1

I would use TransactionScope instead. Folks will warn you that it will elevate to use distributed transactions, but that's only if it needs it. Any inner transactions (like the one created with SaveChanges()) should enlist itself in the outer ambient scope.

TransactionScope creation code

public static TransactionScope CreateTransactionScope(TransactionScopeOption transactionScopeOption = TransactionScopeOption.Required)
        {
            var transactionOptions = new TransactionOptions
            {
                IsolationLevel = System.Transactions.IsolationLevel.ReadCommitted,
                Timeout = TransactionManager.MaximumTimeout
            };

            return new TransactionScope(transactionScopeOption, transactionOptions);
        }

Usage

using(var transaction = CreateTransactionScope()){
//do stuff

transaction.Complete();  //don't forget this...and it's 'complete' not 'commit'
}
BlackjacketMack
  • 5,472
  • 28
  • 32
  • I tried your code, but it produces an equal result as `.BeginTransaction()`. I started the app 5 times with a delay of 3 seconds. When the value is 0 the expected result is 1000 because of 5*200 increments. But the result was: 855. That means 145 values were used twice. – Ronin Aug 21 '15 at 20:31
  • I think I see what you're saying. There's no lock on the 'read' side of things. If you make this a single sql statement you'll be guaranteed the transaction and won't even have to mess with wrapping transactions or anything. UPDATE dbo.Tests SET Value = Value + 1 – BlackjacketMack Aug 22 '15 at 10:31
0

Locking...Take 2.

Locking entire tables is messy business. So I would approach this in one of two ways:

1.) Perform operation in SQL statement

You could simply increment the value in the update statement.

UPDATE dbo.Tests
SET Value = Value + 1
OUTPUT inserted.Id, inserted.Value
WHERE ...

This will guarantee that 'Value' is only ever incremented once. Note the OUTPUT clause which guarantees a clean read. That row is locked for the operation and read.

2.) Use RowVersions

Why not just have them also try to identify the row with an ID AND ROWVERSION in the WHERE clause. The ROWVERSION data type will always change whenever a row is touched by an update (even if the value doesn't change).

Although you might get updates that are no longer the most recent version of the row, it's a cleaner approach IMHO than waiting for something to be deadlocked.

BlackjacketMack
  • 5,472
  • 28
  • 32