2

I am trying to run a test case and trying to pass two schedules inside like below:

 var itemToAdd = new ScheduleInputItemDto
    {
        Start = DateTime.UtcNow,
        End = DateTime.UtcNow.AddHours(1),
        ProdType = "Prod1"
    };



 var response = await Task.WhenAll(addItemRequest.Post(itemToAdd, false), addItemRequest.Post(itemToAdd, false));

This will post two items with same start and end time, which is causing a race condition

AddItem calls the DAO layer like below.

public Schedule()
{
    ScheduleItems = new List<ScheduleItem>();
}


public ICollection<ScheduleItem> ScheduleItems { get; set; }

public void AddItem(DateTime start, DateTime end, string cementType, DateTime now)
{
    var item = new ScheduleItem(start, end, cementType, now);
    ScheduleItems.Add(item);
    ConcurrentBag<ScheduleItem> concurrentBag = new ConcurrentBag<ScheduleItem>(ScheduleItems.ToList());        
    item.ValidateDoesNotOverlapWithItems(concurrentBag);
    
}

But for my case, it is inserting both of them instead of the following check I made:

The validation code is below:

    public static void Validate(this ScheduleItem currentItem, ConcurrentBag<ScheduleItem> scheduleItems)
    {
        if (scheduleItems.Any(scheduleItem => currentItem.Start < scheduleItem.End && scheduleItem.Start < currentItem.End))
        {
            throw new ValidationException("A conflict happened.");
        }
    }

The ScheduleItem model has a property named UpdatedOn which can be used as a concurrency token.

After debugging the test case, I saw both of the items posted from inside .WhenAll() have exact same DateTime. How can I prevent the later item to be inserted? Optimistic or Pessimistic concurrency control should be used in this case?

Priom Biswas
  • 85
  • 2
  • 12
  • What is the type of the application? ASP.NET? – Theodor Zoulias Mar 15 '22 at 08:01
  • ASP.NET core with .NET 6 – Priom Biswas Mar 15 '22 at 08:02
  • Is it possible for one request to interfere with another concurrent request by a different client? – Theodor Zoulias Mar 15 '22 at 08:22
  • No, it is not possible. If there are two schedules with same start time, always only one should win, not both. – Priom Biswas Mar 15 '22 at 08:28
  • Put adding items in the queue and process the queue in single thread one after another - then validation will prevent "similar" scheduled to be added. – Fabio Mar 15 '22 at 08:31
  • You could consider using a `lock` (synchronous) or a `SemaphoreSlim(1, 1)` (asynchronous) to prevent concurrent execution of a block of code within a single process, but I am not sure if ASP.NET uses always a single process per site. – Theodor Zoulias Mar 15 '22 at 08:36
  • After debugging, I have found that in the following `Validate(this ScheduleItem currentItem, List scheduleItems)` the scheduleItems is always empty, I am not sure what causing this list to be empty all the time. – Priom Biswas Mar 15 '22 at 08:50
  • 1
    Can you post a complete working example? You should return the list of values from `Task.WhenAll` and validate/add them after to avoid concurrency. – Paulo Morgado Mar 15 '22 at 10:19
  • Actually this is a huge project, with database connections and docker stuffs. I have provided the validation code above, the problem is WhenAll(item1, item2) posts the two items at exactly same time, and my code now inserting both of them. But this should not happen. – Priom Biswas Mar 15 '22 at 10:29
  • @Fabio Probably an extension to **List** that can check the validation? – Priom Biswas Mar 15 '22 at 12:01
  • Your question needs much more information before it can be answered. For example, what does `Post` do? What type is `ScheduleItems`? Please post a minimal reproducible example. – Stephen Cleary Mar 15 '22 at 13:06
  • @StephenCleary Please check the updated post with service layer code. – Priom Biswas Mar 15 '22 at 13:44
  • `ScheduleItems.ToList()` why do you have `ToList`, why not pass an `IEnumerable`? Also, repositories are generally not thread-safe, so it's unclear why you would try to write to the same object on two different threads – Charlieface Mar 15 '22 at 14:07
  • @Charlieface it is basically a test case to insert same object and check whether the business logic can remove the duplicate request based on the validation. – Priom Biswas Mar 15 '22 at 14:12
  • To prevent duplicate inserts, you need something like `insert () select .... where not exists(select ....)`, or a unique constraint, so that the DB can enforce the constraint in a (hopefully?) atomic way. – Jeremy Lakeman Mar 21 '22 at 00:35

2 Answers2

2

In summary, when using a database with multiple actors (e.g. threads, requests), it is hard to ensure that the data doesn't change between a read (required for validation) and a write (insert/update).

In my opinion, the only sure way to handle race conditions is to try the insert/update and act on failures.

There is a path of using consistency levels and transactions, where the row - or the whole table - is locked for the whole unit-of-work (read, do things, write) but for this to work a strong change control is required so that the system not inadvertently broken by changing one piece and not knowing another part depends on it.

Optimistic concurrency model

One simple way to with concurrency for database updates is to use a token to help detecting conflicts. For example:

update x set y = z, timestamp = now where timestamp = timestamp_from_my_last_read;

If number of updated rows is 0 then you know someone updated the row and you need to retry, report error, or do whatever else is required. You don't need to use a timestamp; the token can be anything, even all the values - the key element is where something = something_value_when_I_last_read_this_row.

This method is called optimistic because it assumes things will be OK and you react on failures rather than assume that things will be bad from the start.

Some ORMs, including Entity Framework, natively support for this kind of concurrency handling. Please see EF Core's Handling Concurrency Conflicts.

Data store in memory of a single process

If your data store is in memory of a single process you can protect against race conditions with locking.

Let's consider this method

void Insert(X toInsert) 
{
  lock (collectionLock)
  {
     var collection = GetCollection(); 
     if (ValidateOrThrow(collection, toInsert))
     {
        collection.Insert();
     }
  }
}

As long as:

  • all code paths that wish to insert use Insert method
  • other methods (Delete, etc.) also use collectionLock when interacting with the collection

then it is guaranteed that the collection is not modified between GetCollection and .Insert.

tymtam
  • 31,798
  • 8
  • 86
  • 126
  • Should I use **Pessimistic concurrency control** to prevent duplicate insertion? How can I use a try catch block in async Task AddItemToSchedule function to handle the situation, means preventing the invalid schedule insert? Invalid schedules are those who have same start time. – Priom Biswas Mar 17 '22 at 17:30
  • Also please check the updated post – Priom Biswas Mar 17 '22 at 17:38
0

List<ScheduleItem> scheduleItems is not thread-safe. Try using a ConcurrentBag<ScheduleItem>

klekmek
  • 533
  • 3
  • 11
  • also added the full DAO layer in the post – Priom Biswas Mar 15 '22 at 14:22
  • Updated the post with ConcurrentBag, but now it throws exception, and the first one element is not get inserted. – Priom Biswas Mar 15 '22 at 14:28
  • 1
    The `ConcurrentBag` is not a thread-safe `List`. It's a [highly specialized](https://stackoverflow.com/questions/15400133/when-to-use-blockingcollection-and-when-concurrentbag-instead-of-listt/64823123#64823123) collection with very few real-world applications. If you just want a container that allows concurrent adding and enumeration, the `ConcurrentQueue` is a better collection to use. – Theodor Zoulias Mar 15 '22 at 14:33