March 25, 2010 at 3:56 am
Hey Bob and Eric,
I have no idea what is causing the duplicate insertions in the first place, but I do know that the trigger code is not sufficient to prevent duplicate insertions. The problematic code is:
INSERT INTO TableName
SELECT *
FROM inserted
WHERE NOT EXISTS (
SELECT TableID
FROM TableName
WHERE TableID = inserted.TableID
);
How reliable that is (regardless of whether EXISTS or a JOIN is used) depends entirely on isolation level. Unless the transaction is running at the SERIALIZABLE level, that code can miss stuff. Not that I am recommending SERIALIZABLE for a highly-concurrent system - my usual modification is as follows:
INSERT TableName
SELECT *
FROM inserted
WHERE NOT EXISTS
(
SELECT TableID
FROM TableName WITH (UPDLOCK, HOLDLOCK)
WHERE TableID = inserted.TableID
);
This has been gone over many times, but one blog entry that springs to mind is the following one by Alex Kuznetsov: http://sqlblog.com/blogs/alexander_kuznetsov/archive/2010/01/12/t-sql-tuesday-002-patterns-that-do-not-work-as-expected.aspx. Please don't draw any unshakeable conclusions from the article before you have read the comments 🙂
By the way, can I ask if the database is currently enabled for READ_COMMITTED_SNAPSHOT? If so, you will want to check your trigger code over and add WITH (READCOMMITTEDLOCK) in appropriate places.
Paul White
SQLPerformance.com
SQLkiwi blog
@SQL_Kiwi
March 25, 2010 at 6:38 am
Paul, thanks for the save. I wasn't even close to thinking about isolation levels last night.
__________________________________________________
Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills
March 25, 2010 at 7:00 am
The Dixie Flatline (3/25/2010)
Paul, thanks for the save. I wasn't even close to thinking about isolation levels last night.
It's a team effort, Bob. 🙂
Paul White
SQLPerformance.com
SQLkiwi blog
@SQL_Kiwi
March 25, 2010 at 7:07 am
Understood and agreed. I'm just glad that we're on the same team.
__________________________________________________
Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills
March 25, 2010 at 7:20 am
Paul White NZ (3/25/2010)
Hey Bob and Eric,This has been gone over many times, but one blog entry that springs to mind is the following one by Alex Kuznetsov: http://sqlblog.com/blogs/alexander_kuznetsov/archive/2010/01/12/t-sql-tuesday-002-patterns-that-do-not-work-as-expected.aspx. Please don't draw any unshakeable conclusions from the article before you have read the comments 🙂
By the way, can I ask if the database is currently enabled for READ_COMMITTED_SNAPSHOT? If so, you will want to check your trigger code over and add WITH (READCOMMITTEDLOCK) in appropriate places.
Paul,
Thanks for the link and info, I'll take a read of that this morning and do some testing on your suggested fix.
Running:
SELECTis_read_committed_snapshot_on
FROMsys.databases
For my database returns 0 So looks like I don't have READ_COMMITTED_SNAPSHOT enabled.
March 25, 2010 at 7:33 am
Eric Zierdt (3/25/2010)
...so looks like I don't have READ_COMMITTED_SNAPSHOT enabled.
Ok, cool. One less thing to worry about. I am a fan of RCS, but you do have to be careful in triggers and with foreign keys.
Let us know if you have any questions after reading that stuff.
I did make some assumptions by the way...
1. You have a useful index to support the "WHERE TableID = inserted.TableID"
2. The trigger usually operates on a small number of rows (maybe just one?)
3. The trigger is not too long, and there is no long outer transaction
4. The expected case is that the TableID will not exist already
Assuming all of most of those are true, the UPDLOCK, HOLDLOCK hints should be fine.
Paul White
SQLPerformance.com
SQLkiwi blog
@SQL_Kiwi
March 26, 2010 at 8:33 am
We implemented the change this morning, I'll be monitoring throughout the day.
Thanks again.
March 26, 2010 at 8:39 am
Eric Zierdt (3/26/2010)
We implemented the change this morning, I'll be monitoring throughout the day.
How exciting! I take it the testing went well.
Paul White
SQLPerformance.com
SQLkiwi blog
@SQL_Kiwi
March 26, 2010 at 9:01 am
Eric Zierdt (3/24/2010)
The Dixie Flatline (3/24/2010)
The duplicates you mention: Are they simply duplicates of the [TableID], or is the [createdTime] also identical. Since it's a datetime field it would have to be identical down to the millisecond. If it isn't, that's the problem right there, because the VALUES clause uses getdate() to populate that column, and that will be different every time the procedure is executed.I ran this:
SELECT*
FROMdbo.tablename WITH (NOLOCK)
WHEREtableid = '7277D2DB-2D50-4FE4-888E-8E7C9261B256'
and got this:
TableIdCreatedTimeOsIdDefaultBrowserVersionIdIEVersionIddefaultbrowseridLangId
7277D2DB-2D50-4FE4-888E-8E7C9261B2562010-03-24 02:40:12.860NULL4NULLNULLNULL
7277D2DB-2D50-4FE4-888E-8E7C9261B2562010-03-24 02:40:12.863NULL4NULLNULLNULL
And in all but one of the duplicates I looked over the past two day the difference was .003 seconds apart
So since they are all different times, we can assume the proc is firing more than once, but if thats the case, I don't know why the tirgger doesn't catch it.
remember 3ms is the lowest precision for Datetime. So - they oculd be coming in at the same time, serialized
----------------------------------------------------------------------------------
Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?
March 26, 2010 at 9:08 am
Hey Matt... I just got some new glasses.
Is your avatar the Master Control Program from "Tron" ??
__________________________________________________
Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills
March 26, 2010 at 11:48 am
The Dixie Flatline (3/26/2010)
Hey Matt... I just got some new glasses.Is your avatar the Master Control Program from "Tron" ??
The user "Matt" you speak of has been terminated from this network. MCP controls all.
----------------------------------------------------------------------------------
Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?
March 26, 2010 at 12:01 pm
HERESY!
The Thread controls all.
Great is The Thread.
Mighty, mighty is the Thread
__________________________________________________
Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills
April 1, 2010 at 10:42 am
Just wanted to send a followup, after a week of running the changes, we haven't gotten a new duplicate.
Thanks all!!!
April 1, 2010 at 10:55 am
Eric Zierdt (4/1/2010)
Just wanted to send a followup, after a week of running the changes, we haven't gotten a new duplicate. Thanks all!!!
Sweet as.
Paul White
SQLPerformance.com
SQLkiwi blog
@SQL_Kiwi
April 1, 2010 at 12:04 pm
The problem should be easily solved with a MERGE statement inside your trigger. I had a similar task to solve and it's been taken care of very elegantly using that merge.
No chance for a duplicate to come in.
Example:
IF EXISTS (SELECT * FROM sys.triggers WHERE object_id = OBJECT_ID(N'[dbo].[TG_vi_applicant]'))
DROP TRIGGER [dbo].[TG_vi_applicant]
GO
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
SET ANSI_PADDING ON
GO
CREATE TRIGGER [dbo].[TG_vi_applicant]
ON vi_applicant
INSTEAD OF INSERT
AS
BEGIN
SET NOCOUNT ON
SET XACT_ABORT ON
BEGIN TRANSACTION
--call_time_pref
MERGE dbo.call_time_pref WITH(SERIALIZABLE) AS Target
USING (SELECT DISTINCT call_time_pref FROM inserted) AS Source
ON ( Target.name = Source.call_time_pref )
WHEN NOT MATCHED BY TARGET THEN
INSERT( date_created, name, active )
VALUES( GETDATE(), Source.call_time_pref, 1 );
--date_of_birth --NON-REFERENCE DATA, HIGH CONCURRENCY
MERGE INTO dbo.date_of_birth WITH(SERIALIZABLE) AS Target
USING (SELECT DISTINCT date_of_birth FROM inserted) AS Source
ON ( Target.date_of_birth = Source.date_of_birth )
WHEN NOT MATCHED BY TARGET THEN
INSERT( date_created, date_of_birth )
VALUES( GETDATE(), Source.date_of_birth );
--legal_id_number --NON-REFERENCE DATA, HIGH CONCURRENCY
MERGE INTO dbo.legal_id_number WITH(SERIALIZABLE) AS Target
USING (SELECT DISTINCT legal_id_number FROM inserted) AS Source
ON ( Target.legal_id_number = Source.legal_id_number )
WHEN NOT MATCHED BY TARGET THEN
INSERT( date_created, legal_id_number )
VALUES( GETDATE(), Source.legal_id_number );
--legal_id_type
MERGE INTO dbo.legal_id_type WITH(SERIALIZABLE) AS Target
USING (SELECT DISTINCT legal_id_type FROM inserted) AS Source
ON ( Target.name = Source.legal_id_type )
WHEN NOT MATCHED BY TARGET THEN
INSERT( date_created, name, active)
VALUES( GETDATE(), Source.legal_id_type, 1 );
--ssn -- NON-REFERENCE DATA, HIGH CONCURRENCY
MERGE INTO dbo.ssn WITH(SERIALIZABLE) AS Target
USING (SELECT DISTINCT ssn FROM inserted) AS Source
ON ( Target.ssn = Source.ssn )
WHEN NOT MATCHED BY TARGET THEN
INSERT( date_created, ssn )
VALUES( GETDATE(), Source.ssn );
INSERT INTO applicant
(
[date_created],
[date_modified],
[age],
,
[email_alt],
[phone_home],
[phone_cell],
[phone_fax],
[call_time_pref_id],
[name_first],
[name_middle],
[name_last],
[date_of_birth_id],
[application_lead_id],
[legal_id_number_id],
[legal_id_state],
[legal_id_type_id],
[ssn_id]
)
SELECTi.date_created,
i.date_modified,
i.age,
i.email,
i.email_alt,
i.phone_home,
i.phone_cell,
i.phone_fax,
ctp.call_time_pref_id,
i.name_first,
i.name_middle,
i.name_last,
dob.date_of_birth_id,
i.application_lead_id,
lin.legal_id_number_id,
i.legal_id_state,
lit.legal_id_type_id,
ssn.ssn_id
FROM inserted i
INNER JOIN call_time_pref ctp
ON i.call_time_pref = ctp.name
INNER JOIN date_of_birth dob
ON i.date_of_birth = dob.date_of_birth
INNER JOIN legal_id_number lin
ON i.legal_id_number = lin.legal_id_number
INNER JOIN legal_id_type lit
ON i.legal_id_type = lit.name
INNER JOIN ssn ssn
ON i.ssn = ssn.ssn
COMMIT TRANSACTION
END
GO
Viewing 15 posts - 16 through 30 (of 37 total)
You must be logged in to reply to this topic. Login to reply