0

I have a Web API that has a method that has been working for several months without any issues (PurchaseController). The list passed in was always around 100 or less. A few days ago, the volume spiked to around 600 items in the list which resulted in the app that passes the list getting a timeout instead of a success/failure response. It also seemed to make all other Web API methods run very slow until this method had finished running. So I modified the app code to break up the submissions into a configurable number of items at a time (currently set to 20 for the purpose of testing). I also modified the way the list was organized so that I could wrap each purchase in its own SQL transaction, in the hopes that this would allow other API methods to run without interruption.

However, once these changes were made, I ran into frequent errors like this when PurchaseController was called:

Transaction (Process ID 62) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.

The original PurchaseController code:

public class PurchaseController : ControllerBase
{
    /// <summary>
    /// 
    /// </summary>
    /// <param name="Purchases"></param>
    /// <returns></returns>
    public JsonResult<APIResponse> Put([FromBody]CPurchases Purchases)
    {
        if (!ValidateCredentials(Request, this.ServiceName))
        {
            throw new HttpResponseException(HttpStatusCode.Unauthorized);
        }

        APIResponse response = new APIResponse();

        try
        {
            using (var dbTransaction = Members.Database.BeginTransaction())
            {
                response = base.RecordEventRegistrations(Purchases.EventRegistrations);

                if (!response.IsSuccess)
                    throw new Exception(response.Error);

                response = base.RecordPublicationPurchases(Purchases.PublicationPurchases);

                if (!response.IsSuccess)
                    throw new Exception(response.Error);

                response = base.RecordSubscriptionPurchases(Purchases.SubscriptionPurchases);

                if (!response.IsSuccess)
                    throw new Exception(response.Error);

                response = base.RecordTransactions(Purchases.Transactions);

                if (!response.IsSuccess)
                    throw new Exception(response.Error);

                dbTransaction.Commit();
            }
        }
        catch (Exception ex)
        {
            Utilities.Logging.LogToEventLog(ex);
            response.IsSuccess = false;
            response.Error = Utilities.Logging.GetInnermostException(ex);
        }

        return Json(response);
    }
}

The new code that ran into deadlocks on virtually every call:

public class PurchaseController : ControllerBase
{
    /// <summary>
    /// </summary>
    /// <param name="Purchases"></param>
    /// <returns></returns>
    public JsonResult<APIResponse> Put([FromBody]List<PurchaseDetails> Purchases)
    {
        if (!ValidateCredentials(Request, this.ServiceName))
        {
            throw new HttpResponseException(HttpStatusCode.Unauthorized);
        }

        APIResponse response = new APIResponse();

        try
        {
            using (var dbTransaction = Members.Database.BeginTransaction())
            {
                foreach (var Purchase in Purchases)
                {
                    response = base.RecordEventRegistrations(Purchase.EventRegistrations);

                    if (!response.IsSuccess)
                        throw new Exception(response.Error);

                    response = base.RecordPublicationPurchases(Purchase.PublicationPurchases);

                    if (!response.IsSuccess)
                        throw new Exception(response.Error);

                    response = base.RecordSubscriptionPurchases(Purchase.SubscriptionPurchases);

                    if (!response.IsSuccess)
                        throw new Exception(response.Error);

                    response = base.RecordTransactions(Purchase.PaymentTransactions);

                    if (!response.IsSuccess)
                        throw new Exception(response.Error);

                    dbTransaction.Commit();
                }
            }
        }
        catch (Exception ex)
        {
            Utilities.Logging.LogToEventLog(ex);
            response.IsSuccess = false;
            response.Error = Utilities.Logging.GetInnermostException(ex);
        }

        return Json(response);
    }
}

I later moved the SQL transaction outside the foreach like this:

