SQL Server Forum / General / Other SQL Server Topics / March 2008
concurrency problem with lists ("check constraint" on groups of rows)
|
|
Thread rating:  |
B D Jensen - 11 Mar 2008 23:12 GMT Hi! Should I add retry logic avoiding deadlock as described many places or is there an better solution if one need "check constraints" (I know it doesn't exists 'yet') on group of rows?
CREATE TABLE myGroup ( [myGroupID] [int] IDENTITY(1, 1) NOT NULL , CONSTRAINT [PK_myGroup] PRIMARY KEY CLUSTERED ([myGroupID] ASC) ) ;
CREATE TABLE myGroupMember ( [myGroupMemberID] [int] IDENTITY(1, 1) NOT NULL , [myGroupID] [int] NOT NULL , [ID] [int] NOT NULL , CONSTRAINT [PK_myGroupMember] PRIMARY KEY CLUSTERED ([myGroupMemberID] ASC) , CONSTRAINT [UK_myGroupMember_myGroupID_ID] UNIQUE NONCLUSTERED -- not enough because this will not avoid dublet lists ([myGroupID] ASC, [ID] ASC) ) ; -- FK connecting myGroupMember.myGroupID with myGroup.myGroupID...
CREATE FUNCTION split ( --CREATE FUNCTION utility.split ( @rowdata NVARCHAR(MAX) -- #param row data , @split_character VARCHAR(3) -- #param typical , or ; ) RETURNS @rettbl TABLE ( nbr INT IDENTITY(1, 1) , data NVARCHAR(MAX) ) AS -- #author ? -- #version 2007-05-20 -- #desc returns string with seperators as table of elements BEGIN WHILE (CHARINDEX(@split_character, @rowdata) > 0) BEGIN INSERT INTO @rettbl (data) SELECT data = LTRIM(RTRIM(SUBSTRING(@rowdata, 1, CHARINDEX(@split_character, @rowdata) - 1))) ;
SET @rowdata = SUBSTRING(@rowdata, CHARINDEX(@split_character, @rowdata) + 1, LEN(@rowdata)) ; END ;
INSERT INTO @rettbl (data) SELECT Data = LTRIM(RTRIM(@rowdata)) ;
RETURN ; END ; go
CREATE PROCEDURE createMyGroup @list_in VARCHAR(MAX) -- #param list of id's , @myGroupID_out INT OUTPUT -- #param pseudo key AS /* Description: variation of sql divide Change History: $Date: $ , $Author: bdj $, $Revision: 0 $ */ BEGIN SET NOCOUNT ON SET TRANSACTION ISOLATION LEVEL SERIALIZABLE DECLARE @rowcnt INT ; DECLARE @list TABLE (id INT PRIMARY KEY) ; -- important constraint! DECLARE @message VARCHAR(100)
SELECT @myGroupID_out = NULL ; -- real output -- exec utility.debug_info '.....' BEGIN TRY BEGIN TRAN
INSERT INTO @list SELECT CONVERT(INT, s.data) FROM DBO.split(@list_in, ',') s
SELECT @myGroupID_out = a2.myGroupID FROM dbo.myGroupMember a2 WHERE EXISTS ( SELECT NULL FROM @list b1 WHERE b1.id = a2.ID ) GROUP BY a2.myGroupID HAVING (SELECT COUNT (*) FROM myGroupMember a3 WHERE a3 . myGroupID = a2 . myGroupID) = (SELECT COUNT (*) FROM @list) AND COUNT(*) = (SELECT COUNT (*) FROM @list)
SELECT @rowcnt = @@ROWCOUNT WAITFOR DELAY '00:01:01' -- wait 1min 1 second IF @rowcnt > 1 -- when this happens we have a problem BEGIN
SELECT @message = 'More than 1 myGroupID ' + COALESCE(CAST(@myGroupID_out AS VARCHAR(100)), 'null ') SELECT @message = @message + COALESCE(CAST(id AS VARCHAR(100)), '-') + ',' FROM @list SELECT @myGroupID_out = NULL
RAISERROR (@message , 16 , 1) END ELSE IF @rowcnt = 1 BEGIN PRINT 'be happy' END ELSE IF @rowcnt = 0 BEGIN INSERT INTO dbo.myGroup DEFAULT VALUES ; SELECT @myGroupID_out = SCOPE_IDENTITY() ;
INSERT INTO dbo.myGroupMember SELECT @myGroupID_out gid, id FROM @list ; END ; COMMIT TRAN END TRY BEGIN CATCH ROLLBACK TRAN SELECT @myGroupID_out = NULL --EXEC raise_error SELECT ERROR_MESSAGE() END CATCH SET TRANSACTION ISOLATION LEVEL READ COMMITTED END ; GO
DECLARE @list VARCHAR(MAX), @myGroupID INT EXEC createMyGroup '11,12,13', @myGroupID OUTPUT SELECT @myGroupID
SELECT * FROM myGroup SELECT * FROM myGroupMember
-- now do the same again DECLARE @list VARCHAR(MAX), @myGroupID INT EXEC createMyGroup '11,12,13', @myGroupID OUTPUT SELECT @myGroupID -- remark now new rows has to be created :-)
-- try a different example DECLARE @list VARCHAR(MAX), @myGroupID INT EXEC createMyGroup '11,12', @myGroupID OUTPUT SELECT @myGroupID -- a new group has been created :-)
-- try a different example DECLARE @list VARCHAR(MAX), @myGroupID INT EXEC createMyGroup '111,222', @myGroupID OUTPUT SELECT @myGroupID -- a new group has been created :-)
-- all fine until now... -- Now from 2 sessions: WAITFOR TIME '22:58'; DECLARE @list VARCHAR(MAX), @myGroupID INT EXEC createMyGroup '1000,2000,3000,4000,5008', @myGroupID OUTPUT SELECT @myGroupID
-- 1. session returned an ID, but -- 2. session : Transaction (Process ID 53) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction.
Best regards Bjorn
Erland Sommarskog - 11 Mar 2008 23:15 GMT > Should I add retry logic avoiding deadlock as described many places > or is there an better solution if one need "check constraints" (I know > it doesn't exists 'yet') on group of rows? I don't really get what you are trying to achieve. It's great that you posted the code, but without knowing the story behind it, it's a little difficult to give advice.
But I note that you use the serializable isolation level. This is a good recipe to get plentyful of deadlocks.
 Signature Erland Sommarskog, SQL Server MVP, esquel@sommarskog.se
Books Online for SQL Server 2005 at http://www.microsoft.com/technet/prodtechnol/sql/2005/downloads/books.mspx Books Online for SQL Server 2000 at http://www.microsoft.com/sql/prodinfo/previousversions/books.mspx
B D Jensen - 12 Mar 2008 06:17 GMT Hi! The purpose is to store lists so I later can make easy sql-exists consctructs: select * from T t where exists (select null from myGroupMember p where p.id=t.id and p.myGroupID=@myGroupID)
The table myGroupMember will become as small as possible, since a list only will be stored once. The lists comes from user input, so I don't know them before.
The code above gives oppurtunity to make a FK's on myGroupID in other tables! => analysis of which rows (in a new table A) uses the same group become trivial!
And there is a reason for setting the isolation level: try to comment out the lines with "SET..." Both session complete returning different id's :-( Next time you run call to proc with same input param it will result in something like this: More than 1 myGroupID 121000,2000,3000,4000,5009,
Hope its now more clear what I want to achieve ;-) Best regards Bjorn
Erland Sommarskog - 12 Mar 2008 23:37 GMT > The purpose is to store lists so I later can make easy sql-exists > consctructs: [quoted text clipped - 3 lines] > The table myGroupMember will become as small as possible, since a list > only will be stored once. So if you first enter 1,2,3 and then 3,2,1 that will be the same list? But 1,2,3,4 is a new list?
I don't see the point with the column myGroupMemberID. That seems just to be a useless IDENTITY column.
Shouldn't you have an index on (ID, MyGroupID) as well, so that you don't have to scan the entire table when trying to match an input list?
> And there is a reason for setting the isolation level: try to comment > out the lines with "SET..." > Both session complete returning different id's :-( Next time you run > call to proc with same input param it will result in > something like this: More than 1 myGroupID 121000,2000,3000,4000,5009, I think you can avoid the serializable transaction. Remove the IDENTITY property of the myGroup table. Then first in the transaction do:
SELECT @newgroupid = coalesce(MAX(myGroupID), 0) + 1 FROM myGroups WITH (UPDLOCK)
This will create a serialization point for all entries to the procedure. Except for when the table is entirely empty, but this can be managed by a dummy entry.
 Signature Erland Sommarskog, SQL Server MVP, esquel@sommarskog.se
