Oct 19, 2015

|

by: james

|

Tags: Debugging, Lightweight Tools, Unit Testing

|

Categories: Tech Tips

Massive ORM and SCOPE_IDENTITY() vs @@IDENTITY

Massive ORM and SCOPE_IDENTITY() vs @@IDENTITY

We’ve been using Massive for a few years now and have been very pleased with it.  If you don’t know, Massive is a micro ORM that makes use of C#’s dynamic data type.  It’s super lightweight — in fact, it’s a couple hundred lines of code in a single file that you drop into your project.

Well, we stumbled across a bug the other day that had us scratching our heads for a while, and it was one of those inconsistent bugs that scream “threading” problem, but we were running in a largely single threaded process.  Here’s the run down:

  1. We have a list of records that we insert into the DB.
  2. Iterate through that list of records and identify duplicates based on some specific criteria.
  3. Update (we actually don’t do an update, but rather insert a second record into this table, so we have historical point in time data, but that’s not important) those records that we’ve marked as duplicate as such in the DB.

Seems pretty easy, right?  Well, what we found was that some records were not being marked as duplicate.  As an example, we started with 60k records, of which 4k were dupes.  But, at the end of this process we had 32 duplicate records that were not marked as such.  I should mention that those 32 fell into the same criteria as the others.

The first thing we tried was to reproduce this scenario in our unit tests, but those tests passed with flying colors.

The next thing we tried was to reproduce this scenario in an integrated, manual fashion and debug.  We reset the scenario and put in some conditional breakpoints to break when one of those specific “missed” records were encountered.  Stepping through the logic for one of those records showed that the duplicate logic was working.  That the record was getting marked as a duplicate and updated in the database.  Wait . . . we just saw this not work previously but now it is working!  Here’s the thing, re-running would yield a different set of records that were “missed”.  More interestingly, the duplicate logic looked correct, but it was occasionally marking a record that wasn’t a duplicate a duplicate.  That is, if we had a set { A, B, C } of which A and B were dupes, the logic would correctly identify A and B as dupes, but the update would mark C as a dupe.

A couple of us got together to bounce some ideas around.  Since the set of records that were missed was different for each run, we concluded that it must be a threading issue.  Well, the duplicate logic isn’t multithreaded . . . but, the insert logic is.  But, the inserts are fine.  We see all the records in the DB after the inserts.  No problem, let’s test that out.  We implemented the inserts as a Parallel.ForEach and already had a configuration setting in place to allow us to set that to 1.  When we did that, it worked fine — so, the issue must be in the parallel inserts.  But, all that was wrapped up in a single call:

ConcurrentBag<Record> records = GetRecords();
Parallel.ForEach(records, parallelOptions, record =>
{
record.Id = RecordRepository.Add(record);
});

We were appropriately using a threadsafe container in the ForEach, so that wasn’t the issue.  The RecordRepository was also basically a one liner using Massive:

dynamic table = new DynamicModel(“Database”, tableName: “Database.dbo.Record”, primaryKeyField: “Id”);
var row = table.Insert(new
{
Data = record.Data
});
return Convert.ToInt64(row.ID);

So, what could it be?  Well, it must be some of the insert code, we thought.  No big deal, let’s jump right into the Massive code.  This is one reason we like lightweight tools.  We can get in and easily understand what is going on and in this case, easily step right into the third party code base.  The Massive code that we were using had this implementation that had alarm bells going off for us:

using (dynamic conn = OpenConnection()) {
var cmd = CreateInsertCommand(ex);
cmd.Connection = conn;
cmd.ExecuteNonQuery();
cmd.CommandText = “SELECT @@IDENTITY as newID”;
ex.ID = cmd.ExecuteScalar();
Inserted(ex);
}

Right away we saw the @@IDENTITY as being a problem in a multi-threaded insert scenario.  @@IDENTITY returns the last identity returned for any table in the current session.  In our case, this was limited to one table, but the problem is that the INSERT / SELECT here isn’t an atomic operation.  We could be getting a different ID returned to us than the one we just inserted because another thread inserted another row!

Easy enough, we can change that to SCOPE_IDENTITY() — which will return the last identity inserted in the current scope!  Well, it’s not quite that easy because the way it’s written above, the select runs in a separate scope and you’d get NULL (we know this because that was our first attempt).

We found this line of code in the CreateInsertCommand method above:

var stub = “INSERT INTO {0} ({1}) \r\n VALUES ({2})”;

And we changed it to this:

var stub = “INSERT INTO {0} ({1}) \r\n VALUES ({2});SELECT SCOPE_IDENTITY() as newID;”;

And then changed the code in the Insert method to:

using (dynamic conn = OpenConnection()) {
var cmd = CreateInsertCommand(ex);
cmd.Connection = conn;
var id = cmd.ExecuteScalar();
ex.ID = id;
Inserted(ex);
}

Problem solved!

Of course, the guys that are maintaining Massive have made a similar change in one of their branches (but it’s not the “released” branch yet).  You might think that we’ve come to the conclusion that we should avoid open source, third party tools like this, but that’s far from it.  This actually cements our philosophy to rely on simple, lightweight tools.  Bugs happen everywhere and we’d rather use one that we can get in and come up-to-speed on and deeply understand than one that is a black box that leaves us without option if we need to do something that’s unsupported.