Need opinion on this logic

[ViperPit]

We have about 1000 client machines that need regular processing performed, which is done by a bank of 20 servers.
We have a table that lists all the client ids, a flag that indicates whether they need processing, and field indicating which server is currently working with that client.
A client can only be connected to by one server at a time, so I wrote the following logic so that the servers regularly poll the client table to look for the ones that need processing, and then "lock" that record by populating the server field.
This logic works 99% of the time. But for some strange reason, a couple times a week we will have an error where 2 servers start trying to work on the same client at once.
I can't reproduce it, and the logic seems correct to me (obviously I wrote it!).
If you could take a look at this and tell me if you see how 2 servers could attempt to work on the same client at once, I would appreciate it.
Each server program has a timer event that fires every minute to run this code and check for possible processing to perform.
Table has 3 fields:
client_id - an integer contain the id's for each client
process_flag - an integer indicating a process needs performed (zero means nothing needs done)
locked_by_server - a string containing the machine name of the server working with that client

The code is written in VB6 using ADO to connect to a SQL server, but the logical is all T-SQL based, so I figured the folks in this board might have some good ideas.

MySQL = "SELECT client_id, process_flag, locked_by_server
         FROM tbl_clients
         WHERE process_flag <> 0
         AND locked_by_server Is Null"

MyRST.Open MySQL, gDB, adOpenDynamic, adLockOptimistic
[b]'This initial query checks for clients that currently have some process to perform based on the flag, and are not currently locked by another server[/b]

Do While Not MyRST.EOF [b]'Loop for all records[/b]
   MySQL = "UPDATE tbl_clients
            SET locked_by_server = '" & varServerName & "'"
            WHERE process_flag <> 0
            AND locked_by_server Is Null"

   gDB.Execute MySQL
   [b]'The update query sets the server field for the
    current row, but only if the flag is still on, and
    another server hasn't already locked it[/b]

   MySQL = "SELECT client_id
            FROM tbl_clients
            WHERE process_flag <> 0
            AND locked_by_server = '" & varServerName & "'"

   MyRSTLocked.Open MySQL, gDB, adOpenStatic, adLockOptimistic
   [b]'Now we do another select to see if the update we
    performed went through and we are actually the 
    server that got the lock[/b]

   If Not MyRSTLocked.EOF Then
     [b]'Perform appropriate processing
     'This is where the code to connect to the client
     'and perform the processing takes place
     'Call process procedure passing MyRSTLocked.Fields("client_id")[/b]

     MySQL = "UPDATE tbl_clients
              SET process_flag = 0, locked_by_server = Null"
              WHERE client_id = " & MyRSTLocked.Fields("client_id")

     gDB.Execute MySQL
     [b]'Now that processing is done, turn off the flag and
      set the server to Null[/b]
   End If

   MyRSTLocked.Close [b]'Close the locked recordset[/b]
   MyRST.MoveNext 
Loop [b]'Go back to the next client in the original process[/b]
MyRST.Close [b]'Close the original recordset[/b]

Thanks for any suggestions you have.

 

[mrdenny]

Your logic looks good. I can see why it would be tough to duplicate the problem.

I had a client where I had to do something similar to this. I used a slightly different approach that may work for you. I use two stored procedures to handle the SQL work. The app simply called the procedures and process based on what was returned from the proc.

You have one proc that locks and returns the record to be processed. Something like this.

create procedure usp_SelectRecord
    @ServerName varchar(50)
as
BEGIN TRANSACTION
declare @Client_ID int
select top 1 @Client_ID = Client_ID
FROM tbl_clients
WHERE process_flag <> 0
    AND locked_by_server Is Null

update tbl_clients
set locked_by_server = @ServerName
where Client_ID = @Client_ID

select Client_ID, ProcessFlag
from tbl_clients
where Client_ID = @Client_ID
COMMIT TRANSACTION
go

You would then do your processing off of the client ID returned to you.

When finished with your processing you run this stored procedure.

create procedure usp_MarkProcessed
    @Client_ID int
as
update tbl_Clients
set Processed = 1,
     locked_by_server = null
where client_ID = @Client_ID
go

You then loop back in your sub to the top and pull the next available client ID. Your sub would look something like this.

private sub ProcessClients
   varClientID = -1
   Do until varClientID = ""
      MySQL = usp_SelectRecord @ServerName='" & varServerName & "'
      MyRST.Open MySQL, gDB, adOpenDynamic, adLockOptimistic
      varClientID = MyRST.fields("Client_ID").value
      varProcessFlag = MyRST.fields("ProcessFlag").value
      MyRST.Close

      [COLOR=blue]Do your processing based on the ProcessFlag here where you connect to the clients machine.[/color]

      MySQL = "usp_MarkProcessed @Client_ID=" & varClientID
      MyRST.Open MySQL, gDB, adOpenDynamic, adLockOptimistic
      MyRST.close
   Loop
end sub

My non-SQL code sucks I know, but you probably get the general idea.

Denny
MCSA (2003) / MCDBA (SQL 2000)

--Anything is possible. All it takes is a little research. (Me)

http://www.mrdenny.com

(Not quite so old any more.)

 

[tektipdjango]

Hi ViperPit,

For me your logic seems a bit strange :

First you check the clients to look for those to process, and get a list of them
Then you lock all your clients to perform the task with only one, then you release all your clients and treat the next client
It seems that the 19 other servers are waiting when you treat each client that you realease between two clients.
At that time they go in a hurry to make their job and the 20th server who is working with his old list is trying to access to a client already in work with another server.

This logic must use very unefficiently the servers which are most of the time waitnig the realease from their partners.

You'd better once the server has found the first in the list of the free clients lock only this one letting the other clients free for processing with the other servers

Am I right?

--------------------------------------------------
[highlight]Django[/highlight]
bug exterminator
tips'n tricks addict​

 

[ViperPit]

django,
You are right, I left a critical piece out of the code I wrote here (I didn't copy and paste the actual code, but just wrote psuedo code here that would be simpler and more understandable to someone without knowledge of our real structure).
The initial lock update query is actually:

   MySQL = "UPDATE tbl_clients
            SET locked_by_server = '" & varServerName & "'"
            WHERE process_flag <> 0
            AND locked_by_server Is Null
            AND client_id = " & MyRST.Fields("client_id")

Of course, referencing the particular client id selected for the particular row being looped for!

 

[tektipdjango]

ViperPit,

your locking logic is better but you still have the problem that the server is working with a list established before the orther servers have worked with the rest of the free clients.

For this I think you have to end the loop and begin a new search for a a free client before beginning a new treatment. So if it is locked by another server, it will not be taken in charge.

--------------------------------------------------
[highlight]Django[/highlight]
bug exterminator
tips'n tricks addict​