Trigger deadlock testing with PowerShell

Call me crazy but triggers are usually bad news. In my experience they tend to rise to the top of the queue for bug fixing more frequently than their relatively small SLOC would belie, and often have performance problems that don't become apparent until months or years down the track as record counts increase.

But recently I was asked to create one and learned a thing or two about the importance of testing them thoroughly. Here's a sample database and table.

If Not Exists (Select Top 1 0 From sys.databases Where name = 'TriggerTest')
	Create Database TriggerTest
Go

Use TriggerTest
Go

If Object_Id('dbo.Invoice') Is Not Null
	Drop Table dbo.Invoice
Go

Create Table Invoice (
	InvoiceID Int Identity(1,1) Primary Key,
	InvoiceNumber Nvarchar(12)
)
Go

Create Index Invoice_InvoiceNumber On dbo.Invoice (
	InvoiceNumber
)
Go

The situation was straightforward and one that may be all too familiar:

An order-entry application accepted invoice numbers that were keyed in manually by users.
Things changed (!) and the application would now also be hosted online, requiring it to generate its own invoice numbers.
The business did not want to change the web services to do this, for their own reasons.
But the web GUI could ensure a placeholder invoice number ("WEB") was used for online orders.
The trigger's job is to alter this to "WEB000000001″, and rise roughly sequentially. If any numbers are missed it's not the end of the world.
It had to run on SQL Server 2008.

I may have gone a little overboard on my first attempt. I'm only a little embarrassed.

Attempt 1

If Exists (Select Top 1 0 From sys.triggers Where name = 'Invoice_InvoiceNumberWEB')
	Drop Trigger dbo.Invoice_InvoiceNumberWEB
Go

Create Trigger dbo.Invoice_InvoiceNumberWEB On Invoice
After Insert
As
Begin
	If Exists (Select Top 1 0 From Inserted Where InvoiceNumber = 'WEB')
	Begin
		Set Nocount On

		-- Get the last WEB integer, null is acceptable
		Declare @LastInvoiceNumber Int
		Select	@LastInvoiceNumber = Right(Max(InvoiceNumber), 9)
		From	Invoice
		Where	InvoiceNumber Like 'WEB%'
		And		Len(InvoiceNumber) = 12
		And		Right(InvoiceNumber, 9) Not Like '%[^0-9]%'

		-- Print @LastInvoiceNumber
		-- Calculate an incremental integer for each WEB row and apply the change
		; With Cte As (
			Select	Inserted.InvoiceID,
					Row_Number() Over (Order By Inserted.InvoiceID) As RowNumber
			From	Inserted
			Where	InvoiceNumber = 'WEB'
		)
		Update	Invoice
		Set		InvoiceNumber	= 'WEB' + Right(Replace(Space(9), ' ', '0') + Cast(Coalesce(@LastInvoiceNumber, 0) + Cte.RowNumber As NvarChar(Max)), 9)
		From	Invoice
		Join	Cte
		On		Invoice.InvoiceID = Cte.InvoiceID
	End
End
Go

It looked okay to me. It handled multiple rows, and if there's a WEB invoice number passed in it works out the last WEB invoice number in the table and increments it for each row being added. What could possibly go wrong? Here were some statements I used each time for testing:

Insert	Invoice (InvoiceNumber) Values ('ABCDEFGHIJKL') -- Traditional record
Insert	Invoice (InvoiceNumber) Values ('WEB000000001') -- Our own number to throw things off
Insert	Invoice (InvoiceNumber) Values ('WEB') -- Assign a number, it should be 2
Insert	Invoice (InvoiceNumber) Values ('WEB00000000A') -- A test to make sure letters are ignored
Insert	Invoice (InvoiceNumber) Values ('WEB') -- Assign a number, it should be 3

Select	InvoiceID,
		InvoiceNumber
From	Invoice

And the basic results, as I expected:

InvoiceID	InvoiceNumber
1	ABCDEFGHIJKL
2	WEB000000001
3	WEB000000002
4	WEB00000000A
5	WEB000000003

Everything seemed perfect but I was aware that this application would see a bit more load than a single set of statements executed sequentially. So I created a PowerShell script to test a heavily loaded scenario.

