15

If i have an function that edits the database in my webservice that i only want one thread to execute at one time if they try to edit the same row.

void EditCheck(long checkid)
    {

        if (isCheckCosed)
            throw new Exception("check is already closed");

        //do stuff

        //after i edit my check i want to close it.
        CloseCheck();
    }

i know i can lock the whole function, but then i loose preformance because its almost never that different threads will try to edit the same check.

is there a way to only lock out other threads that have the same checkid?

UPDATE

i use OleDbConnection and MySqlConnection

OleDbCommand oleDbCommand = AccessTrans != null ?
new OleDbCommand(sql, AccessConn, AccessTrans) : new OleDbCommand(sql, AccessConn); 
oleDbCommand.ExecuteNonQuery();

the same function for MySqlCommand

and then i jus use the normal INSERT and UPDATE sql commands. the and the transaction is checked if it is present or not. so this functions works if you want an transaction or not in your upper level function.

for reading from the database i'll fill DataTable

OleDbCommand oleDbCommand = AccessTrans != null ? new OleDbCommand(sql, AccessConn, AccessTrans) : new OleDbCommand(sql, AccessConn);
OleDbDataAdapter dAdapter = new OleDbDataAdapter();
dAdapter.SelectCommand = oleDbCommand;
dAdapter.Fill(dTable);
return dTable;
Dendei
  • 551
  • 1
  • 7
  • 18
  • 2
    Shouldn't the this be handled on the database end? Do you actually have an issue or is this just based on assumptions? – Kyle C Sep 25 '13 at 15:27
  • How is data access written? If your'e using EF this may help:http://blogs.msdn.com/b/alexj/archive/2009/05/20/tip-19-how-to-use-optimistic-concurrency-in-the-entity-framework.aspx – Lev Sep 25 '13 at 15:30
  • 1
    Can you show how you are accessing the database, there are good tools built in to many data access methods to handle this problem (any solution happening client side only would only work if only one client ever connects to the database. If two copies of your program both call `EditCheck` for the same `checkid` the current answers would not work) – Scott Chamberlain Sep 25 '13 at 16:14
  • @ScottChamberlain updated my question to incluse database code. – Dendei Sep 26 '13 at 07:01

2 Answers2

28

You can use a ConcurrentDictionary to map each id to an object that you can lock on:

public class Foo
{
    private ConcurrentDictionary<long, object> dictionary = 
        new ConcurrentDictionary<long, object>();

    private void EditCheck(long checkid)
    {
        var key = dictionary.GetOrAdd(checkid, new object());
        lock (key)
        {
            //Do stuff with key
        }
    }
}
Servy
  • 202,030
  • 26
  • 332
  • 449
  • nice. an improvement over [this answer](http://stackoverflow.com/a/781324/571637) – jltrem Sep 25 '13 at 15:31
  • Thank you for your answer i will read up on ConcurrentDictionary and test this. But seems like just what i need. – Dendei Sep 25 '13 at 15:42
  • 2
    You may also want to look in to using a [ReadWriterLock](http://stackoverflow.com/questions/2116957/readerwriterlock-vs-lock) if you are going to be having a lot of threads doing only read-only operations on the `checkid` at the same time, this will let multiple threads all perform read operations on the same `checkid` but once someone wants to write it locks out all the concurrent readers. – Scott Chamberlain Sep 25 '13 at 16:12
  • Sorry if im slow on accepting anwers i havent been able to test this properly yet bacuase we will test today if our client computers are able to have .net 4.0. as they are based on xp but not listed in the Supported Operating Systems xD – Dendei Sep 26 '13 at 07:08
  • I know it depends on the exact requirement and how this class is going to be used, but you will *most likely* want to declare that `ConcurrentDictionary` as static, no? – Todd Menier May 30 '17 at 21:29
  • @ToddMenier No, I would be highly suspicious of that. You're right, that it would depend on the circumstances, but I would consider seeing a static version of this to be a very big red flag. – Servy May 30 '17 at 21:33
  • @Servy I guess I'm thinking of a typical server architecture where Foo is a service-y class that gets instantiated on demand as needed and checkid is some sort of unique identifier from the data store. Common scenario in my world but not all I suppose. – Todd Menier May 30 '17 at 21:42
  • 1
    Shouldn't we clean it up after we're done? Otherwise it's going to be an infinitely growing dictionary – Alex from Jitbit Jan 17 '20 at 17:56
  • @Alex It's not easy to to do without introducing race conditions, unless there's more domain specific information (i.e. some point based on the specifics of the situation where you know a given resource will never be accessed again. – Servy Jan 18 '20 at 02:38
0

As you want to improve performance, you should make locking at the recent moment as possible. As you use database, you can use DB locking. You got two options:

  1. Use Select for update, look at Proper way to SQL select and update
  2. Use optimistic locking which is IMHO best approach to do that, http://en.wikipedia.org/wiki/Optimistic_concurrency_control or http://docs.jboss.org/hibernate/orm/4.0/devguide/en-US/html/ch05.html

Note that both approaches allows you to use DB resources more than programmatic ones. This is better more that proposed dictionary, why?

Once you will duplicate data, those in DB are duplicated in the runtime dictionary, you are forced to sync them, so if someone updates database and add new id you need to immediately update dictionary.

Sure, it's doable for small maintaining (e.g. repository) class but once your project gets grow, some of your colleague can forget this information and you will start to find such kind of bugs.

Community
  • 1
  • 1
Martin Podval
  • 1,097
  • 1
  • 7
  • 16