April 15, 2014 at 10:19 am
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?
April 15, 2014 at 10:43 am
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
April 15, 2014 at 10:55 am
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.
April 15, 2014 at 11:07 am
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
April 15, 2014 at 11:24 am
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.
April 16, 2014 at 5:09 am
Is it possible that the @KeyCountReserve parameter is sometimes zero or negative? That would cause duplicates to be generated.
April 16, 2014 at 8:35 am
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