foreach (var Purchase in Purchases)
{
    using (var dbTransaction = Members.Database.BeginTransaction())
    {

and this made a huge difference in the frequency of deadlocks, but did not eliminate them. I also added System.Data.IsolationLevel.RepeatableRead to the SQL transaction but it seemed to make no difference.

The code calling this method changed from:

public static APIResponse SavePurchases(IEnumerable<EventRegistration> Registrations, IEnumerable<PublicationPurchase> PublicationPurchases, IEnumerable<SubscriptionPurchase> SubscriptionPurchases, IEnumerable<PaymentTransactionModel> Transactions)
{
    if (Registrations.Count() == 0 && PublicationPurchases.Count() == 0 && SubscriptionPurchases.Count() == 0 && Transactions.Count() == 0)
        return new APIResponse() { IsSuccess = true };

    CPurchases Purchases = new CPurchases()
        {
            EventRegistrations = Registrations,
            PublicationPurchases = PublicationPurchases,
            SubscriptionPurchases = SubscriptionPurchases,
            Transactions = Transactions
        };

    return PutAPI("/Purchase", Purchases);
}

to

public static APIResponse SavePurchases(IEnumerable<PurchaseDetails> Purchases)
{
    int PageSize = AppSettings.MaxNumberOfPurchasesPerAPIBatch;

    APIResponse Response = new APIResponse() { IsSuccess = true };

    List<PurchaseDetails> Page;

    for (int i = 0; i < Purchases.Count(); i = i + PageSize)
    {
        Page = Purchases.Skip(i).Take(PageSize).ToList();

        if (Page.Any())
        {
            Response = PutAPI("/Purchase", Page);

            if (!Response.IsSuccess)
                break;
        }
    }

    return Response;
}

In addition, it is worth noting that the table the RecordEventRegistrations modifies has a single trigger to enforce that the varchar event number is in all caps. This was put in place when the original code went live. And all database changes are done using Entity Framework 6.

No other code changes to the database, the app, or the Web API have been made in the past 2 weeks, so I'm pretty sure the problem is due to these code changes but I can't seem to figure out what or why.

UPDATE: I've been trying to get the deadlock graph for several hours but my account didn't have the correct permissions. Now that I have it I believe the problem is in the trigger, if I'm reading the graph properly. I've temporarily disabled the trigger and it seems to have resolved the deadlocks. But since the trigger has been unchanged for the past 6+ months, I'm guessing it has something to do with the volume of insert/updates over the last couple of days?

xml_report  
<deadlock>
    <victim-list>
        <victimProcess id="process23368b65468"/>
    </victim-list>
    <process-list>
        <process id="process23368b65468" taskpriority="0" logused="248" waitresource="PAGE: 6:1:84432 " waittime="3847" ownerId="22806353" transactionname="user_transaction" lasttranstarted="2020-12-30T10:33:09.433" XDES="0x2360aa86408" lockMode="U" schedulerid="4" kpid="3916" status="suspended" spid="54" sbid="2" ecid="0" priority="0" trancount="2" lastbatchstarted="2020-12-30T10:33:09.437" lastbatchcompleted="2020-12-30T10:33:09.420" lastattention="1900-01-01T00:00:00.420" clientapp="EntityFramework" hostname="***-API-01" hostpid="1400" loginname="sa" isolationlevel="read committed (2)" xactid="22806353" currentdb="6" lockTimeout="4294967295" clientoption1="671219744" clientoption2="128056">
            <executionStack>
                <frame procname="members.dbo.tr_event_registration" line="8" stmtstart="332" stmtend="584" sqlhandle="0x03000600e3d0825d1a48970094ac000000000000000000000000000000000000000000000000000000000000">  UPDATE [Event_Registration]  SET event_number = UPPER(event_number)  WHERE  event_number in (select event_number from inserted    </frame>
                <frame procname="adhoc" line="1" stmtstart="156" stmtend="506" sqlhandle="0x0200000015ef0508db49eeb79e583b351130d3afdb0ebac40000000000000000000000000000000000000000">  unknown    </frame>
                <frame procname="unknown" line="1" sqlhandle="0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000">  unknown    </frame>
            </executionStack>
            <inputbuf>  (@0 datetime2(7),@1 char(2),@2 datetime2(7),@3 datetime2(7),@4 int,@5 char(5))UPDATE [dbo].[Event_Registration]  SET [registration_date] = @0, [source] = @1, [payment_date] = @2, [time_stamp] = @3  WHERE (([entity_number] = @4) AND ([event_number] = @5))     </inputbuf>
        </process>
        <process id="process233a1618ca8" taskpriority="0" logused="328" waitresource="PAGE: 6:1:1848653 " waittime="3824" ownerId="22806350" transactionname="UPDATE" lasttranstarted="2020-12-30T10:33:09.383" XDES="0x2360db58e58" lockMode="U" schedulerid="1" kpid="6096" status="suspended" spid="90" sbid="0" ecid="0" priority="0" trancount="2" lastbatchstarted="2020-12-30T10:33:09.380" lastbatchcompleted="2020-12-30T10:33:09.393" lastattention="1900-01-01T00:00:00.393" clientapp=".Net SqlClient Data Provider" hostname="***-MBR-01" hostpid="3936" loginname="sa" isolationlevel="read committed (2)" xactid="22806350" currentdb="6" lockTimeout="4294967295" clientoption1="671219744" clientoption2="128056">
            <executionStack>
                <frame procname="members.dbo.tr_event_registration" line="8" stmtstart="332" stmtend="584" sqlhandle="0x03000600e3d0825d1a48970094ac000000000000000000000000000000000000000000000000000000000000">  UPDATE [Event_Registration]  SET event_number = UPPER(event_number)  WHERE  event_number in (select event_number from inserted    </frame>
                <frame procname="adhoc" line="1" stmtstart="714" stmtend="1696" sqlhandle="0x02000000f367b4079d5a8152b57beb111052747ccde62e3d0000000000000000000000000000000000000000">  unknown    </frame>
                <frame procname="unknown" line="1" sqlhandle="0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000">  unknown    </frame>
            </executionStack>
            <inputbuf>  (@is_canceled bit,@source nvarchar(1),@event_number nvarchar(5),@entity_number int,@registration_date datetime,@cancel_date nvarchar(4000),@fee_code nvarchar(2),@amount_paid float,@pay_status nvarchar(1),@show_up nvarchar(1),@payment_date nvarchar(10),@payment_method nvarchar(6),@hours_override_reason nvarchar(4000),@quantity int,@user_stamp nvarchar(11))update event_registration set registration_date = @registration_date, cancel_date = @cancel_date, is_canceled = @is_canceled, fee_code = @fee_code, amount_paid = @amount_paid, pay_status = @pay_status, show_up = @show_up, payment_date = @payment_date, payment_method = @payment_method, hours_override_reason = @hours_override_reason, quantity = @quantity, user_stamp = @user_stamp, source = @source, time_stamp = getdate() where event_number = @event_number and entity_number = @entity_number; IF @@ROWCOUNT = 0 BEGIN insert into event_registration (event_number, entity_number, registration_date, cancel_date, is_canceled, fee_code, amount_paid, pay_status, show_up   </inputbuf>
        </process>
    </process-list>
    <resource-list>
        <pagelock fileid="1" pageid="84432" dbid="6" subresource="FULL" objectname="members.dbo.Event_Registration" id="lock233a5ad4d00" mode="U" associatedObjectId="72057594463256576">
            <owner-list>
                <owner id="process233a1618ca8" mode="U"/>
            </owner-list>
            <waiter-list>
                <waiter id="process23368b65468" mode="U" requestType="wait"/>
            </waiter-list>
        </pagelock>
        <pagelock fileid="1" pageid="1848653" dbid="6" subresource="FULL" objectname="members.dbo.Event_Registration" id="lock233a5858d00" mode="IX" associatedObjectId="72057594463256576">
            <owner-list>
                <owner id="process23368b65468" mode="IX"/>
            </owner-list>
            <waiter-list>
                <waiter id="process233a1618ca8" mode="U" requestType="wait"/>
            </waiter-list>
        </pagelock>
    </resource-list>
</deadlock>

The trigger is:

ALTER Trigger [dbo].[tr_event_registration]
on [dbo].[Event_Registration]
AFTER insert, update
as
if ((trigger_nestlevel() > 1) or (@@rowcount = 0))
    return
UPDATE [Event_Registration]
SET event_number = UPPER(event_number)
WHERE
event_number in (select event_number from inserted);
Aaron S
  • 25
  • 6
  • You database may be badly fragmented. See : https://learn.microsoft.com/en-us/troubleshoot/sql/admin/defragmenting-database-disk-drives – jdweng Dec 30 '20 at 15:29
  • 1
    Can you share with us the Info of Deadlock Err logged in SQL Error Log ? so that we will get a chance to investigate and overcome the situation – SQLServerBuddy Dec 30 '20 at 15:36
  • 2
    Adding to the comment by @SQLServerBuddy, extract a sample deadlock graph xml using [this query](https://stackoverflow.com/questions/65072228/deadlock-graph-from-extended-events-not-showing/65073177#65073177) and add it to your question. Deadlocks are often due to queries in need of query/index tuning so I suggest you take a closer look at the plans, especially the trigger quereis. – Dan Guzman Dec 30 '20 at 15:43

1 Answers1

1

Replace the trigger with a CHECK constraint. Updating a key value every time after inserting a row is wasteful, and is the cause of excessive locking.

primary key which is a composite made up of entity_number ( int ) and event_number ( char(5) )

Then your trigger requires a scan and will update all the rows for the entity_number on every insert and update.

Change it to conditionally update only the rows affected by the triggering statement. Something like:

ALTER Trigger [dbo].[tr_event_registration]
on [dbo].[Event_Registration]
AFTER insert, update
as
begin
   if ((trigger_nestlevel() > 1) or (@@rowcount = 0))
      return;

   
   
   UPDATE [Event_Registration] e
   SET event_number = UPPER(event_number)
   WHERE event_number != upper(event_number) COLLATE Latin1_General_CS_AS
   and EXISTS 
   (
     select * 
     from inserted i
     where i.event_number = e.event_number
       and i.entity_number = e.entity_number
   );
   
end

Will these changes prevent or just reduce the likelihood of a deadlock?

Probably prevent. Your deadlock included page locks, probably because the trigger was scanning and updating additional rows. So any two sessions touching the same event_number would likely cause a deadlock.

David Browne - Microsoft
  • 80,331
  • 6
  • 39
  • 67
  • We considered this approach but a check constraint will unfortunately break multiple applications if/when they attempt to store a lowercase value. The uppercase/lowercase wasn't even an issue until we started using Entity Framework, which doesn't treat uppercase as equal to lowercase in varchar key fields. – Aaron S Dec 31 '20 at 15:50
  • What is the table and index DDL? Make sure your trigger is looking up rows by the clustered index key. But updating the clustered index key is a bad idea. So perhaps you can check the values in the trigger, and only update them if they need to be changed. – David Browne - Microsoft Dec 31 '20 at 16:18
  • I'm not sure I understand what you're asking for. The table consists of about 25 fields and the only index is the primary key which is a composite made up of entity_number ( int ) and event_number ( char(5) ). I believe I understand that your advice is to modify the WHERE to include the entity_number as well so that it looks up by the entire PK and to modify the SET to only update if lowercase letters are present, correct? Will these changes prevent or just reduce the likelihood of a deadlock? – Aaron S Jan 04 '21 at 15:20
  • 1
    Restricting the update by entity_number and event_number was an obvious oversight. I feel silly not catching that. But checking to see if the event_number is already uppercase is a great idea that I didn't know I could do that way. Thanks for the complete example. – Aaron S Jan 04 '21 at 19:52