If you're familiar with PowerShell there is a cmdlet called Start-Job you can use to create asynchronously running processes. Unfortunately it's very, very slow and takes up a lot of memory so it not very good for this kind of concurrency testing. Instead I used the psasync cmdlet and its worker threads. (I want to fork that to use standard semantics like Start-Job2 down the track).

function Test-Invoice_InvoiceNumber {
    Cls

    Import-Module psasync
    Import-Module SQLPS -DisableNameChecking

    # Clean up the table
    Invoke-Sqlcmd -ServerInstance . -Database TriggerTest -Query "Truncate Table Invoice; Dbcc Checkident('dbo.Invoice', Reseed, 1)"

    $maxThreads = 25
    $asyncPipelines = @()
    $asyncPool = Get-RunspacePool $maxThreads

    for ($i = 1; $i -le $maxThreads; $i++) {
        # Create a thread
        $asyncPipelines += Invoke-Async -RunspacePool $asyncPool -ScriptBlock {

            try {
                # Connect to the server and add a WEB entry
                $con = New-Object System.Data.SqlClient.SqlConnection("Data Source=.;Database=TriggerTest;Integrated Security=True;")
                $con.Open()
                $cmd = $con.CreateCommand()
                $cmd.CommandText = "BEGIN TRAN Insert dbo.Invoice (InvoiceNumber) Values ('WEB') COMMIT"
                $cmd.ExecuteNonQuery() | Out-Null
                "Success"
            } catch {
                try {
                    # Try to just show the base message
                    $_.Exception.InnerException.Message
                } catch {
                    $_
                }
            }
            if ($cmd -ne $null) { $cmd.Dispose() }
            if ($con -ne $null) { $con.Close() }
        }
    }

    do {
        # Display a running tally
        $jobCountRunning = (Receive-AsyncStatus -Pipelines $asyncPipelines | Where-Object { $_.Status -eq "Running" }).Count
        $jobCountCompleted = (Receive-AsyncStatus -Pipelines $asyncPipelines | Where-Object { $_.Status -eq "Completed" }).Count

        Write-Progress -Id 1 -Activity "Waiting To Finish" `
                    -Status "Running Jobs" `
                    -CurrentOperation "$jobCountCompleted jobs completed, $jobCountRunning jobs left to go" `
                    -PercentComplete ($jobCountCompleted / $asyncPipelines.Count * 100)

        Start-Sleep -Milliseconds 500
    } while ($jobCountRunning -ne 0)

    # Finish up
    $results = @()
    $results = Receive-AsyncResults -Pipelines $asyncPipelines
    $asyncPool.Close()

    Write-Host "These are the results:"
    $results | %{ "`t$_" }
    Write-Host
    Write-Host "Successful inserts: " ($results -eq "Success" | Measure-Object).Count
    Write-Host "Failed inserts: " ($maxThreads - ($results -eq "Success" | Measure-Object).Count)
}

Test-Invoice_InvoiceNumber

This was the output.

These are the results:
Success
Success
Success
Success
Transaction (Process ID 60) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 65) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 74) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 62) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 58) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 64) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 67) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 69) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 71) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 73) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Success
Transaction (Process ID 70) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 68) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 66) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 59) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 61) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 79) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 88) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 86) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 63) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 57) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.

Successful jobs: 5
Failed jobs: 20

Shock! Horror! Deadlocks that would have caused web services to have their connections killed immediately, a career-ending problem if it made it to production. Funnily (not important, but interesting) it also threw off identity values in the table in the process:

InvoiceID	InvoiceNumber
1	WEB000000001
2	WEB000000002
3	WEB000000003
4	WEB000000004
15	WEB000000005

I don't normally encounter deadlocks but I suspected it was caused by the trigger trying to work out the most recent sequence number. I had an idea to get around it but first I wanted to see if it was only deadlocking because I was using the same table I was inserting into. Maybe if I put the sequential number into a table of its own:

Attempt 2

If Object_Id('dbo.InvoiceNumberHolder') Is Not Null
	Drop Table dbo.InvoiceNumberHolder
Go

Create Table dbo.InvoiceNumberHolder (
	InvoiceNumberHolderId Int Identity (1,1),
	LastInvoiceNumber Decimal(9, 0) Not Null
)
Go

Insert dbo.InvoiceNumberHolder (LastInvoiceNumber) Values (0)
Go

If Exists (Select Top 1 0 From sys.triggers Where name = 'Invoice_InvoiceNumberWEB')
	Drop Trigger dbo.Invoice_InvoiceNumberWEB
