Concurrency problem, vb6/sql server 1

[BobRodes]

I have a table with two fields: hospitalID (int) PK and internalID (int) FK. The problem is to update the internalid with a new value, that is gotten by finding the first available integer (minimum unused integer). Feel free to say this itself is a bad idea and suggest another.

So, I have this prototype code in VB6 (bear with me):

newID = GetNewID() 'Iterates a recordset of logical ID's, and returns the first available one.
cmdString = "update hospid_intid set internalid = " & newID & " where hospitalid in ("
cmdString = cmdString & CStr(hospids(0))
For i = 1 To UBound(hospids)
    cmdString = cmdString & ", " & CStr(hospids(i))
Next i
cmdString = cmdString & ")"
Set cmd = New ADODB.Command
With cmd
    .ActiveConnection = cn
    .CommandText = cmdString
    .Execute
End With

This works just fine with a single user. So, the client put it in production and had several people entering data on it, and it becomes clear that GetNewID sometimes returns the same ID to two different people. The first thought was to put GetNewID and the Execute method in a single transaction, but it's slow and we think it could be better done in a stored proc.

I'm thinking I need to take this code

update hospid_intid set internalid = " & newID & " where hospitalid in ("
cmdString = cmdString & CStr(hospids(0))
For i = 1 To UBound(hospids)
    cmdString = cmdString & ", " & CStr(hospids(i))
Next i
cmdString = cmdString & ")"

and turn it into something like

update hospid_intid set internalid = ([COLOR=red] select whatever from whereever that gives me the next available ID--the minimum value that isn't already being used[/color]) where hospitalid in ([COLOR=red]@this, @that, @theother, @n...[/color])

. I have a few problems beyond not being sure of the best way to do that. First, I suspect it might be necessary to execute all of this code atomically to avoid concurrency issues, by enclosing it in a transaction. Is it? Next, I don't like the idea of sending a variable number of parameters to the stored proc, and I don't much like the idea of calling the proc multiple times, once for each hospitalid, either. Finally, the IN clause is light duty, and I'm wondering if there's a better way to do it. (20 different values would be atypically large, usually there are just a few values that change.) Is there a cool way of putting all this together in a stored proc?

TIA

An unforeseen consequence of the information revolution has been the exponential propagation of human error.

 

[RiverGuy]

Can't you just use an Identity column which increments automatically?

 

[JoeAtWork]

Instead of @this, @that, @theother, etc., can you just make a SELECT statement that retrieves all the ID's for the IN clause?

I agree with RiverGuy about using an auto-incrementing column, if the goal is simply to get a unique number.

Putting the whole operation in a stored procedure is the best idea. Judging by the code and comments you are first loading all records into a recordset and iterating through them - that by itself will take an eternity compared to how fast it could be processed server-side.

 

[BobRodes]

Thanks for your input, guys. While it would be easier to use an identity column, it would require more rewriting than is feasible at present. There are business rules that are associated with specific numbers, and that requires the set of available numbers to remain within a certain range. The numbers for records that are deleted have to be recycled.

Joe, the user selects a group of hospital id's, and they have to be updated to the new number. So the id's are never in a table that can be selected; rather I plug them into the hospids array in the VB code. I'm iterating through the array, rather than a recordset. So, I'm looking for a way to pass them into the SP as parameters, although that might not be the best solution.

An unforeseen consequence of the information revolution has been the exponential propagation of human error.

 

[gmmastros]

The big challenge here is how to handle the comma delimited list of values. There are various ways to handle this.

I assume the array you have in VB is some sort of number, probably a VB6 long (which is the equivalent of a SQL Server Int).

There are different options for handling data like this. You could pass a comma delimited string to a stored procedure and use a split function to parse it in to a table variable. You could use dynamic SQL to execute the query. You could pass XML to the stored procedure and handle it that way.

In my opinion, it may be easier (and faster) to use dynamic sql to do this. Something like this:

Create Procedure UpdateInternalId
  @HospitalList VarChar(8000)
As 

Set NoCount On

Declare @InternalId Int
Declare @SQL VarChar(8000)

SET XACT_ABORT ON
begin tran

  Select @InternalId = whatever 
  from   whereever that gives me the next available ID--the minimum value that isn't already being used

  Set @SQL = 'update hospid_intid 
              set    internalid = ' + Convert(VarChar(10), @InternalId) + '
              where  hospitalid in (' + @HospitalList + ')'

  Exec (@SQL)

commit tran
SET XACT_ABORT OFF

-George
Microsoft SQL Server MVP
My Blogs
SQLCop
"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[BobRodes]

That looks very clean, George, and I'll certainly use it. Now the next part of the question is: is there an easy way to get the first blank value in an ordered list? I can better explain what I'm looking for with an example.

Suppose I have a field, which is (as you have correctly supposed) a long integer. Let's say I have ten records in this field, with the values 1 through 5, 7, 8, 10 and 11. I would like to find a select statement that returns 6. Is there such a thing? The problem that I have is that this process needs to be part of the transaction, because it is here that I have the concurrency problem.

An unforeseen consequence of the information revolution has been the exponential propagation of human error.

 

[BobRodes]

Excuse me, I find a lot of information on this on google. I'll read up on it and post back.

An unforeseen consequence of the information revolution has been the exponential propagation of human error.

 

[gmmastros]

There's likely to be various ways to do this. Here's an example that uses a self join. The code below uses a table variable with hard coded data so you can see how it works.

Declare @Temp Table(Id Int)

Insert Into @Temp Values(1)
Insert Into @Temp Values(2)
Insert Into @Temp Values(3)
Insert Into @Temp Values(4)
Insert Into @Temp Values(5)
Insert Into @Temp Values(7)
Insert Into @Temp Values(8)
Insert Into @Temp Values(10)
Insert Into @Temp Values(11)

Select Top 1 A.Id + 1
From   @Temp As A
       Left Join @Temp As B
         On A.Id = B.Id - 1
Where B.Id Is NULL
Order By A.Id

-George
Microsoft SQL Server MVP
My Blogs
SQLCop
"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[BobRodes]

As it turns out, there are indeed plenty of ways to accomplish this. I abandoned a subquery as the worst performer and also the most complicated, and was looking at a solution using where exists. I tried both your suggestion and the one using where exists:

select top 1 a.internalid + 1 from temp0 a
where not exists
(select 1 from temp0 b where b.internalid = a.internalid + 1 )
order by internalid;

Select Top 1 A.internalid + 1
From   temp0 As A       
Left Join temp0 As B On A.internalid = B.internalid - 1
Where B.internalid Is NULL
Order By A.internalid

The where exists outperformed the self join by 47% to 53%. Out of curiosity, I changed your a = b - 1 join criterion to b = a + 1 (Left Join temp0 As B On B.internalid = A.internalid + 1) and interestingly, the performance comparison improved to 51-49%. The differences in the execution plan are that the where exists has a hash match (right anti semi join) compared to a hash match (right outer join), and the self join has an additional filter, presumably because of the where clause.

Once I pull everything together, I'll post what I have. Thank you all!


An unforeseen consequence of the information revolution has been the exponential propagation of human error.

 

[BobRodes]

Ok. Here's the code for the stored proc:

CREATE PROCEDURE [dbo].[UpdateInternalId] @HospitalList varchar(MAX), @InternalID int OUT
AS

SET NOCOUNT ON

DECLARE @SQL varchar(MAX)

SET XACT_ABORT ON
BEGIN TRAN
	select @InternalID =  
		(
		select top 1 a.internalid + 1 from hospid_intid a
		where not exists
			(
			select 1 from hospid_intid b where b.internalid = a.internalid + 1 
			)
		order by internalid	)

	SET @SQL =	'update hospid_intid set internalid = ' + Convert(VarChar(10), @InternalID) + 
				'where  hospitalid in ' + @HospitalList
	EXEC (@SQL)
COMMIT TRAN
SET XACT_ABORT OFF

And, here's the VB code that calls it:

cmdString = "('" & CStr(hospids(0))
For i = 1 To UBound(hospids)
    cmdString = cmdString & "', '" & CStr(hospids(i))
Next i
cmdString = cmdString & "')"
With New ADODB.Command
    .ActiveConnection = cn
    .CommandType = adCmdStoredProc
    .CommandText = "UpdateInternalID"
    .Parameters.Refresh
    .Parameters(1) = cmdString
    .Execute
    newID = .Parameters(2)
End With

Where hospids is an array of the hospitalIDs that need to have their internalid changed to the new value.

I most appreciate the help. George, I'd like to thank you in particular for your ongoing generosity in sharing your expertise.

An unforeseen consequence of the information revolution has been the exponential propagation of human error.

 

[gmmastros]

I would suggest one minor change.

I suggest you remove the parenthesis from the VB side and put it in the stored procedure side. If you later decide to modify the procedure to do this a different way (like using a split function), it will be easier to modify the stored procedure if the parenthesis are not there. It's a very minor thing, and mostly a preference thing.

I also suggest that you remove the single-quotes (and the space) from the VB side. They are not needed.

-George
Microsoft SQL Server MVP
My Blogs
SQLCop
"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[BobRodes]

Thanks for the suggestions, George. Will do. However, I've run into a bigger problem. It seems that putting the language in a transaction doesn't lock the tables, because my concurrency problem hasn't been resolved. I need to make the select statement and the update statement atomic. What is happening now is that two people are getting the same value in the select statement because the first person hasn't updated the table before the second person does the select. I'm looking at ways to specifically lock the table, but I'm having trouble finding my way. Can you help?

An unforeseen consequence of the information revolution has been the exponential propagation of human error.

 

[BobRodes]

Meanwhile, I've kept reading. I've been thinking about atomicity but not isolation. I really need isolation, and it would seem that the default is to allow dirty reads in a transaction. It is precisely this that is causing my problem, because the consistency of the database is dependent on a clean read. So, I'm going to try using lock hints (for the first time). I've made this change and we'll test it after lunch. Meanwhile, is there a more granular way to accomplish what I want?

BEGIN TRAN
	select @InternalID =  
		(
		select top 1 a.internalid + 1 
		from hospid_intid a
		[COLOR=red] with (tablockx, holdlock)[/color]
		where not exists
			(
			select 1 from hospid_intid b where b.internalid = a.internalid + 1 
			)
		order by internalid	)

	SET @SQL =	'update hospid_intid set internalid = ' + Convert(VarChar(10), @InternalID) + 
				'where  hospitalid in ' + @HospitalList
	EXEC (@SQL)
COMMIT TRAN

An unforeseen consequence of the information revolution has been the exponential propagation of human error.

 

[BobRodes]

This change solved the problem. Hopefully it won't create new ones! I'm thinking that the reason that SQL Server allows dirty reads by default (i. e. write lock is the default when I need full) is because most of the time consistency in a transaction isn't threatened by allowing reading in the middle of it. In my case it is, so there you are.

An unforeseen consequence of the information revolution has been the exponential propagation of human error.