Books Online for SQL Server 2005 at http://www.microsoft.com/technet/prodtechnol/sql/2005/downloads/books.mspx Books Online for SQL Server 2000 at http://www.microsoft.com/sql/prodinfo/previousversions/books.mspx
B D Jensen - 13 Mar 2008 13:32 GMT Hi again! "So if you first enter 1,2,3 and then 3,2,1 that will be the same list? But 1,2,3,4 is a new list? " YES.
"I don't see the point with the column myGroupMemberID. That seems just to be a useless IDENTITY column. " You are right. If you are interested we can take this classic discussion somewhere? ;-) I have removed it from the new example below.
"Shouldn't you have an index on (ID, MyGroupID)" It was there (-: [UK_myGroupMember_myGroupID_ID]
I don't think I fully understand your suggestion, please see new version of code below. The new version again gives two different results :-( which will result in error with next run with same input.
Best regards Bjorn
--------------------- DROP TABLE myGroup;
CREATE TABLE myGroup ( [myGroupID] [int] --IDENTITY(1, 1) NOT NULL , CONSTRAINT [PK_myGroup] PRIMARY KEY CLUSTERED ([myGroupID] ASC) ) ;
INSERT INTO myGroup VALUES(0);
DROP TABLE myGroupMember;
CREATE TABLE myGroupMember ( --[myGroupMemberID] [int] IDENTITY(1, 1) NOT NULL [myGroupID] [int] NOT NULL , [ID] [int] NOT NULL , CONSTRAINT [PK_myGroupMember] PRIMARY KEY CLUSTERED ([myGroupID], [ID]) -- , CONSTRAINT [PK_myGroupMember] PRIMARY KEY CLUSTERED ([myGroupMemberID] ASC) -- , CONSTRAINT [UK_myGroupMember_myGroupID_ID] UNIQUE NONCLUSTERED -- not enough because this will not avoid dublet lists -- ([myGroupID] ASC, [ID] ASC) ) ; -- FK connecting myGroupMember.myGroupID with myGroup.myGroupID...
CREATE FUNCTION split ( --CREATE FUNCTION utility.split ( @rowdata NVARCHAR(MAX) -- #param row data , @split_character VARCHAR(3) -- #param typical , or ; ) RETURNS @rettbl TABLE ( nbr INT IDENTITY(1, 1) , data NVARCHAR(MAX) ) AS -- #author ? -- #version 2007-05-20 -- #desc returns string with seperators as table of elements BEGIN WHILE (CHARINDEX(@split_character, @rowdata) > 0) BEGIN INSERT INTO @rettbl (data) SELECT data = LTRIM(RTRIM(SUBSTRING(@rowdata, 1, CHARINDEX(@split_character, @rowdata) - 1))) ;
SET @rowdata = SUBSTRING(@rowdata, CHARINDEX(@split_character, @rowdata) + 1, LEN(@rowdata)) ; END ;
INSERT INTO @rettbl (data) SELECT Data = LTRIM(RTRIM(@rowdata)) ;
RETURN ; END ; go
DROP PROC createMyGroup GO
CREATE PROCEDURE createMyGroup @list_in VARCHAR(MAX) -- #param list of id's , @myGroupID_out INT OUTPUT -- #param pseudo key AS /* Description: variation of sql divide Change History: $Date: $ , $Author: bdj $, $Revision: 0 $ */ BEGIN SET NOCOUNT ON --SET TRANSACTION ISOLATION LEVEL SERIALIZABLE DECLARE @rowcnt INT ; DECLARE @list TABLE (id INT PRIMARY KEY) ; -- important constraint! DECLARE @message VARCHAR(100)
SELECT @myGroupID_out = NULL ; -- real output -- exec utility.debug_info '.....' BEGIN TRY BEGIN TRAN
INSERT INTO @list SELECT CONVERT(INT, s.data) FROM DBO.split(@list_in, ',') s
SELECT @myGroupID_out = a2.myGroupID FROM dbo.myGroupMember a2 WHERE EXISTS ( SELECT NULL FROM @list b1 WHERE b1.id = a2.ID ) GROUP BY a2.myGroupID HAVING (SELECT COUNT (*) FROM myGroupMember a3 WHERE a3 . myGroupID = a2 . myGroupID) = (SELECT COUNT (*) FROM @list) AND COUNT(*) = (SELECT COUNT (*) FROM @list)
SELECT @rowcnt = @@ROWCOUNT WAITFOR DELAY '00:01:01' -- wait 1min 1 second IF @rowcnt > 1 -- when this happens we have a problem BEGIN
SELECT @message = 'More than 1 myGroupID ' + COALESCE(CAST(@myGroupID_out AS VARCHAR(100)), 'null ') SELECT @message = @message + COALESCE(CAST(id AS VARCHAR(100)), '-') + ',' FROM @list SELECT @myGroupID_out = NULL
RAISERROR (@message , 16 , 1) END ELSE IF @rowcnt = 1 BEGIN PRINT 'be happy' END ELSE IF @rowcnt = 0 BEGIN SELECT @myGroupID_out = COALESCE(MAX(myGroupID), 0) + 1 FROM myGroup WITH (UPDLOCK) -- or ?: SELECT @myGroupID_out = myGroupID + 1 FROM myGroup WITH (UPDLOCK)
UPDATE myGroup SET myGroupID = myGroupID +1
-- INSERT INTO dbo.myGroup -- DEFAULT VALUES ; -- SELECT @myGroupID_out = SCOPE_IDENTITY() ;
INSERT INTO dbo.myGroupMember SELECT @myGroupID_out gid, id FROM @list ; END ; COMMIT TRAN END TRY BEGIN CATCH ROLLBACK TRAN SELECT @myGroupID_out = NULL --EXEC raise_error SELECT ERROR_MESSAGE() END CATCH --SET TRANSACTION ISOLATION LEVEL READ COMMITTED END ; GO
Erland Sommarskog - 13 Mar 2008 23:07 GMT > "Shouldn't you have an index on (ID, MyGroupID)" It was there (-: > [UK_myGroupMember_myGroupID_ID] No, that is a UNIQUE constraint on (MyGroupID, ID). I specifically meant an index in reverse order, to speed up the lookup from @list which is done on ID.
> I don't think I fully understand your suggestion, please see new > version of code below. > The new version again gives two different results :-( which will > result in error with next run with same input. The idea was that you should have:
> BEGIN TRAN -- Get the id for a new group if we need one. This also acts as -- queuing point for simultaneous requests. SELECT @newgroupid = COALESCE(MAX(myGroupID), 0) + 1 FROM myGroup WITH (UPDLOCK)
> INSERT INTO @list > SELECT CONVERT(INT, s.data) > FROM DBO.split(@list_in, ',') s >... > IF @rowcnt = 0 > BEGIN SELECT @myGroupID_out = @newgroupid
INSERT myGroup (myGroupId) VALUES (@newgroupid)
> END ; > COMMIT TRAN
 Signature Erland Sommarskog, SQL Server MVP, esquel@sommarskog.se
Books Online for SQL Server 2005 at http://www.microsoft.com/technet/prodtechnol/sql/2005/downloads/books.mspx Books Online for SQL Server 2000 at http://www.microsoft.com/sql/prodinfo/previousversions/books.mspx
B D Jensen - 14 Mar 2008 11:04 GMT Hi Erland!
> No, that is a UNIQUE constraint on (MyGroupID, ID). I specifically > meant an index in reverse order, to speed up the lookup from @list > which is done on ID. Ahh, ok.
I made an new example below, but I didn't fully understand why to select on myGroup only with UPDLOCK, why not on myGroupMember also? But it seems to work. Before I fix this in production: please explain!
Greetings & Thanks Bjorn
=========================== DROP TABLE myGroup;
CREATE TABLE myGroup ( [myGroupID] [int] --IDENTITY(1, 1) NOT NULL , CONSTRAINT [PK_myGroup] PRIMARY KEY CLUSTERED ([myGroupID] ASC) ) ;
INSERT INTO myGroup VALUES(0);
DROP TABLE myGroupMember;
CREATE TABLE myGroupMember ( --[myGroupMemberID] [int] IDENTITY(1, 1) NOT NULL [myGroupID] [int] NOT NULL , [ID] [int] NOT NULL , CONSTRAINT [PK_myGroupMember] PRIMARY KEY CLUSTERED ([myGroupID], [ID]) -- , CONSTRAINT [PK_myGroupMember] PRIMARY KEY CLUSTERED ([myGroupMemberID] ASC) -- , CONSTRAINT [UK_myGroupMember_myGroupID_ID] UNIQUE NONCLUSTERED -- not enough because this will not avoid dublet lists -- ([myGroupID] ASC, [ID] ASC) ) ; -- FK connecting myGroupMember.myGroupID with myGroup.myGroupID...
DROP PROC createMyGroup GO
CREATE PROCEDURE createMyGroup @list_in VARCHAR(MAX) -- #param list of id's , @myGroupID_out INT OUTPUT -- #param pseudo key AS /* Description: variation of sql divide Change History: $Date: $ , $Author: bdj $, $Revision: 0 $ */ BEGIN SET NOCOUNT ON --SET TRANSACTION ISOLATION LEVEL SERIALIZABLE DECLARE @rowcnt INT ; DECLARE @list TABLE (id INT PRIMARY KEY) ; -- important constraint! DECLARE @message VARCHAR(100) DECLARE @newgroupid INT
SELECT @myGroupID_out = NULL ; -- real output -- exec utility.debug_info '.....'
BEGIN TRY BEGIN TRAN
SELECT @newgroupid = COALESCE(MAX(myGroupID), 0) + 1 FROM myGroup WITH (UPDLOCK)
INSERT INTO @list SELECT CONVERT(INT, s.data) FROM DBO.split(@list_in, ',') s
SELECT @myGroupID_out = a2.myGroupID FROM dbo.myGroupMember a2 WHERE EXISTS ( SELECT NULL FROM @list b1 WHERE b1.id = a2.ID ) GROUP BY a2.myGroupID HAVING (SELECT COUNT (*) FROM myGroupMember a3 WHERE a3.myGroupID = a2.myGroupID) = (SELECT COUNT (*) FROM @list) AND COUNT(*) = (SELECT COUNT (*) FROM @list)
SELECT @rowcnt = @@ROWCOUNT WAITFOR DELAY '00:01:01' -- wait 1min 1 second IF @rowcnt > 1 -- when this happens we have a problem BEGIN
SELECT @message = 'More than 1 myGroupID ' + COALESCE(CAST(@myGroupID_out AS VARCHAR(100)), 'null ') SELECT @message = @message + COALESCE(CAST(id AS VARCHAR(100)), '-') + ',' FROM @list SELECT @myGroupID_out = NULL
RAISERROR (@message , 16 , 1) END ELSE IF @rowcnt = 1 BEGIN PRINT 'be happy' END ELSE IF @rowcnt = 0 BEGIN
-- or ?: SELECT @myGroupID_out = myGroupID + 1 FROM myGroup WITH (UPDLOCK)
SELECT @myGroupID_out = @newgroupid
INSERT INTO myGroup (myGroupID) VALUES (@newgroupID)
-- INSERT INTO dbo.myGroup -- DEFAULT VALUES ; -- SELECT @myGroupID_out = SCOPE_IDENTITY() ;
INSERT INTO dbo.myGroupMember SELECT @myGroupID_out gid, id FROM @list ; END ; COMMIT TRAN END TRY BEGIN CATCH ROLLBACK TRAN SELECT @myGroupID_out = NULL --EXEC raise_error SELECT ERROR_MESSAGE() END CATCH --SET TRANSACTION ISOLATION LEVEL READ COMMITTED END ; GO
Erland Sommarskog - 14 Mar 2008 15:10 GMT > I made an new example below, but I didn't fully understand why to > select on myGroup only with UPDLOCK, why not on myGroupMember also? > But it seems to work. Before I fix this in production: please explain! An Update lock is a read-lock. That is, it does not block readers. But you cannot get an update-lock on a resource if there is an exclusive lock on it (of course), and neither can you get an update-lock on a resource if there is already an update-lock on it.
The resource here in this case is the current max value of myGroupID. If you have read 5, the lock guarantees that you will read 5 next time you read the row as well.
However, the lock itself does not guarantee that SELECT MAX will return the same value next time. Someone could insert an ID of 87 outside the procedure and not be blocked by this update-lock. To prevent this, you would need HOLDLOCK as well, that is serializable. But since serializable is very prone to deadlocks, this is something you want to avoid. And it seems a very reasonable assumption that the only piece of code that ever will insert rows into myGroups is this procedure.
You don't need any such locks on myGroupMembers, since by having this lock in the beginning you have created a serialization point. And it's difficult to grab a meaningful lock in myGroupMembers without using serializable.
While the technique with UPDLOCK is common, one could argue that in this case it would be cleaner to use an application lock instead.
 Signature Erland Sommarskog, SQL Server MVP, esquel@sommarskog.se
Books Online for SQL Server 2005 at http://www.microsoft.com/technet/prodtechnol/sql/2005/downloads/books.mspx Books Online for SQL Server 2000 at http://www.microsoft.com/sql/prodinfo/previousversions/books.mspx
B D Jensen - 14 Mar 2008 18:21 GMT Hi Erland! "And it seems a very reasonable assumption that the only piece of code that ever will insert rows into myGroups is this procedure. " YES, all create-access via this proc.
"you have created a serialization point" So SELECT @groupID = myGroupID FROM myGroup WITH (UPDLOCK) -- select only for lock purpose also "locks" myGroupMember? I suppose it would not be enough to write: select null WITH (UPDLOCK)
This is 1. time I heard about "application lock " I found this example: http://www.sqlteam.com/article/application-locks-or-mutexes-in-sql-server-2005 I suppose then I should use @LockMode = 'Update'
Thank you very much. I expect to update this tread at the end of this month, after I have released a fix and jobs had run for a while, then I can tell, it the problem go away i production also... Best regards Bjorn
Erland Sommarskog - 15 Mar 2008 10:34 GMT > "you have created a serialization point" > So > SELECT @groupID = myGroupID > FROM myGroup WITH (UPDLOCK) -- select only for lock purpose > also "locks" myGroupMember? There will be no physical lock on myGroupMember, but since this is the only procedure that will write to the table, logically there is a lock.
> I suppose it would not be enough to write: > select null WITH (UPDLOCK) No, because NULL is not a resource you can lock. But theoretically you could use another table, but that would be a bit obscure.
> This is 1. time I heard about "application lock " > I found this example: > http://www.sqlteam.com/article/application-locks-or-mutexes-in-sql-server-2005 > I suppose then I should use @LockMode = 'Update' Or Exclusive. You also need @owner = 'Transaction' (which I think is the default). Application locks can be session-bound which can be useful at times.
 Signature Erland Sommarskog, SQL Server MVP, esquel@sommarskog.se
Books Online for SQL Server 2005 at http://www.microsoft.com/technet/prodtechnol/sql/2005/downloads/books.mspx Books Online for SQL Server 2000 at http://www.microsoft.com/sql/prodinfo/previousversions/books.mspx
B D Jensen - 16 Mar 2008 12:01 GMT Hi! I now tried with applock, but: 1. session completes other session are blocked (after 10minutes i stopped all)
Whats wrong in code below? /Bjorn
--------------------------------------------------------------------- CREATE PROCEDURE createMyGroup @list_in VARCHAR(MAX) -- #param list of id's , @myGroupID_out INT OUTPUT -- #param pseudo key AS /* Description: variation of sql divide Change History: $Date: $ , $Author: bdj $, $Revision: 0 $ */ BEGIN SET NOCOUNT ON --SET TRANSACTION ISOLATION LEVEL SERIALIZABLE DECLARE @rowcnt INT ; DECLARE @list TABLE (id INT PRIMARY KEY) ; -- important constraint! DECLARE @message VARCHAR(100) DECLARE @groupid INT DECLARE @res INT
SELECT @myGroupID_out = NULL ; -- real output -- exec utility.debug_info '.....'
BEGIN TRY BEGIN TRANSACTION
EXEC @res = sp_getapplock @resource = 'myGroup', @lockMode = 'Update', @lockOwner = 'Transaction', @lockTimeOut = 5000 -- 5000ms
-- SELECT @groupID = myGroupID -- FROM myGroup WITH (UPDLOCK) -- select only for lock purpose
INSERT INTO @list SELECT CONVERT(INT, s.data) FROM DBO.split(@list_in, ',') s
SELECT @myGroupID_out = a2.myGroupID FROM dbo.myGroupMember a2 WHERE EXISTS ( SELECT NULL FROM @list b1 WHERE b1.id = a2.ID ) GROUP BY a2.myGroupID HAVING (SELECT COUNT (*) FROM myGroupMember a3 WHERE a3 . myGroupID = a2 . myGroupID) = (SELECT COUNT (*) FROM @list) AND COUNT(*) = (SELECT COUNT (*) FROM @list)
SELECT @rowcnt = @@ROWCOUNT WAITFOR DELAY '00:01:01' -- wait 1min 1 second IF @rowcnt > 1 -- when this happens we have a problem BEGIN
SELECT @message = 'More than 1 myGroupID ' + COALESCE(CAST(@myGroupID_out AS VARCHAR(100)), 'null ') SELECT @message = @message + COALESCE(CAST(id AS VARCHAR(100)), '-') + ',' FROM @list SELECT @myGroupID_out = NULL
RAISERROR (@message , 16 , 1) END ELSE IF @rowcnt = 1 BEGIN PRINT 'be happy' END ELSE IF @rowcnt = 0 BEGIN IF @res NOT IN (0,1) BEGIN RAISERROR('Unable to acquire lock', 16, 1) END ELSE BEGIN INSERT INTO dbo.myGroup DEFAULT VALUES ; SELECT @myGroupID_out = SCOPE_IDENTITY() ;
INSERT INTO dbo.myGroupMember SELECT @myGroupID_out gid, id FROM @list ; END END ; EXEC @res = sp_releaseappLock @resource = 'myGroup', @lockOwner = 'Transaction' COMMIT TRANSACTION END TRY BEGIN CATCH ROLLBACK TRANSACTION SELECT @myGroupID_out = NULL --EXEC raise_error SELECT ERROR_MESSAGE() END CATCH --SET TRANSACTION ISOLATION LEVEL READ COMMITTED END ; GO
Erland Sommarskog - 16 Mar 2008 12:46 GMT > I now tried with applock, but: > 1. session completes > other session are blocked (after 10minutes i stopped all) > > Whats wrong in code below? I tested it, and I did not see the problem you describe, but it worked as expected. I had to clean up your procedure bit, because I use a case-sensitive collation. I don't think that is the issue though. My guess is that you made the classical mistake of pressing the red button to cancel the batch while it was running, and then forgot to rollback the open transaction.
For the record, here my version that also works with a case-sensitive collation:
CREATE PROCEDURE createMyGroup @list_in VARCHAR(MAX) -- #param list of id's , @myGroupID_out INT OUTPUT -- #param pseudo key AS /* Description: variation of sql divide Change History: $Date: $ , $Author: bdj $, $Revision: 0 $ */ BEGIN SET NOCOUNT ON --SET TRANSACTION ISOLATION LEVEL SERIALIZABLE DECLARE @rowcnt INT ; DECLARE @list TABLE (id INT PRIMARY KEY) ; -- important constraint! DECLARE @message VARCHAR(100) DECLARE @groupid INT DECLARE @res INT
SELECT @myGroupID_out = NULL ; -- real output -- exec utility.debug_info '.....'
BEGIN TRY BEGIN TRANSACTION
EXEC @res = sp_getapplock @Resource = 'myGroup', @LockMode = 'Update', @LockOwner = 'Transaction', @LockTimeout = 5000 -- 5000ms
-- SELECT @groupID = myGroupID -- FROM myGroup WITH (UPDLOCK) -- select only for lock purpose
INSERT INTO @list SELECT CONVERT(INT, s.data) FROM dbo.split(@list_in, ',') s
SELECT @myGroupID_out = a2.myGroupID FROM dbo.myGroupMember a2 WHERE EXISTS ( SELECT NULL FROM @list b1 WHERE b1.id = a2.ID ) GROUP BY a2.myGroupID HAVING (SELECT COUNT (*) FROM myGroupMember a3 WHERE a3 . myGroupID = a2 . myGroupID) = (SELECT COUNT (*) FROM @list) AND COUNT(*) = (SELECT COUNT (*) FROM @list)
SELECT @rowcnt = @@ROWCOUNT -- WAITFOR DELAY '00:01:01' -- wait 1min 1 second IF @rowcnt > 1 -- when this happens we have a problem BEGIN
SELECT @message = 'More than 1 myGroupID ' + COALESCE(CAST(@myGroupID_out AS VARCHAR(100)), 'null ') SELECT @message = @message + COALESCE(CAST(id AS VARCHAR(100)), '-') + ',' FROM @list SELECT @myGroupID_out = NULL
RAISERROR (@message , 16 , 1) END ELSE IF @rowcnt = 1 BEGIN PRINT 'be happy' END ELSE IF @rowcnt = 0 BEGIN IF @res NOT IN (0,1) BEGIN RAISERROR('Unable to acquire lock', 16, 1) END ELSE BEGIN INSERT INTO dbo.myGroup DEFAULT VALUES ; SELECT @myGroupID_out = SCOPE_IDENTITY() ;
INSERT INTO dbo.myGroupMember SELECT @myGroupID_out gid, id FROM @list ; END END ; EXEC @res = sp_releaseapplock @Resource = 'myGroup', @LockOwner = 'Transaction' COMMIT TRANSACTION END TRY BEGIN CATCH ROLLBACK TRANSACTION SELECT @myGroupID_out = NULL --EXEC raise_error SELECT ERROR_MESSAGE() END CATCH --SET TRANSACTION ISOLATION LEVEL READ COMMITTED END ; GO
 Signature Erland Sommarskog, SQL Server MVP, esquel@sommarskog.se
Books Online for SQL Server 2005 at http://www.microsoft.com/technet/prodtechnol/sql/2005/downloads/books.mspx Books Online for SQL Server 2000 at http://www.microsoft.com/sql/prodinfo/previousversions/books.mspx
B D Jensen - 17 Mar 2008 16:48 GMT Hi! I activated the line with "wait for delay" again (to simulate the case that statement before takes time, du to high load, big table etc) Now using the example one session gives me an id, but the others say: "unable to acquire lock"... /Bjorn
B D Jensen - 30 Mar 2008 13:42 GMT Hi! In the meantime I found out that most of the actual problems came from another corner: an FK with "on cascade delete", which result in dublet groups. But the problem is still actual since I plan to scale- up the application; but before doing that I must ensure that it will be able to handle more concurrency.
Erland presented 2 good solutions here: 1. updlock 2. application lock
At another place he also suggested use of Service Broker - and I expect that use of SB will be better than my DYI of queues (even if I think my do-yourself-implementation is simpler).
The reason why got these "unable to acquire lock" is that I had not attention on the fact that my sleep step took 61s, but the application lock was set to 50s - so of course I must get this error. The sleep step was to ensure simulate some form of "stress" on system.
Here the final code that now works as I expect, and thanks to people for their help (-:
DROP TABLE myGroup ;
CREATE TABLE myGroup ( [myGroupID] [int] IDENTITY(1, 1) NOT NULL , CONSTRAINT [PK_myGroup] PRIMARY KEY CLUSTERED ([myGroupID] ASC) ) ;
DROP TABLE myGroupMember ;
CREATE TABLE myGroupMember ( [myGroupMemberID] [int] IDENTITY(1, 1) NOT NULL , [myGroupID] [int] NOT NULL , [ID] [int] NOT NULL , CONSTRAINT [PK_myGroupMember] PRIMARY KEY CLUSTERED ([myGroupMemberID] ASC) , CONSTRAINT [UK_myGroupMember_myGroupID_ID] UNIQUE NONCLUSTERED ([myGroupID] ASC, [ID] ASC) -- not enough because this will not avoid dublet lists ) ; -- FK connecting myGroupMember.myGroupID with myGroup.myGroupID... CREATE UNIQUE INDEX UK_myGroupMember_ID_myGroupID ON myGroupMember (ID, myGroupID);
DROP PROC createMyGroup GO
CREATE PROCEDURE createMyGroup @list_in VARCHAR(MAX) -- $param list of id's , @myGroupID_out INT OUTPUT -- $param pseudo key AS /* Description: variation of sql divide Change History: $Date: $ , $Author: bdj $, $Revision: 0 $ */ BEGIN SET NOCOUNT ON --SET TRANSACTION ISOLATION LEVEL SERIALIZABLE DECLARE @rowcnt INT ; DECLARE @list TABLE (id INT PRIMARY KEY) ; -- important constraint! DECLARE @message VARCHAR(100) DECLARE @groupid INT DECLARE @res INT
SELECT @myGroupID_out = NULL ; -- real output -- exec utility.debug_info '.....'
BEGIN TRY BEGIN TRANSACTION
EXEC @res = sp_getapplock @Resource = 'myGroup', @LockMode = 'Update', @LockOwner = 'Transaction', @LockTimeout =100000 -- 100000ms = 100s
-- SELECT @groupID = myGroupID -- FROM myGroup WITH (UPDLOCK) -- select only for lock purpose
INSERT INTO @list SELECT CONVERT(INT, s.data) FROM dbo.split(@list_in, ',') s
SELECT @myGroupID_out = a2.myGroupID FROM dbo.myGroupMember a2 WHERE EXISTS ( SELECT NULL FROM @list b1 WHERE b1.id = a2.ID ) GROUP BY a2.myGroupID HAVING (SELECT COUNT (*) FROM myGroupMember a3 WHERE a3 . myGroupID = a2 . myGroupID) = (SELECT COUNT (*) FROM @list) AND COUNT(*) = (SELECT COUNT (*) FROM @list)
SELECT @rowcnt = @@ROWCOUNT WAITFOR DELAY '00:01:01' -- wait 1min 1 second = 61 s IF @rowcnt > 1 -- when this happens we have a problem BEGIN
SELECT @message = 'More than 1 myGroupID ' + COALESCE(CAST(@myGroupID_out AS VARCHAR(100)), 'null ') SELECT @message = @message + COALESCE(CAST(id AS VARCHAR(100)), '-') + ',' FROM @list SELECT @myGroupID_out = NULL
RAISERROR (@message , 16 , 1) END ELSE IF @rowcnt = 1 BEGIN PRINT 'be happy' END ELSE IF @rowcnt = 0 BEGIN IF @res NOT IN (0,1) BEGIN RAISERROR('Unable to acquire lock', 16, 1) END ELSE BEGIN INSERT INTO dbo.myGroup DEFAULT VALUES ; SELECT @myGroupID_out = SCOPE_IDENTITY() ;
INSERT INTO dbo.myGroupMember SELECT @myGroupID_out gid, id FROM @list ; END END ; EXEC @res = sp_releaseapplock @Resource = 'myGroup', @LockOwner = 'Transaction' COMMIT TRANSACTION END TRY BEGIN CATCH ROLLBACK TRANSACTION SELECT @myGroupID_out = NULL --EXEC raise_error SELECT ERROR_MESSAGE() END CATCH --SET TRANSACTION ISOLATION LEVEL READ COMMITTED END ; GO
DECLARE @list VARCHAR(MAX), @myGroupID INT EXEC createMyGroup '1000,2000,3000,4000', @myGroupID OUTPUT SELECT @myGroupID
-- from more than 1 session simultaneous: WAITFOR TIME '14:33'; DECLARE @list VARCHAR(MAX), @myGroupID INT EXEC createMyGroup '1000,2000,3000,4000,5008', @myGroupID OUTPUT SELECT @myGroupID
Best regards Bjorn D. Jensen P. S. Feel free to use this code example for simulating concurrency problems ;-)
B D Jensen - 14 Mar 2008 11:05 GMT and here the alternative solution using indentity column
DROP TABLE myGroup ;
CREATE TABLE myGroup ( [myGroupID] [int] IDENTITY(1, 1) NOT NULL , CONSTRAINT [PK_myGroup] PRIMARY KEY CLUSTERED ([myGroupID] ASC) ) ;
DROP TABLE myGroupMember ;
CREATE TABLE myGroupMember ( [myGroupMemberID] [int] IDENTITY(1, 1) NOT NULL , [myGroupID] [int] NOT NULL , [ID] [int] NOT NULL , CONSTRAINT [PK_myGroupMember] PRIMARY KEY CLUSTERED ([myGroupMemberID] ASC) , CONSTRAINT [UK_myGroupMember_myGroupID_ID] UNIQUE NONCLUSTERED ([myGroupID] ASC, [ID] ASC) -- not enough because this will not avoid dublet lists ) ; -- FK connecting myGroupMember.myGroupID with myGroup.myGroupID... CREATE UNIQUE INDEX UK_myGroupMember_ID_myGroupID ON myGroupMember (ID, myGroupID);
DROP PROC createMyGroup GO
CREATE PROCEDURE createMyGroup @list_in VARCHAR(MAX) -- #param list of id's , @myGroupID_out INT OUTPUT -- #param pseudo key AS /* Description: variation of sql divide Change History: $Date: $ , $Author: bdj $, $Revision: 0 $ */ BEGIN SET NOCOUNT ON --SET TRANSACTION ISOLATION LEVEL SERIALIZABLE DECLARE @rowcnt INT ; DECLARE @list TABLE (id INT PRIMARY KEY) ; -- important constraint! DECLARE @message VARCHAR(100) DECLARE @groupid INT
SELECT @myGroupID_out = NULL ; -- real output -- exec utility.debug_info '.....'
BEGIN TRY BEGIN TRAN
SELECT @groupID = myGroupID FROM myGroup WITH (UPDLOCK) -- select only for lock purpose
INSERT INTO @list SELECT CONVERT(INT, s.data) FROM DBO.split(@list_in, ',') s
SELECT @myGroupID_out = a2.myGroupID FROM dbo.myGroupMember a2 WHERE EXISTS ( SELECT NULL FROM @list b1 WHERE b1.id = a2.ID ) GROUP BY a2.myGroupID HAVING (SELECT COUNT (*) FROM myGroupMember a3 WHERE a3 . myGroupID = a2 . myGroupID) = (SELECT COUNT (*) FROM @list) AND COUNT(*) = (SELECT COUNT (*) FROM @list)
SELECT @rowcnt = @@ROWCOUNT WAITFOR DELAY '00:01:01' -- wait 1min 1 second IF @rowcnt > 1 -- when this happens we have a problem BEGIN
SELECT @message = 'More than 1 myGroupID ' + COALESCE(CAST(@myGroupID_out AS VARCHAR(100)), 'null ') SELECT @message = @message + COALESCE(CAST(id AS VARCHAR(100)), '-') + ',' FROM @list SELECT @myGroupID_out = NULL
RAISERROR (@message , 16 , 1) END ELSE IF @rowcnt = 1 BEGIN PRINT 'be happy' END ELSE IF @rowcnt = 0 BEGIN INSERT INTO dbo.myGroup DEFAULT VALUES ; SELECT @myGroupID_out = SCOPE_IDENTITY() ;
INSERT INTO dbo.myGroupMember SELECT @myGroupID_out gid, id FROM @list ; END ; COMMIT TRAN END TRY BEGIN CATCH ROLLBACK TRAN SELECT @myGroupID_out = NULL --EXEC raise_error SELECT ERROR_MESSAGE() END CATCH --SET TRANSACTION ISOLATION LEVEL READ COMMITTED END ; GO
WAITFOR TIME '10:51'; DECLARE @list VARCHAR(MAX), @myGroupID INT EXEC createMyGroup '100,200,300,400,409', @myGroupID OUTPUT SELECT @myGroupID
--CELKO-- - 13 Mar 2008 20:48 GMT >> So if you first enter {1,2,3} and then {3,2,1} that will be the same list? But {1,2,3,4} is a new list? << That is a classic Todd's division problem.
CREATE TABLE Lists (list_name CHAR(2) NOT NULL, list_member INTEGER NOT NULL, PRIMARY KEY (list_name, list_member));
INSERT INTO Lists VALUES ('A', 1), ('A', 2), ('A', 3), ('B', 1), ('B', 2), ('B', 3), ('C', 1), ('C', 2), ('C', 3), ('C', 4), ('D', 1), ('D', 2), ('D', 3), ('D', 5);
SELECT DISTINCT L1.list_name, L2.list_name FROM Lists AS L1, Lists AS L2 WHERE L1.list_name < L2.list_name --different lists -- same size lists for a quick test AND ((SELECT COUNT(*) FROM Lists AS L3 WHERE L3.list_name = L1.list_name) = (SELECT COUNT(*) FROM Lists AS L4 WHERE L4.list_name = L2.list_name)) -- one to one mapping of elements AND ((SELECT COUNT(*) FROM Lists AS L3 WHERE L3.list_name = L1.list_name) = (SELECT COUNT(*) FROM Lists AS L5 FULL OUTER JOIN Lists AS L6 ON L5.list_name = L1.list_name AND L6.list_name = L2.list_name AND L5.list_member = L6.list_member));
B D Jensen - 13 Mar 2008 21:32 GMT Hi Joe! Your insert statement: is this iso syntax not implemented by MS? - It results in syntax error on MSSQL, so i did this rewrite of your statement: INSERT INTO Lists SELECT 'A' a, 1 b UNION ALL SELECT 'A', 2 UNION ALL SELECT 'A', 3 UNION ALL SELECT 'B', 1 UNION ALL SELECT 'B', 2 UNION ALL SELECT 'B', 3 UNION ALL SELECT 'C', 1 UNION ALL SELECT 'C', 2 UNION ALL SELECT 'C', 3 UNION ALL SELECT 'C', 4 UNION ALL SELECT 'D', 1 UNION ALL SELECT 'D', 2 UNION ALL SELECT 'D', 3 UNION ALL SELECT 'D', 5 ;
running your select: result in no rows... (your relationel divide statement - I know relation division is part of the solution here, thats why I wrote "Description: variation of sql divide ")
The problem is not the correct identification of lists. Identification works fine The problem is concurrency, this should be clear when you run my example. When two concurrent session both tries to create the same list then either I got error (deadlock), or I got the lists created twice.'
How to solve that?? Best regards Bjorn
--CELKO-- - 13 Mar 2008 21:54 GMT >> Is this ANSI/ISO syntax not implemented by MS? << Nope, not yet. Wait for 2008.
>> When two concurrent session both try to create the same list then either I got error (deadlock), or I got the lists created twice. How to solve that?? << Since we don't have a lot of stuff in SQL Server, one idea might be to create a stored procedure that takes a long parameter list (list_name, v1, v2, ..vn), puts the non-NULL parameter values in a working table, and does the Todd division. If there is a matching list, then return a duplication error (the name of the existing list would be useful) else we insert the working table and commit it. But you will have to watch the isolation level.
A solution I don't like is to permit the duplicate lists and clean them out later with a DELETE FROM. Ugh!
B D Jensen - 14 Mar 2008 11:14 GMT Hi Joe! We have the year 2008... ;^) I know what you mean... I have SS2005.
"Since we don't have a lot of stuff in SQL Server" you are right - which one do you prefer?
"create a stored procedure that takes a long parameter list ...and does the Todd division" Thats what I'm trying to make here ;-)
Haan & Koppelaars wrote a book, where they discuss "multi row constraints" Do you think we can expect this from db in near future?
Greetings Bjorn
--CELKO-- - 14 Mar 2008 16:21 GMT >> Haan & Koppelaars wrote a book, where they discuss "multi row constraints" Do you think we can expect this from db in near future? << I will have to look up that book. Technically, this is already in the Standards, since you are supposed to be able to put any search condition in a CHECK().
B D Jensen - 14 Mar 2008 18:21 GMT ... but unfornutate, not all good ideas from the standard are implemented yet... /Bjorn
Tony Rogerson - 14 Mar 2008 20:27 GMT > Since we don't have a lot of stuff in SQL Server, one idea might be to > create a stored procedure that takes a long parameter list (list_name, > v1, v2, ..vn), puts the non-NULL parameter values in a working table, > and does the Todd division. If there is a matching list, then return I see Celko is back to pushing his 1,000 parameter solution to passing an array of values.
Celko - you have not addressed nor answered a number of outstanding questions about your proposed solution, why do you keep ignoring these real world questions?
You proposed this...
WITH Parmlist(parm) --p# is an input parameter AS (VALUES (CAST (p01 AS DECIMAL(6,3)), (p02), .., (p99)))
SELECT a, b, c FROM Foobar WHERE x IN (SELECT DISTINCT parm -- can add expressions here, like UPPER(parm) FROM ParmList WHERE parm IS NOT NULL AND <<other stuff> );
Let's start summarising what is wrong with the above....
1) Requires database and application code change when the list of possible values increases - give that there could be dozens, may be 100's of applications using those stored procedures that is a very big change and resource requirement in the test, sync and release cycle. 2) The above does not work on SQL Server 3) It does not verify if values are in a specific range for instance 0 - 9 (the suduko puzzle you keep posting) 4) The optimiser has no clue as to how many rows have been passed so query plans will be effected possibly resulting in poor performance
You have been asked several times to post a working example that validates each parameter is between 0 and 9 - you gave this...
> SELECT SUM (ParmList.parm) -- i had to do something > FROM (VALUES [quoted text clipped - 5 lines] > AS ParmList(parm) > WHERE parm BETWEEN 0 AND 9); This simply sum's the values from the valid rows - that's all; it does no verification that the values are between 0 and 9 - where is the assertion when a value is out of range? If all rows passed where 0 then you'd get 0; if 1 row passed was 0 and one was 90 then you'd still think you had no invalid values.
For code maintanence you state this...
> Tony, if I add, remove or change the data type of a parameter in > **any** stored procedure, I have to re-deploy it. We have gotten [quoted text clipped - 6 lines] > might not cause a problem because of automatic casting, so you need to > watch out. You obvisouly think the development life cycle only contains these steps...
Bulk edit source code Use macro to make change Deploy
I questioned this and am still waiting for an answer.
Well celko; there is just a few steps missing with that - would you care to point out which ones and how much resource in percentage terms each step in the cycle will take? Development 101 - once the bug has found it's way into production it's orders of magnitude more costly to fix.
If you are going to come up with these wild and "wonderful" solutions you need to be able to justify them OUT - OF - THE - CLASS - ROOM.
 Signature Tony Rogerson, SQL Server MVP http://sqlblogcasts.com/blogs/tonyrogerson [Ramblings from the field from a SQL consultant] http://sqlserverfaq.com [UK SQL User Community]
--CELKO-- - 12 Mar 2008 04:02 GMT Have you considered a normalized database instead of a non-1NF hand- made monster, with things like split() in it?
B D Jensen - 12 Mar 2008 06:24 GMT Hi Joe! The tables myGroup and myGroupMember are normalized. And actually the purpose is to avoid dublet groups in myGroupMember
The reason for using split is to handle input with at list, since SS2005 not gives oppurtunity to exchange collections (table variables). Another solution i often use for small list are xml's, but that is of no interest here.
Another table solution could be (then there is no need for myGroupMember): myGroup(groupID int identity(1,1), str varchar(max))
storing the input list (ensuring that it is sorted), but making an UK on str will cause trouble... Best regards Bjorn
Ed Murphy - 12 Mar 2008 08:31 GMT > Have you considered a normalized database instead of a non-1NF hand- > made monster, with things like split() in it? The tables themselves are 1NF, it's the procedure logic that gets to be a mess.
Bjorn, what is your conceptual intent here? To reject attempts to insert a group of values that exactly matches (ignoring order) an existing group? Would allowing duplicate groups cause any problems other than increasing the size of the database?
B D Jensen - 12 Mar 2008 12:07 GMT Hi Ed! Regarding the intend: you probably now have seen them in my answer above ;-) It also gives performance by design, since a smaller table tend to be better.
Of course: I could remove the level setting, but as described then I got duplicate lists. I would prefer to avoid this, because with non dublet lists it will be easy to see in table A which rows are using same groups.
Independent of my concrete problem I also would like to get some ideas about how to implement "multiple row constraint" - and the code example shown, is usefull for that ;-)
I see forward to receive ideas (code examples if possible :-) from all of you!
Best regards Bjorn
|
|
|