Go

Create Trigger dbo.Invoice_InvoiceNumberWEB On Invoice
After Insert
As
Begin
	If Exists (Select Top 1 0 From Inserted Where InvoiceNumber = 'WEB')
	Begin
		Set Nocount On

		-- Get the last WEB integer, null is acceptable
		Declare @LastInvoiceNumber Int = (Select LastInvoiceNumber From dbo.InvoiceNumberHolder)

		-- Print @LastInvoiceNumber
		-- Calculate an incremental integer for each WEB row and apply the fix
		; With Cte As (
			Select	Inserted.InvoiceID,
					Row_Number() Over (Order By Inserted.InvoiceID) As RowNumber
			From	Inserted
			Where	InvoiceNumber = 'WEB'
		)
		Update	Invoice
		Set		InvoiceNumber	= 'WEB' + Right(Replace(Space(9), ' ', '0') + Cast(Coalesce(@LastInvoiceNumber, 0) + Cte.RowNumber As NvarChar(Max)), 9)
		From	Invoice
		Join	Cte
		On		Invoice.InvoiceID = Cte.InvoiceID

		Update	InvoiceNumberHolder
		Set		LastInvoiceNumber = Coalesce(LastInvoiceNumber, 0) + @@rowcount
	End
End
Go

Having done this I confirmed with my basic test:

InvoiceID	InvoiceNumber
1	ABCDEFGHIJKL
2	WEB000000001
3	WEB000000001
4	WEB00000000A
5	WEB000000002

Note that we had two WEB000000001, one that was manually inserted and the system didn't know about, and one that was generated. This was acceptable to the business (because users would not normally manually enter invoice numbers with that prefix). But at least we know. And under load?

These are the results:
Success
Success
Success
Success
Success
Success
Transaction (Process ID 62) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Success
Success
Transaction (Process ID 60) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 63) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 64) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 65) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 66) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 67) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 68) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 69) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 70) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 71) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 72) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 73) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 74) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 75) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 76) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Transaction (Process ID 77) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.

Successful inserts: 8
Failed inserts: 17

Surprise (or not if you know locking intimately), deadlocks!

InvoiceID	InvoiceNumber
1	WEB000000003
2	WEB000000004
3	WEB000000005
4	WEB000000006
5	WEB000000007
6	WEB000000008
7	WEB000000009
10	WEB000000010

Then I had a better idea. How about using SQL Server soft locks? So that critical portion of the trigger can only run one at a time?

Attempt 3

If Exists (Select Top 1 0 From sys.triggers Where name = 'Invoice_InvoiceNumberWEB')
	Drop Trigger dbo.Invoice_InvoiceNumberWEB
Go

Create Trigger dbo.Invoice_InvoiceNumberWEB On Invoice
After Insert
As
Begin
	Exec	sp_GetAppLock @Resource='Invoice_InvoiceNumberWEB', @LockMode='Exclusive', @LockTimeout=10000

	If Exists (Select Top 1 0 From Inserted Where InvoiceNumber = 'WEB')
	Begin
		Set Nocount On

		-- Get the last WEB integer, null is acceptable
		Declare @LastInvoiceNumber Int
		Select	@LastInvoiceNumber = Right(Max(InvoiceNumber), 9)
		From	Invoice
		Where	InvoiceNumber Like 'WEB%'
		And		Len(InvoiceNumber) = 12
		And		Right(InvoiceNumber, 9) Not Like '%[^0-9]%'

		-- Print @LastInvoiceNumber
		-- Calculate an incremental integer for each WEB row and apply the fix
		; With Cte As (
			Select	INSERTED.InvoiceID,
					Row_Number() Over (Order By Inserted.InvoiceID) As RowNumber
			From	Inserted
			Where	InvoiceNumber = 'WEB'
		)
		Update	Invoice
		Set		InvoiceNumber	= 'WEB' + Right(Replace(Space(9), ' ', '0') + Cast(Coalesce(@LastInvoiceNumber, 0) + Cte.RowNumber As NvarChar(Max)), 9)
		From	Invoice
		Join	Cte
		On		Invoice.InvoiceID = Cte.InvoiceID
	End

	Exec	sp_ReleaseAppLock @Resource='Invoice_InvoiceNumberWEB'
End
Go

