Potential Locking Issue - Inserts Failing Due to Duplicate Values

  • I have an interesting issue. I have a very old database that I am managing that at one point in time could not use identity columns due to programming issues at the time. Instead, as a workaround, development created what is basically a primary key table. This table contains two fields, one for table name and one for the "PK" value. When an insert occurs, a stored procedure runs that pulls the next pk value from that table and then increments it by one. I'm trying to get them to rethink this logic but that probably won't happen anytime soon.

    This table is being hammered by multiple programs both old and new and I am running into an issue with a newer .net program. It's pulling a pk from that pk table and then trying to insert a record, however, when it does, it will periodically get errors related to that PK already existing. The .net program is using the same stored procedure as the other programs, it's not using nolocks or any funky stuff like that. Logically, I feel that by using the stored proc, when grabbing and updating that field in the PK table, it should lock it, select it, increment it, and then release the lock, thereby preventing any duplicate values from being selected. So it looks like this isn't always happening. Any ideas? Is it possible that because it's there could be dozens of these events happening every second that the stored procedure executes and selects the record at the exact same time?

  • Josh i inherited a similar solution once, and it depends on where it gets the next value from;

    the original badly written proc would get the max value from a table, add one to it, and return it as the value.

    the problem with that is that it didn't allow for concurrency...if the same app was called 4 times BEFORE any instance finished inserting, you'd get duplicates.

    instead we created a key table, that kept the last value returned for a given table,and the procedure got data form that...so it did two things: checked that the current max in the real table was less than the current incremented the value in the "keys" table, incremented the keys table by one, and returned the value.

    can you show us the proc that is returning the next value? that's where the tweak to fix concurrency is going to benefit from some peer review.

    Lowell


    --help us help you! If you post a question, make sure you include a CREATE TABLE... statement and INSERT INTO... statement into that table to give the volunteers here representative data. with your description of the problem, we can provide a tested, verifiable solution to your question! asking the question the right way gets you a tested answer the fastest way possible!

  • It's not a historical table so each table has only one value, that value is selected then updated so max is not used.

    Here's the table,

    CREATE TABLE [dbo].[key_table](

    [alias] [char](25) NOT NULL,

    [lastkey] [int] NULL,

    CONSTRAINT [PK_key_table] PRIMARY KEY CLUSTERED

    (

    [alias] ASC

    )WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]

    ) ON [PRIMARY]

    GO

    SET ANSI_PADDING OFF

    GO

    Here's the stored proc

    ALTER PROCEDURE [dbo].[GET__PK] @ALIAS VARCHAR(50), @NextKey INT OUTPUT, @KeyCntReserve INT

    AS

    DECLARE @LastKey INT

    UPDATE pk_gen SET @LastKey = lastkey, lastkey = lastkey + @KeyCntReserve WHERE ALIAS = @ALIAS

    SELECT @NextKey = @LastKey + 1

    RETURN

    Within the key_table is simply the table name, say Table_1, and the last key value, say 0. I really don't like the way this is written.

  • Since you're posting in the SQL Server 2008 forum, I'm assuming the legacy database has been ported over to this version. The solution below may also require the database Compatibility Level to be at least 2005.

    Rather than performing UPDATE followed by SELECT, you can leverage the update's OUTPUT clause to perform both operations within the same statement, which is very efficient and atomic.

    Here is basically what I'm proposing:

    -- First we'll setup the table:

    create table table_id

    (

    table_name varchar(180) not null primary key,

    id int not null

    );

    insert into table_id (table_name, id) values ('mytable',0);

    -- Your existing stored procedure can be retrofitted to work like the following:

    create procedure get_table_id ( @table_name varchar(180) )

    as

    declare @table_id as table ( id int not null );

    update table_id set id = id + 1

    output inserted.id into @table_id

    where table_name = @table_name;

    return (select id from @table_id);

    GO

    -- Here is an example of usage.

    -- It returns the next incremented ID each time it's called.

    declare @id int;

    exec @id = get_table_id 'mytable';

    print @id;

    "Do not seek to follow in the footsteps of the wise. Instead, seek what they sought." - Matsuo Basho

  • This seems like a good solution, but still requires buy-in from dev. I'll see if I can get them to modify their code.

    Thanks.

  • Is it possible that the @KeyCountReserve parameter is sometimes zero or negative? That would cause duplicates to be generated.

  • Chris Wooding (4/16/2014)


    Is it possible that the @KeyCountReserve parameter is sometimes zero or negative? That would cause duplicates to be generated.

    I double checked that, it's not used currently. That variable was used for certain rare situations in the past.

Viewing 7 posts - 1 through 6 (of 6 total)

You must be logged in to reply to this topic. Login to reply