June 12, 2017 at 4:00 pm
To setup the question, please start with this table:
CREATE TABLE [dbo].[Test1](
[identityFld] [int] IDENTITY(1,1) NOT NULL,
[account] [varchar](8) NULL,
[createdUTC] [datetime] NULL,
PRIMARY KEY CLUSTERED ([identityFld] ASC))
GO
CREATE UNIQUE NONCLUSTERED INDEX [Test1_idx2] ON [dbo].[Test1]
(
[account] ASC
)
GO
We have a procedure that can be called by external users to submit transactions. [account] should be a unique value (in Test1, but can have multiple transactions submitted), but the user can submit new values for [account] that do not exist in our data (Test1) yet. If the transaction data includes an [account] that does not yet exist in the TEST1, we need to insert it. (This is a simplified version of the data, and there is a good reason that [account] is not the primary key). The challenge we are having is that external users can call our stored procedure at virtually the exact same time, with different transaction rows, but that have the same [account] field. Attempting to insert new [account] values in race conditions is resulting either in key violations or deadlocks, depending on the approach.
First, we tried this insert, with a subquery to test for existence before doing the insert. We tested by setting up two windows to each call this with the same account value at exactly the same time. This resulted in deadlocks intermittently.
DECLARE
@account varchar(8)
SET @account = 'abc123'
-- set two windows with identical code, each firing off at same system time
WHILE getDate() < '2017-06-12 13:50:00'
CONTINUE
INSERT INTO Test1
(account, createdUTC)
SELECT @account, getUTCDate()
WHERE NOT EXISTS (
SELECT 1
FROM Test1
WHERE account = @account)
Adding a HOLDLOCK worked in all of our tests, and never resulted in deadlock. However we are unsure if that is 100% or we were just lucky.
INSERT INTO Test1
(account, createdUTC)
SELECT @account, getUTCDate()
WHERE NOT EXISTS (
SELECT 1
FROM Test1 WITH (HOLDLOCK)
WHERE account = @account)
Setting the isolation level also never failed. However, I don't have enough experience with this to be sure this is a good approach.
SET TRANSACTION ISOLATION LEVEL SERIALIZABLE
INSERT INTO Test1
(account, createdUTC)
SELECT @account, getUTCDate()
WHERE NOT EXISTS (
SELECT 1
FROM Test1
WHERE account = @account)
So, we have 2 solutions that seem to work, but we could use advice on what is best practice in this scenario for concurrency while limiting potential for deadlocks. Thanks in advance!
June 12, 2017 at 4:30 pm
I think you're making it too difficult... Just create an INSERT proc and a separate UPDATE proc. Then use a 3rd "control flow" proc to check & see on the account already exists and execute the appropriate procedure based on the results.
June 12, 2017 at 5:08 pm
Jason A. Long - Monday, June 12, 2017 4:30 PMI think you're making it too difficult... Just create an INSERT proc and a separate UPDATE proc. Then use a 3rd "control flow" proc to check & see on the account already exists and execute the appropriate procedure based on the results.
Thanks for your response. It seems if 2 users called the "control flow" proc at exactly the same time, and those 2 executions tested for existence at exactly the same time, I would have the same potential for race condition? I also should have added, the table Test1 can be considered more like a event log itself. When just kicking around options (whether good or not), we considered options like the MERGE statement, or a delete/insert wrapped in an explicit transaction, but those aren't appropriate. We also have solutions we know would work, for instance having a separate task check for new accounts in the transaction data. Perhaps we are making it too difficult - but to be honest the question is posted partly out of curiosity / continuing education because it seems there SHOULD be a straightforward way to do this with a few well constructed lines of code!
June 13, 2017 at 4:02 am
rice.tx - Monday, June 12, 2017 5:08 PMJason A. Long - Monday, June 12, 2017 4:30 PMI think you're making it too difficult... Just create an INSERT proc and a separate UPDATE proc. Then use a 3rd "control flow" proc to check & see on the account already exists and execute the appropriate procedure based on the results.Thanks for your response. It seems if 2 users called the "control flow" proc at exactly the same time, and those 2 executions tested for existence at exactly the same time, I would have the same potential for race condition?
Yes, you would.
Have a look at this article: http://source.entelect.co.za/why-is-this-upsert-code-broken
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
June 13, 2017 at 6:07 am
GilaMonster - Tuesday, June 13, 2017 4:02 AMrice.tx - Monday, June 12, 2017 5:08 PMJason A. Long - Monday, June 12, 2017 4:30 PMI think you're making it too difficult... Just create an INSERT proc and a separate UPDATE proc. Then use a 3rd "control flow" proc to check & see on the account already exists and execute the appropriate procedure based on the results.Thanks for your response. It seems if 2 users called the "control flow" proc at exactly the same time, and those 2 executions tested for existence at exactly the same time, I would have the same potential for race condition?
Yes, you would.
Have a look at this article: http://source.entelect.co.za/why-is-this-upsert-code-broken
Well, that was a perfect answer! Thank you.
June 16, 2017 at 11:12 am
Postscript. We wanted to explore using the MERGE operation to solve the above problem. We tested using similar scripts that GilaMonster demonstrated in his article. After many, many tests, we never found an example that the MERGE operation failed the concurrency test. However, when we deployed that into our production app, we very quickly experienced a duplicate row, indicating a concurrency problem. I wish we had an answer for why we can't reproduce that, but we have to move on. We switched to using the approach in GilaMonster arcticle, and no further problems have occurred. Just wanted to add our experience with the MERGE to this ticket.
Viewing 6 posts - 1 through 5 (of 5 total)
You must be logged in to reply to this topic. Login to reply