I won't paste the results, but the results were still deadlocks! Now I was dusting the chalk off my hands and getting serious. I would rewrite the trigger into a more compact statement and try using an SQL Server SEQUENCE datatype. (In retrospect I don't need the Row_Number section, but I was just hacking the above script quickly to try it out.)

Sequence Attempt

If Exists (Select Top 1 0 From sys.sequences Where schema_id = schema_id('dbo') And name = 'InvoiceNumber')
	Drop Sequence dbo.InvoiceNumber
Go

Create Sequence dbo.InvoiceNumber As Int Start With 1 Increment By 1
Go

If Exists (Select Top 1 0 From sys.triggers Where name = 'Invoice_InvoiceNumberWEB')
	Drop Trigger dbo.Invoice_InvoiceNumberWEB
Go

Create Trigger dbo.Invoice_InvoiceNumberWEB On Invoice
After Insert
As
Begin
	Set Nocount On

	; With InsertedRow As (
		Select	Inserted.InvoiceID,
				Row_Number() Over (Order By Inserted.InvoiceID) As RowNumber
		From	Inserted
		Where	InvoiceNumber = 'WEB'
	)
	Update	Invoice
	Set		InvoiceNumber = 'WEB' + Right(Replace(Space(9), ' ', '0') + Cast(Next Value For dbo.InvoiceNumber As NvarChar(Max)), 9)
	From	Invoice
	Join	InsertedRow
	On		Invoice.InvoiceID = InsertedRow.InvoiceID
End
Go

These are the results:
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success
	Success

Successful inserts:  25
Failed inserts:  0

Success! All the InvoiceIDs were sequential, too. It all seemed great. I mean for SQL Server 2012, and this had to work on SQL Server 2008. I had almost wasted my time except that it led me to Google search where people were talking about the lack of SEQUENCE in SQL Server 2008 and using an identity table instead.

I didn't really understand how that would work, I already had an identity column in my table and that wasn't going to be suitable for my sequence number. There were no code samples but I decided they might be talking about having a dedicated table, and so set out to create my last trigger using everything I had learned so far:

Final Attempt

If Object_Id('dbo.InvoiceNumberWEB') Is Not Null
	Drop Table dbo.InvoiceNumberWEB
Go

If Not Exists (Select Top 1 0 From sys.tables Where name = 'InvoiceNumberWEB')
Begin
	Create Table dbo.InvoiceNumberWEB (
		InvoiceSequence Int Identity(1, 1) Primary Key,
		InvoiceID Int Not Null
	)
End
Go

If Exists (Select Top 1 0 From sys.triggers Where name = 'Invoice_InvoiceNumberWEB')
	Drop Trigger dbo.Invoice_InvoiceNumberWEB
Go

Create Trigger dbo.Invoice_InvoiceNumberWEB On dbo.Invoice
After Insert
As
Begin
	Set Nocount On

	Declare @Id Table (
		InvoiceSequence Int,
		InvoiceID Int
	)

	Insert	dbo.InvoiceNumberWEB
	Output	Inserted.InvoiceSequence, Inserted.InvoiceID
	Into	@Id
	Select	Trigger_Inserted.InvoiceID
	From	Inserted Trigger_Inserted
	Where	Trigger_Inserted.InvoiceNumber = 'WEB'

	Update	Invoice
	Set		InvoiceNumber = 'WEB' + Right(Replace(Space(9), ' ', '0') + Cast(Coalesce(Id.InvoiceSequence, 0) As NvarChar(Max)), 9)
	From	Invoice
	Join	@Id Id
	On		Invoice.InvoiceID = Id.InvoiceID
End
Go

Success! It worked like a real SEQUENCE and with no more deadlocks. It does, occasionally, skip numbers if you insert a transaction and rollback, but this was acceptable.

You can see the part where I insert the existing InvoiceID into my sequence table (dbo.InvoiceNumberWEB). I didn't want to do that and just wanted to have SequenceNumber, but it turns out you can't insert that way while using the Output statement and getting your identity columns back. So, it was required. No matter.

Conclusion

What I learned, aside from having a blind spot in understanding internal SQL Server locking (I'll get to it one day) is that you really need to test your triggers under load.

I know some people will immediately reject this entire post and say a trigger just shouldn't be used at all. That's arguable but businesses want what they want and someone has to provide it. As long as it is tested properly and works in production I don't feel particularly strongly about it.