Duplicate key violation with composite primary key on high frequency insert

Question

I have a table as follows:

CREATE TABLE [Alg].[Sequence](
    [Id] [int] IDENTITY(1,1) NOT NULL,
    [SequenceId] [int] NOT NULL,
    [CustomerId] [bigint] NOT NULL,
    [Data] [text] NULL,
 CONSTRAINT [PK_Sequence] PRIMARY KEY CLUSTERED 
(
    [SequenceId] ASC,
    [CustomerId] ASC
)

And this is a block from insert/update trigger of another table where I insert data to Sequence table:

--insert data into sequence table
SELECT @MaxSeqId =  ISNULL(MAX(SequenceId),0)
FROM Alg.[Sequence] WITH (ROWLOCK)
WHERE CustomerId = @CustomerId

INSERT INTO Alg.[Sequence]
VALUES (
@MaxSeqId + 1
,@CustomerId
,@SendingData
,GETDATE()
)

So whenever high frequency insertion processes (on the table with trigger), violation of duplicate key error occurs. I tried ROWLOCK, but didn't work. How can I prevent this from happenning?

Update

I have been asked why I am not using built-in sequence, I tried but couldn't find how to use sequence with composite primary key. I don't want the SequenceId column be identity, in fact, I would like to keep SequenceId as identity for each CustomerId. You can see my another question related with this

Why would you want to create your own Sequence table? It's recommended to use the Sequence Objects. Find more info here: https://learn.microsoft.com/en-us/sql/t-sql/statements/create-sequence-transact-sql — t16n, Sep 13 '17 at 15:04
Doesn't `IDENTITY(1,1)` do the incrementing for you, therefore you don't need to insert it? Asking for a friend. — justiceorjustus, Sep 13 '17 at 15:09
You absolutely can and should use a sequence here. I don't pretend to know what your objective is (nor do I understand your objection) but you are in fact recreating a broken version of a sequence. If you can (almost) do it with the code you wrote then you can do it properly with a sequence. A sequence is just another way to get a unique integer. That is all. You will still get a usable sequence number for each customer, it just happens that it will also be unique. — LoztInSpace, Sep 14 '17 at 06:44

David Browne - Microsoft · Accepted Answer · 2017-09-13T17:56:23.810

3

This is a poor design. You should really use sequences. Really.

One of the reasons this is a poor design is that it's easy to get bugs like the one you have. Here's how to make that code actually work:

begin transaction

SELECT @MaxSeqId =  ISNULL(MAX(SequenceId),0)
FROM Alg.[Sequence] WITH (ROWLOCK, UPDLOCK, SERIALIZABLE)
WHERE CustomerId = @CustomerId

INSERT INTO Alg.[Sequence](SequenceId, CustomerId, Data)
VALUES (
@MaxSeqId + 1
,@CustomerId
,@SendingData
)

commit transaction

UPDLOCK instructs SQL Server to place a restrictive lock on the rows read, and to ignore the version store. SERIALIZABLE instructs SQL to use range locking, so that even if no row is found, a U lock is taken on the key range to prevent a concurrent SELECT from discovering that there is no row until the first session performs the INSERT and commits the transaction.

A slightly better pattern is to use a Application Lock around the sequence generator, calling sp_getapplock before generating the key, and then calling sp_releaseapplock immediately afterword. Then concurrent sessions wouldn't have to wait until the first session's transaction to commit before generating the next key value.

edited Sep 13 '17 at 17:56

answered Sep 13 '17 at 15:09

David Browne - Microsoft

80,331
6
39
67

Thanks David, I updated my question why I couldn't use sequence objects, as I indicate on my update I need the SequenceId behave as identity for each different CustomerId. I guess your last paragraph suggest an approach using sequences and is this consistent with my case? – ibubi Sep 14 '17 at 05:46
Yes the script above which adds extra `UPDLCOK` and `SERIALIZABLE` is working, however I would love to see an example with sequence object which guarantees seperate sequenceId increment for each customerId without any violation. – ibubi Sep 14 '17 at 09:15
1

@ibubi A [sequence](https://learn.microsoft.com/en-us/sql/t-sql/statements/create-sequence-transact-sql) can't be used that way, it is just one sequence and can't be subdivided for several categories (e.g. by customer ID). If you really need such a construct I would advise creating a counter table (see link in my answer). – TT. Sep 14 '17 at 09:32
The usual solution is to remove the "requirement" for each customer to have their own sequence, and use a single sequence across all customers. It's ultimately irrelevant if a customer's orders are numbered 1,2,4,5, . . . or 1923,2334,2556,5585, . . .. Alternatively you can provision a sequence for each customer. – David Browne - Microsoft Sep 14 '17 at 13:19

TT. · Answer 2 · 2017-09-13T15:54:43.180

You can try the following intead:

SELECT @MaxSeqId =  ISNULL(MAX(SequenceId),0)
FROM Alg.[Sequence] WITH (ROWLOCK, UPDLOCK, HOLDLOCK)
WHERE CustomerId = @CustomerId

INSERT INTO Alg.[Sequence]
VALUES (
@MaxSeqId + 1
,@CustomerId
,@SendingData
,GETDATE()
)

This will keep a lock on the sequence table (either a range lock or a table lock) until the transaction completes. Do make sure this runs in a transaction. If as you say this is run from inside a trigger, this should be implicit.

Note that determining the maximum ID this way is not efficient. Maybe an IDENTITY column would suffice if you don't mind the occasional gap in numbering. If you don't want gaps maybe a counter table can help (an example here).

Note HOLDLOCK is equivalent to SERIALIZABLE https://learn.microsoft.com/en-us/sql/t-sql/queries/hints-transact-sql-table — David Browne - Microsoft, Sep 13 '17 at 15:19

Duplicate key violation with composite primary key on high frequency insert

2 Answers2