6

I'm getting an error that I thought was impossible. I have a SQL Server (ver 11.0.6594) table that looks like this:

CREATE TABLE [IDMaster](
  [ID] [int] NOT NULL,
 CONSTRAINT [PK_IDMaster] PRIMARY KEY CLUSTERED 
([ID] ASC) ON [PRIMARY]
) ON [PRIMARY]

It's basically supposed to do what the IDENTITY does now (it's been around a while) – it is used to get new unique integer values, that are used in a few other tables.

Now I have a SQL statement, which selects the MAX value, adds to it, and INSERTs that value into the same table – all inside one statement. So in theory, it's not possible to get a PK violation on this statement (although I think could possibly get a deadlock). But, somehow, it is getting one.

Violation of PRIMARY KEY constraint 'PK_IDMaster'. Cannot insert duplicate key in object 'IDMaster'. The duplicate key value is (25309587).

I don't know how this is possible, but apparently it is; because it just happened.

Imagine any crazy, odd mixture of other ways to try generating a new ID value from this same table, and they probably aren't far off from what exists in this system. But I can't think of any way that they could cause that particular error. It was my understanding that the transaction locking used by SQL Server enforces that, within one SQL statement that doesn't have (NOLOCK) on it or anything, there can't be any interference with the rows that it is referencing.

The only other factor, which I don't think could be related but who knows, is that this SQL is wrapped in a TRY, with some options set. Here's the SQL that got that error:

SET XACT_ABORT ON;
SET IMPLICIT_TRANSACTIONS ON;
BEGIN TRY

INSERT INTO IDMaster (ID) 
 SELECT COALESCE(
  -- make sure the new value is odd
  CASE WHEN MAX(COALESCE(ID,0)) % 2 = 0
   THEN MAX(COALESCE(ID,0)) + 1
   ELSE MAX(COALESCE(ID,0)) + 2
   END,1)
 FROM IDMaster;

END TRY
BEGIN CATCH
  WHILE (@@TRANCOUNT > 0) ROLLBACK;
  THROW;
END CATCH
WHILE (@@TRANCOUNT > 0) COMMIT;

Please explain:

  1. How is it possible to get that error?
  2. If possible, how can I prevent it from ever happening (without altering the table)?
  • 5
    And do you have multiple threads/processes/connections calling this procedure? – ypercubeᵀᴹ Aug 4 '17 at 18:34
  • 2
    Just to get a bit of context. Why are you ditching the Identity for a "computed" identity? – Nestor Aug 5 '17 at 11:37
  • I second @Nestor's question and go further: you better have a very, very, very good reason to not use the standard constructs for the end goal you're trying to accomplish. If not, the answer is stop it. Switch to IDENTITY or at least an actual SEQUENCE as soon as possible. – jpmc26 Aug 5 '17 at 17:26
  • Yes, there are several different service that generate a new ID value. My service is just one of them. The others use different (worse) methods to generate a new ID value. Re-engineering this table used by these several services is not an option. – Abacus Aug 7 '17 at 18:20
6

Your theory is wrong. Reading from your "identity" table doesn't lock it, the insert into it locks it. While they are part of a single compound statement, they are still seperate actions. Two (or more) connections can be reading from the table during the time between the read and the insert. The insert uses a lock, so only one of the connections will actually be able to write the value they read into the table, other connections, will, as you have discovered, get a primary key violation error.

The best solution is to use one of the builtin methods for handling this: identity columns, sequences, or guids. Roll your own solutions should generally be avoided: you simply don't have the experience or user base that the database management systems do (and you are dealing with an abstraction, so even if you were one of the programmers for one of the major database management systems, you wouldn't be able to do the same kind of things).

  • Thanks. I agree, but it's not my table to ALTER, so I can't do that. I will have to either apply some type of locking, or add "retry" logic. – Abacus Aug 7 '17 at 18:29
23

Now I have a SQL statement, which selects the MAX value, adds to it, and INSERTs that value into the same table -- all inside one statement. So in theory, it's not possible to get a PK violation on this statement (although I think could possibly get a deadlock). But, somehow, it IS getting one.

It definitely IS possible. This is due to your isolation level and how locking works.

Violation of PRIMARY KEY constraint 'PK_IDMaster'. Cannot insert duplicate key in object 'IDMaster'. The duplicate key value is (25309587).

This is due to locking; initially in the read committed isolation level will take a shared lock on the (in this case) key while reading what the maximum value is. After it figures it out, an X lock will be needed to insert into the table. That's the simple overview of it.

When you have a single session, this isn't a problem. When you have multiple sessions this is a huge problem as shared locks are compatible with each other. This means that multiple readers can obtain the same value.

don't know how this is possible, but apparently it is; because it just happened.

See above.

So How Do You Fix It?

  1. Don't try to be smarter than SQL Server. Use the available constructs that fit the need - in SQL Server 2012 (major version 11) has both identity and sequences. Use them.

  2. Purposely kill your concurrency by either using serializable or lock hints such as XLOCK. This will on purpose block other sessions which means you'll have less performance... and you're doing it on purpose... so yeah that seems bad but a possible solution.

  3. Pre-create values (again trying to be smarter than SQL Server) and use XLOCK + READPAST to get a little better concurrency from a different table. Blah. Let me reiterate #1, don't try to be smarter than SQL Server [in this case].

  • I agree in theory, but you're violating the premise of the question -- "without altering the table". – Abacus Aug 7 '17 at 18:21
  • 3
    @Abacus No, #2 and #3 answer that part. It's terrible, but it's an answer given you don't want to fix the actual issue but more have it work at all costs without changing anything even if that means grinding all work to halt. – Sean Gallardy Aug 8 '17 at 3:09
1

An alternative method for serializing access to a section of code is application locks.

The downside is it requires marginally more development work. The upside is multiple statements can be inside the applock scope, preventing deadlocks.

-2

This is only a partial answer -- I figured out the "how to fix" thanks another DBA. The solution is to use

WITH (TABLOCK, HOLDLOCK)
  • 3
    And there goes your concurrency. Seriously, the best approach is to abandon your table and start using a sequence. – LoztInSpace Aug 5 '17 at 2:10
  • 1
    Please explain why before you downvote. And consider reading the context of the question. If someone asks how to fix their car, saying "buy a new car" is not an answer to that question. In my scenario, even an "80% solution" is preferable than "spend days talking to others, to ask them to spend weeks of their time rewriting code, which they don't think is broken" – Abacus Aug 7 '17 at 18:25
  • 5
    The votes may reflect the suitability of this answer for other people finding this Q & A in future (who are unlikely to face the same restrictions as you). People are probably voting on the general idea, so don't take it personally. – Paul White Aug 14 '17 at 6:30
  • Good point Paul, thanks. Yes, in this case I am in a very specific area that I really hope others will avoid. So even if I can't get out of it, I gladly accept the negative votes, on the basis that others might be able to use this to improve their own situation. :) – Abacus Mar 11 at 15:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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