Concurrent Requests Handling in ASP.NET Web API 2

Question

I was having concurrency issues with my ASP.NET Web API 2 project with .NET Framework 4.6.2. (EF 6), although it has been live on production that has been running for more than 1 year, this problem has just arrived from a customer. I sent two concurrent requests to this specific endpoint from Postman, 2 records need to be updated but only one record is updated in the database.

The service I'm having this issue:

private HttpResponseMessage CallGameNew(RequestDto requestDto)
{
    // Code omitted for brevity.
    
    List<GameBank> gameBankResult = null;

    //Query GameBank database
    gameBankResult = _unitOfWork.GameBankRepository.GetGames(g =>
        g.productCode == requestDto.productCode && g.referenceId == Guid.Empty);

    if (gameBankResult != null && gameBankResult.Count() >= requestDto.quantity)
    {
        var k = requestDto.quantity - 1;
        for (var i = k; i >= 0; --i)
        {
            gameBankResult[i].clientTrxRef = gameRequest.clientTrxRef;
            gameBankResult[i].referenceId = gameRequest.referenceId;
            gameBankResult[i].requestDateTime = DateTime.Now;
            gameBankResult[i].responseDateTime = DateTime.Now;
        }

        //***** UPDATE GameBank *****
        _unitOfWork.GameBankRepository.Update(gameBankResult[k]);

        if (requestDto.quantity == 1)
        {
            //Code omitted for brevity.
        }
            
    }

    _unitOfWork.Save();

    return response;
}

I tried many things to successfully support concurrent requests and used WebApiThrottle but It was not right to restrict online customers. I asked this in this post with the support of helpful software developers I came up with this solution.

I tested this with JMeter (10 threads, ramp-up period: 1 sec, 3 loops). I got no errors and I got 30 records updated in the database as well.

private HttpResponseMessage CallGameNew(RequestDto requestDto)
{
    HttpResponseMessage response = null;

    //ProductCode Conversion
    var productCode =
        _unitOfWork.ProductCodeRepository.GetByCode(p => p.clientCode == requestDto.productCode);

    if (productCode != null)
    {
        requestDto.productCode = productCode.gameCode;
    }

    var gameRequest = _mapper.Map<RequestDto, GameRequest>(requestDto);

    //Unique reference ID
    gameRequest.referenceId = Guid.NewGuid();

    var gameRequestDto = _mapper.Map<GameRequest, GameRequestDto>(gameRequest);
    //Create signature
    gameRequest = UtilitiesWatson.CreateSignature(gameRequestDto, RequestType.Initiate);

    //Set service
    gameRequest.service = "OUR";
    gameRequest.customerID = 5; //WATSON
    gameRequest.clientTrxRef = requestDto.clientTrxRef; //WATSON

    //Add initiation request into database
    _unitOfWork.GameRepository.Insert(gameRequest);
    _unitOfWork.Save();

    GameBank gameBankResult = null;

    while (true)
    {
        try
        {
            gameBankResult = _unitOfWork.GameBankRepository.GetGame(g =>
                g.productCode == requestDto.productCode && g.referenceId == Guid.Empty);
            _unitOfWork.Save();

            if (gameBankResult != null)
            {
                gameBankResult.clientTrxRef = gameRequest.clientTrxRef;
                gameBankResult.referenceId = gameRequest.referenceId;
                gameBankResult.requestDateTime = DateTime.Now;
                gameBankResult.responseDateTime = DateTime.Now;

                _unitOfWork.GameBankRepository.Update(gameBankResult);
                _unitOfWork.Save();
                break;  //exit from while loop
            }
        }
        catch (DbUpdateConcurrencyException)
        {
            _unitOfWork.ClearChangeTracker(); //IS REQUIRED, so the next select will read new RowVersion also
            Thread.Sleep((new Random()).Next(0, 1000)); //if you want to add a random pause 0-1 second
        }
    }

    var gameBankConfirmResponse =
            _mapper.Map<GameBank, GameConfirmResponse>(gameBankResult);

    gameBankConfirmResponse.purchaseStatusDate = DateTime.Now;
    gameBankConfirmResponse.clientTrxRef = gameRequest.clientTrxRef;

    //ProductCode Conversion
    var productCodeReverse = _unitOfWork.ProductCodeRepository.GetByCode(p =>
        p.gameCode == requestDto.productCode);

    if (productCodeReverse != null)
    {
        gameBankConfirmResponse.productCode = productCodeReverse.clientCode;
    }

    var resultResponse = JsonConvert.SerializeObject(gameBankConfirmResponse,
        Formatting.Indented,
        new JsonSerializerSettings()
        {
            ReferenceLoopHandling = ReferenceLoopHandling.Ignore
        });
    response = new HttpResponseMessage
    {
        StatusCode = System.Net.HttpStatusCode.OK,
        Content = new StringContent(resultResponse, System.Text.Encoding.UTF8,
            "application/json"),
    };
    //Set service
    gameBankConfirmResponse.service = "OUR";
    gameBankConfirmResponse.clientTrxRef = requestDto.clientTrxRef;

    _unitOfWork.GameConfirmResponseRepository.Insert(gameBankConfirmResponse);
    _unitOfWork.Save();

    return response;
}

