Strategy for thread-safe database access

Question

Say I have the following code that inserts users into MS Dynamics in a MVC Application:

public bool CreateContact(string email)
{
    if (crm.contacts.Count(x => x.Email == email) > 0)
         return false; //Email already exist in the Crm. Skip

    var contact = new contact {Email = email};
    crm.AddTocontacts(contact);
    crm.SaveChanges();

    return true;
}

It works great for stopping users from registering with the same Email address, until recently when we are having MAJOR performance issues with Dynamics.

Apparently, users are getting a huge delay and often triple clicks the button that fires this piece of code.

Problem is, .Count() is firing at the same time in different Http Requests before .SaveChanges() finished in the first Request. As a result, we are seeing contacts with the same Email address.

While I already added a fix from the client-side, I would like to see if this can be done on the server side as well.

What would be a good strategy to make this thread-safe?

Edit:

While adding a constraint on the CRM is the best solution as many suggested here, I cannot implement that solution at the moment because duplicates already exist in the CRM long before this issue is discovered. Apparently there are more than one application that talks to the CRM.

With very little experience with locking and threading, I ended up doing the following:

internal static class ContactLock
{
    internal static readonly object Locker = new object();
}

public bool CreateContact(string email)
{
    lock(ContactLock.Locker)
    {
        if (crm.contacts.Any(x => x.Email == email))
            return false; //Email already exist in the Crm. Skip

        var contact = new contact {Email = email};
        crm.AddTocontacts(contact);
        crm.SaveChanges();

        return true;
    }
}

It passes my unit test and seems to be working without any issues.

Austin, Yup, I did that on the client side already. But I want to see if I can do that from the server as well. — Chi Chan, Jul 24 '12 at 21:32
Prefer `.Any(x => x.Email == email)` over `Count(x => x.Email == email) > 0` for a) intent and b) possible optimizations. — Jesse C. Slicer, Jul 24 '12 at 21:54
Are you using CRM 4 or CRM 2011? And are you using direct DB access? I wasn't aware the Linq provider allowed for the use of `Count`. — Peter Majeed, Jul 25 '12 at 03:51
@PeterMajeed: We are using CRM 4.0 and there is an SDK that you can download here: http://www.microsoft.com/en-us/download/details.aspx?id=38 . It provides a code gen tool that allows you to access the CRM with Linq. It is a poorly written piece of software that should be avoided IMO. (Can't say for 2011 perhaps it improved) — Chi Chan, Jul 25 '12 at 14:42

score 5 · Accepted Answer · answered Jul 24 '12 at 21:17

5

That kind of validation should typically be backed by a unique constraint in the underlying data store. If it is possible to create constraints in the CRM database, that's where the fix should go.

Your code snippet shows a typical place where some kind of locking is needed. The check (with Count()) and the SaveChanges() should be protected by a lock. I would suggest that you start with locking on an object that's static - which means it will be a global lock preventing simultanous registrations. If that proves to be a problem, you could revise the locking strategy.

Regarding the call taking a lot of time - that's something you should address. Adding a unique constraint to the email column will force it to be indexed, which will probably improve performance a lot. If possible (again, I don't know CRM) you should use linq's Any() operator instead of Count() to check for existence. The former can break on the first hit, while the latter will have to continue scanning.

answered Jul 24 '12 at 21:17

Anders Abel

67,989
17
150
217

I don't think application locks are such a good idea. What if the app is load-balanced? It won't work. – usr Jul 24 '12 at 21:50
No, it won't work well in a load-balanced scenario, in that case a DB constraint is the only way to go. – Anders Abel Jul 24 '12 at 21:52
1

Ok, you're right - a DB constraint is not the *only* way, but still what I suggest. Taking locks manually on the DB is awkward when a constraint handles it automatically. – Anders Abel Jul 24 '12 at 21:54
A constraint forces you to catch an exception. It also prevents you from applying any other changes in that datacontext because you cannot get rid of the insert in LINQ to SQL. Your datacontext is hosed. – usr Jul 24 '12 at 21:59
3

@usr: Dynamics CRM doesn't allow direct DB access in the way you suggest, so a DB unique index (with disposed data context) is likely the only way to go. – Peter Majeed Jul 25 '12 at 03:44
In that case you're right. In other settings, it is the best way, though. – usr Jul 25 '12 at 08:31

score 1 · Answer 2 · answered Jul 25 '12 at 05:57

1

a pre-create plug-in on the systemuser entity could check your constraint before passing each transaction onto the platform (i.e. email address is unique) and is the recommended / supported way to do this imo.

answered Jul 25 '12 at 05:57

Greg Owens

3,878
1
18
42

+1 for the server side solution, and we actually implement this as well, but what would happen in a high volume situation if two `Create` requests were sent at the same time with the same email address? I can see a scenario where they pass the `Count`/`Any` test due to timing, allowing duplicated emails, whereas an index on the DB would disallow this altogether. This would definitely be the best solution in an online-only deployment, though. – Peter Majeed Jul 25 '12 at 15:11
A good question and one that I don't know the answer to (though I suspect your premonition is correct!). Perhaps in an on-premise scenario, a `Mutex` could be introduced into the plug-in code? – Greg Owens Jul 26 '12 at 07:50

score 0 · Answer 3 · answered Jul 24 '12 at 21:51

You can add the following line at the beginning to take a write-lock in the database:

crm.ExecuteCommand("select ID from contacts with (updlock, holdlock) where EMail = {0}", EMail);

And you need to wrap a transaction around the method. This achieves uniqueness and is deadlock-free.

Strategy for thread-safe database access

Edit:

3 Answers3