1
\$\begingroup\$

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.

\$\endgroup\$
4
  • 1
    \$\begingroup\$ Why did you write this in seven year old technology? Why not at least use .NET Framework 4.8? Or more importantly, why not in .NET 6 or 7? \$\endgroup\$
    – BCdotWEB
    Dec 12, 2023 at 13:35
  • \$\begingroup\$ I wrote it more than 3 years ago \$\endgroup\$
    – raysefo
    Dec 12, 2023 at 13:39
  • \$\begingroup\$ Can we focus on the pros and cons of this solution? @BCdotWEB \$\endgroup\$
    – raysefo
    Dec 12, 2023 at 15:15
  • \$\begingroup\$ I wonder if querying and then updating the same database in two separate applications at the same time, can cause side effects? One of them has this updated code and the other has the older one. This one is a legacy web API and the other one is an asp.net core web API. \$\endgroup\$
    – raysefo
    Dec 13, 2023 at 11:56

1 Answer 1

2
\$\begingroup\$

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.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.