September 26, 2001 at 4:27 am
HI all,
I have a stopred prodedure with a transaction in it , which includes some static and dynamically made statemenst using EXEC (). When a client executes the proc through a connection , it lock resourses. Obviousely if the client crashes before committing , the resource would be locked until admin kills the connection.
The problem is that , I don't want clients to lock any resource in the proc. I tried all table hints and isolation hints , but they didn't work. How can I tell the proc not to lock tables?
Yours,
A. Sepahram
September 26, 2001 at 4:57 am
can you post the code!!
also what is the client app developed in??
October 2, 2001 at 1:14 am
The lock manager sometimes take it's decision even if you've specified the lock hint. I develop an OLTP system and got experiance in SQL Server 7 locking issues.
Anyhow try "readuncommitted" or "nolock" hints in your queries and use "sp_executesql" instead "execute", that might solve your problems.
As about the crashed client should just keep the connection open till is killed, not keep the sql server process still.
October 4, 2001 at 3:27 am
Than you very much , but it didnt' work yet!
It seems SQL Server does what it likes instead of what developer tells it 🙂
Amir
October 4, 2001 at 6:10 am
October 5, 2001 at 11:57 pm
This is a particularly alarming statement:
quote:
I have a stopred prodedure with a transaction in it , which includes some static and dynamically made statemenst using EXEC (). When a client executes the proc through a connection , it lock resourses. Obviousely if the client crashes before committing , the resource would be locked until admin kills the connection.
As Andy has asked, we do need to see the code to see locking hints, etc. However, is the sproc running for such an extended period of time that you have to consider for a client crash? Also, are client crashes frequent enough to cause this to be of such concern you're willing to bypass locking?
K. Brian Kelley
http://www.sqlservercentral.com/columnists/bkelley/
K. Brian Kelley
@kbriankelley
October 6, 2001 at 5:25 am
Here's the code :
CREATE PROC PRC_SYS_GetLastID
( @TableName sysname ,
@IDField sysname ,
@WhereClause sysname ,
@LastID sysname OUTPUT )
AS
SET @WhereClause = IsNull(@WhereClause, '')
-- Drops ##SeqTemp temp table if it exists in advance
IF EXISTS ( SELECT * from tempdb..sysobjects WHERE name = '##SeqTemp' )
DROP TABLE ##SeqTemp
DECLARE @TempID sysname ,
@SqlStr varchar (2126) ,-- SQL statement to be executed
@UpperBound sysname,-- The upper bound based on data type of @IDField
@WhereStr Varchar(256)-- Used to make WHERE and GROUP BY clauses
-- Finds the upper bound based on data type of @IDField
SELECT @UpperBound = CASE data_type WHEN 'tinyint' THEN '255'
WHEN 'smallint' THEN '32767'
ELSE '0' END
FROM information_schema.columns
WHERE table_name = @TableName
AND column_name = @IDField
IF @UpperBound IS NULL
BEGIN
SET @LastID = '-1'-- Ends and returns -1 if a the last ID exceeds datatype limit
RETURN
END
IF @WhereClause = ''
SET @WhereStr = ''
ELSE
--SET @WhereClause = Replace(@WhereClause, ' ' , '')
SET @WhereStr = ' WHERE ' + Replace(@WhereClause, ',', ' AND ')
SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED
BEGIN TRAN
-- Deletes last used ID of given field of given table
SET @SqlStr = 'DELETE TBC_SYS_LastID ' +
' WHERE TableName = ''' + @TableName + ''' AND WhereClause = ''' + @WhereClause +
''' AND LastID IN ( SELECT ' + @IDField + ' FROM ' + @TableName + @WhereStr + ' )'
EXEC ( @SqlStr )
------------------
IF @@ERROR <> 0
GOTO errLabel
------------------
-- compares the given table with TBC_SYS_IDSequence to finds the gaps or the last free ID
IF @UpperBound <> '0'-- If data type isn't INT
BEGIN
SET @SqlStr = 'SELECT
Min( SeqNO ) newLastID INTO ##SeqTemp
FROM
( SELECT SeqNO
FROM ( SELECT SeqNO
FROM TBC_SYS_IDSequence
WHERE SeqNO <= ' + @UpperBound + ' AND SeqNO NOT IN ( SELECT LastID
FROM TBC_SYS_LastID
WHERE TableName = ''' + @TableName + ''' AND
WhereClause = ''' + @WhereClause + ''') ) A
WHERE SeqNO NOT IN (SELECT ' + @IDField + ' FROM ' + @TableName + @WhereStr + ')) B'
EXEC ( @SqlStr )
------------------
IF @@ERROR <> 0
GOTO errLabel
------------------
SELECT @LastID = newLastID FROM ##SeqTemp
------------------
IF @@ERROR <> 0
GOTO errLabel
------------------
END --If UpperBound
ELSE-- If data type is INT
BEGIN
DECLARE @EQPos TinyInt ,
@ComaPos TinyInt ,
@GroupByStr Varchar(256) ,
@TempStr Varchar(256)
-- Avoid making a string NULL and check for WHERE clause
IF @WhereClause = ''-- If there isn't a WHERE clause
BEGIN
SET @WhereStr = ''
SET @GroupByStr = ''
END
ELSE-- If there's a WHERE clause
BEGIN
SET @WhereStr = @WhereClause
SET @GroupByStr = ' GROUP BY '-- Makes GROUP BY clause
SET @TempStr = @WhereStr + ','
WHILE 1 = 1
BEGIN
SET @EQPos = PatIndex( '%=%', @TempStr)
IF @EQPos = 0
BREAK
SET @ComaPos = PatIndex( '%,%', @TempStr)
SET @TempStr = Replace(@TempStr, SubString(@TempStr, @EQPos , @ComaPos - @EQPos), '')
SET @ComaPos = PatIndex( '%,%', @TempStr)
SET @GroupByStr = IsNull(@GroupByStr, '') + SubString(@TempStr, 1, @Comapos)
SET @TempStr = SubString(@TempStr, @Comapos+1, Len(@TempStr))
END
END
-- Makes GROUP BY clause
IF @WhereClause <> ''-- If there isn't a WHERE clause
BEGIN
SET @GroupByStr = SubString(@GroupByStr, 1, Len(@GroupByStr) - 1)
SET @WhereStr = ' WHERE ' + Replace(@WhereStr, ',', ' AND ')
END
------------------
IF @@ERROR <> 0
GOTO errLabel
------------------
-- Makes a virtual field list to match the field list of TBC_SYS_LasiID
-- @TableName in order to find unused MAX (@IDField) for int data type
DECLARE @TempWhereStr varchar(1000), @TempPlace varchar(1000),
@VirtualFieldList varchar(1000),
@val Varchar(256), @FieldID Varchar(256),@RealFieldList Varchar(1000)
SET @TempWhereStr = @WhereClause + ','
SET @VirtualFieldList = ''
IF @WhereClause = ''
SET @RealFieldList = ''
ELSE
SET @RealFieldList = ',' + Replace(@GroupByStr, 'GROUP BY', '')
IF @WhereClause <> ''
WHILE 1=1
BEGIN
SET @EqPos = Patindex('%=%', @TempWhereStr)
SET @ComaPos = Patindex('%,%', @TempWhereStr)
IF @Comapos = 0
BREAK
SET @TempPlace = SubString(@TempWhereStr, 1, @ComaPos - 1)
SET @FieldID = Substring(@TempPlace, 1, @Eqpos -1)
SET @val = Substring(@TempPlace, @Eqpos +1 , Len(@TempPlace))
SET @VirtualFieldList = @VirtualFieldList + ', ' + @val + ' ' + @FieldID
SET @TempWhereStr = SubString(@TempWhereStr, @ComaPos+ 1, Len(@TempWhereStr))
END
-- Returns the max of the found last IDs
SELECT @SqlStr = 'SELECT ' +
' MAX(' + @IDField + ') + 1 newLastID INTO ##SeqTemp ' +
' FROM ' +
' (SELECT ' +
@IDField + @RealFieldList +
'FROM ' + @TableName +
@WhereStr +
'UNION ' +
'SELECT ' +
'LastID ' + @IDField + @VirtualFieldList +
'FROM TBC_SYS_LastID WHERE TableName = ''' +
@TableName + ''' AND WhereClause = ''' + @WhereClause + ''') A ' +
@GroupByStr
EXEC ( @SqlStr )
------------------
IF @@ERROR <> 0
GOTO errLabel
------------------
SELECT @LastID = newLastID FROM ##SeqTemp
END --Else UpperBound
-- Inserts the found last ID into TBC_SYS_LastID in order to prevent concurrency
SET @SqlStr = 'INSERT INTO TBC_SYS_LastID WITH ( ROWLOCK ) ( TableName, WhereClause, LastID ) VALUES ('''
+ @TableName + ''', ''' + @WhereClause + ''' , ' + @LastID + ') '
EXEC ( @SqlStr )
------------------
IF @@ERROR <> 0
GOTO errLabel
------------------
-- Drops ##SeqTemp temp table if it already exists in advance
IF EXISTS ( SELECT * from tempdb..sysobjects WHERE name = '##SeqTemp' )
DROP TABLE ##SeqTemp
COMMIT TRAN
RETURN
-- Rolls back what's done in the whole proc , if there's a problem somewhere
errLabel:
ROLLBACK TRAN
IF EXISTS ( SELECT * from tempdb..sysobjects WHERE name = '##SeqTemp' )
DROP TABLE ##SeqTemp
SET @LastID = '0'-- Returns 0 if a problem occured and the trans rolled back
October 6, 2001 at 5:47 am
Brian,
The events you mentioned probably rarely take palce. However, the point is that it's a central procedure and so critical. Because, it's a general proc to get last valid ID for given table based on given condition. Therefore, I have to be sure that it'll never locked the tables which many connections may use concurently.
On the other hand, don't you think as an admin or developer I must able to tell explicitely to DB not to lock a resource?
Amir
October 6, 2001 at 6:07 am
That code gives me a headache. I think you're doing a lot of work just to make sure there are no gaps in the sequence. While I think your code probably does catch these and reuse them, they are still being used out of sequence. For example, if your ID's are generated sequentially using an identity col, you might end up with this:
1
2
3
5
4 was assigned to an insert that failed, rolled back, whatever. Using your plan,
you might end up with this:
1
2
3
5
4
If you're date stamping on insert, its confusing even more. So what good does this do you?
Global temp tables are usually not a good thing. If everyone needs to see it, put it a real table!
Finally, in cases where identity is insufficient and you want to use your own id generation scheme, the simplest plan I have seen to date is to create an id table (ID varchar(10), UserID int). Then build a proc that inserts a row (and does whatever you need done to create the next ID, a default function on the table would be good) where userid = @@spid. Then you can select it back.
Personally I have adopted uniqueidentifier for my meaningless keys, lets me do a lot of nice things (I have an article posted).
Anyway - all that and I haven't helped with your problem! What we try to do here on SSC is encourage good solutions and better practices, maybe these notes will help. After I've had some coffee I'll try to work on your problem and see what we can do:-)
Andy
October 7, 2001 at 5:57 am
Dear Andy,
If you wanna spend more time on my problem after your coffee :), and I appreciate it so much, please concentrate on the part:
SELECT Min( SeqNO ) newLastID
INTO ##SeqTemp
FROM ...
Amir
October 7, 2001 at 2:42 pm
I think that you're using the ##table because of the scoping issues with exec(), right? I think you're having your locking issues because exec() does run in a different scope, you might try setting your read uncommitted or whatever as part of the sql that you're exec'ing.
There was an article in this months SQL Magazine that mentioned a very handy tip - sp_executesql apparently has an undocumented feature - the ability to return an output parameter. Might be just the thing for this situation. Here is the link:
http://www.sqlmag.com/Articles/Index.cfm?ArticleID=22091
I know I'm taking the easy way out, but I still think you should reconsider your solution. You're doing a LOT of work for not a lot of return that I can see. Have you benchmarked this to see what it's costing you on each insert, compared to using an identity column?
What happens if you have two users execute this at once? User1 creates the ##table...then user2 comes along and drops it!
Locking is not something to be avoided, just minimized. Without locking you have no data integrity. In your case it could cause you to re-use or attempt to re-use a primary key value.
I hope other readers will comment on this as well - be good to get some other points of view, since much of this is my opinionated opinion!
Andy
October 8, 2001 at 4:39 am
You're right somehow. I can use INT instead of TINYINT , use identity and leave gaps. I'll try the sp_executesql special feature and scoping problem in EXEC() and inform you, if I get to desired result.
By the way, I ommit creating and droping ##temp by using real table , as you said, and it seems to run a faster. Thank you for your tips.
Amir
Viewing 12 posts - 1 through 11 (of 11 total)
You must be logged in to reply to this topic. Login to reply