CODEWITHSUNDEEP
c#asp.netasp.net-mvcmultithreadingc#-4.0
User987
User987

Reputation: 3823

Parallel.ForEach loop Allow only one thread to access DB in C#

I have a piece of code where multiple threads are accessing using a shared ID property from ConcurrentBag type of string like following:

var ids = new ConcurrentBag<string>();
// List contains lets say 10 ID's 
var apiKey = ctx.ApiKey.FirstOrDefault();

Parallel.ForEach(ids, id => 
{
    try
    {
        // Perform API calls
    }
    catch (Exception ex)
    {
        if (ex.Message == "Expired")
        {
            // the idea is that if that only one thread can access the DB record to update it, not multiple ones
            using (var ctx = new MyEntities())
            {
                var findApi= ctx.ApiKeys.Find(apiKey.RecordId);
                findApi.Expired = DateTime.Now.AddHours(1);
                findApi.FailedCalls += 1;
            }
        }
    }

});

So in a situation like this if I have a list of 10 ids and 1 key that is being used for API call, once the key reachces hourly limit of calls, I will catch the exception from the API and then flag the key not to be used for the next hour.

However, in the code I have pasted above, all of the 10 threads will access the record from DB and count the failed calls as 10 times, instead of only 1..:/

So my question here is how do I prevent all of the threads from doing the update of the DB record, but instead to only allow one thread to access the DB, update the record (add failed calls by +1) ?

How can I achieve this?

Upvotes: 1

Views: 800

Answers (2)

Zhaph - Ben Duguid
Zhaph - Ben Duguid

Reputation: 26956

You are in a parallel loop, therefore the most likely behaviour is that each of the 10 threads are going to fire, try to connect to your API with the expired key and then all fail, throwing the exception.

There are a couple of reasonable solutions to this:

Check the key before you use it

Can take the first run through the loop out of sequence? for example:

var ids = new ConcurrentBag<string>();
var apiKey = ctx.ApiKey.FirstOrDefault();

bool expired = true;

try {
  // Perform API calls
  expired = false;
}
catch(Exception ex) {
   // log to database once
}

// Or grab another, newer key?
if (!expired)
{
  Parallel.ForEach(ids.Skip(1), id => 
  {
     // Perform API Calls
  }
}

This would work reasonable well if the key was likely to have expired before you use it, but will be active while you use it.

Hold on to the failures

If the key is possibly valid when you start but could expire while you are using it you might want to try capturing that failure and then logging at the end.

var ids = new ConcurrentBag<string>();
var apiKey = ctx.ApiKey.FirstOrDefault();

// Assume the key hasn't expired - don't set to false within the loops 
bool expired = false;

Parallel.ForEach(ids.Skip(1), id => 
{
  try {
     // Perform API calls
  }
  catch (Exception e) {
    if (e.Message == "Expired") {
      // Doesn't matter if many threads set this to true.
      expired = true;
    }
  }

  if (expired) {
    // Log to database once.
  }
}

Upvotes: 1

NDJ
NDJ

Reputation: 5194

It looks like you only need to update apiKey.RecordId once if an error occurred, why not just track the fact that an error occurred and update once at the end? e.g.

var ids = new ConcurrentBag<string>();
// List contains lets say 10 ID's 
var apiKey = ctx.ApiKey.FirstOrDefault();
var expired = false;

Parallel.ForEach(ids, id => 
{
    try
    {
        // Perform API calls
    }
    catch (Exception ex)
    {
        if (ex.Message == "Expired")
        {
           expired = true;
        }
    }
}

if (expired)
{
   // the idea is that if that only one thread can access the DB record to 
   // update it, not multiple ones
   using (var ctx = new MyEntities())
   {
     var findApi= ctx.ApiKeys.Find(apiKey.RecordId);
     findApi.Expired = DateTime.Now.AddHours(1);
     findApi.FailedCalls += 1;
    }
});

Upvotes: 2

Related Questions