Here is my ClearChangeTracker in the unit of work. (Does this affect other operations?)

public void ClearChangeTracker()
{
    foreach (var entry in _context.ChangeTracker.Entries())
    {
        entry.State = EntityState.Detached;
    }
}

Can you share if there are issues that I can improve or that may be a problem? I'm curious about your comments.

Thank you.

Answer 1

Code does not follow the following best practices :

1. Naming convention (properties should be PascalCased).
2. Method should have a single responsibility.
3. Multiple database roundtrips.
4. Possible bug with inconstant of saving request/response entries.
5. Unnecessary using of Thread.Sleep and new Random().
6. calling DateTime.Now repeatedly will give inconstant time between records, if you want to have a constant time between records, you have to declare it once, and reuse it (e.g. var time = DateTime.Now; .
7. Mapping objects, and then re-assign values to properties manually, is not necessary, let AutoMapper map it instead from the source.

For concurrency part, Microsoft has blogged about it for EF6 and EFCore, which both are closely similar solutions. You can read them, as they are better than clearing out the context just for one entity!.

Also, it does not require you to block the thread using Thread.Sleep, nor using Random.

To summarize the above (EF6) blog, you have three approaches that you can use in case of concurrency (For EF6):

1. Ignore current changes, and reload the latest version from the database.
2. Ignore database values, and just commit current changes.
3. Compare both versions, and decide which value needs to be committed.

In your case, you will need to compare both versions to determine which one is newer, and maybe if both versions happened to update the same property, you can handle that as well.

bool isUpdated = false; 

while (!isUpdated)
{
    try
    {
        _unitOfWork.Save();
        isUpdated = true;
    }
    catch (DbUpdateConcurrencyException exception)
    {
        foreach (DbEntityEntry entry in exception.Entries)
        {
            if (!(entry.CurrentValues.ToObject() is GameBank)) continue;
    
            var currentValues = entry.CurrentValues;

            var databaseValues = entry.GetDatabaseValues();

            foreach (var propertyName in currentValues.PropertyNames)
            {
                var currentValue = currentValues[propertyName];

                var databaseValue = databaseValues[propertyName];
                
                // TODO: handle the changes 
            }

            // Save Changes to the context
            entry.OriginalValues.SetValues(databaseValues);
        }
    }
    catch (Exception exception)
    {
        //TODO: Handle other exceptions
    }
}

The second part you need to change is to move all ProductCode, GameRequest, GameBank, and GameConfirmResponse codes into a service class. As they're linked to GameBank and should be keep grouped, otherwise you might lose some data integrity.

One senario that I can see from your code (which might be a potential bug) is that GameRequest and GameConfirmResponse are committed separately, which means, if GameBank didn't commit successfully, then your database will have GameRequest and possible GameConfirmResponse with invalid entries.

To solve this, you can save the changes into the context, then commit the changes using one transaction, like :

GameBank gameBankResult = _unitOfWork.GameBankRepository.GetGame(g =>
    g.productCode == requestDto.productCode && g.referenceId == Guid.Empty);

if(gameBankResult is null) {
    // TODO: Early Return .. there is nothing to process 
}

var gameRequest = _mapper.Map<RequestDto, GameRequest>(requestDto);

var gameBankConfirmResponse =
        _mapper.Map<GameBank, GameConfirmResponse>(gameBankResult);


_unitOfWork.GameBankRepository.Update(gameBankResult);
_unitOfWork.GameRepository.Insert(gameRequest);
_unitOfWork.GameConfirmResponseRepository.Insert(gameBankConfirmResponse);


bool isUpdated = false; 

while (!isUpdated)
{
    try
    {
        _unitOfWork.Save();
        isUpdated = true;
    }
    catch (DbUpdateConcurrencyException exception)
    {
        //TODO: Handle Concurrency
    }
    catch (Exception exception)
    {
        //TODO: Handle other exceptions
    }
}

this will wrap them into a one transaction, where it will rollback if there is an error, which will prevent from adding invalid entries. You can handle the rest of exceptions as well.