April 21, 2011 at 12:11 am
Hi.
One of the customer provide the s/w for collecting information from Desktop PC details like what are the s/w installed, RAM, MAC, IP, OS etc...
S/W contain more than 400 Procedure and none of the procedure not declare Begin Transaction and commit transaction. whenever executing all the procedure, server process have been blocked with another SPID. ultimately using CPU and Memory 99% resources,
I stopped SQL service and again restart the services still using 99% of CPU. also Blocked process was not cleared after restart the SQL services. Could you give me suggestion me how to resolve this issues?
sample procedure as below
-------------------------
CREATE PROCEDURE dbo.Desktop_SaveGroup
(
@MAC varchar(25),
@host varchar(25),
@gr_name varchar(200),
@type varchar(20),
@flag varchar(5)
)
as
declare @mac_address varchar(25)
declare @hostname varchar(25)
declare @grname varchar(200)
begin
if @flag = '0'
begin
if exists(select * from desktop_group_d
where MAC_address = @MAC and hostname = @host)
begin
declare gr_cursor insensitive CURSOR FOR
select gr_names
from desktop_group_d where MAC_address = @MAC and hostname = @host
open gr_cursor
fetch next from gr_cursor into @grname
while ( @@fetch_status <> -1 )
begin
delete from desktop_group_d
where MAC_address = @MAC and hostname = @host
and gr_names = @grname
and gr_type = @type
fetch next from gr_cursor into @grname
end
close gr_cursor
deallocate gr_cursor
insert into desktop_group_d
values(@MAC, @host, @gr_name, @type, getdate())
end
else if exists(SELECT * from Desktop_Info where MAC_address = @MAC)
-- if MAC is same
begin
declare gr_cursor insensitive CURSOR FOR
select gr_names from desktop_group_d where MAC_address = @MAC
open gr_cursor
fetch next from gr_cursor into @grname
while ( @@fetch_status <> -1 )
begin
delete from desktop_group_d
where MAC_address = @MAC
and gr_names = @grname
and gr_type = @type
fetch next from gr_cursor into @grname
end
close gr_cursor
deallocate gr_cursor
insert into desktop_group_d
values(@MAC, @host, @gr_name, @type, getdate())
--select @grname
end
else if exists(SELECT * from Desktop_Info where hostname = @host)
-- if Hostname is same
begin
declare gr_cursor insensitive CURSOR FOR
select gr_names
from desktop_group_d where hostname = @host
open gr_cursor
fetch next from gr_cursor into @grname
while ( @@fetch_status <> -1 )
begin
delete from desktop_group_d
where hostname = @host
and gr_names = @grname
and gr_type = @type
fetch next from gr_cursor into @grname
end
close gr_cursor
deallocate gr_cursor
insert into desktop_group_d
values(@MAC, @host, @gr_name, @type, getdate())
select @grname
end
else
insert into desktop_group_d
values(@MAC, @host, @gr_name, @type, getdate())
end
else
begin insert into desktop_group_d Values(@MAC, @host, @gr_name, @type, getdate())
end
end
GO
Thanks
ananda
April 21, 2011 at 2:19 am
Hi Ananda
Let's have a closer look at that stored procedure. It's a series of DELETE/INSERT statements separated by conditionals - depending on what's already in the db, one of them will run - or it will simply insert the data into table desktop_group_d.
Here's the first block including the condition to determine if it the block to be run:
if exists(select * from desktop_group_d where MAC_address = @MAC and hostname = @host)
begin
--declare gr_cursor insensitive CURSOR FOR
--select gr_names from desktop_group_d where MAC_address = @MAC and hostname = @host
--open gr_cursor
--fetch next from gr_cursor into @grname
--while ( @@fetch_status <> -1 )
--begin
delete from desktop_group_d
where MAC_address = @MAC
and hostname = @host
and gr_names IN (select gr_names from desktop_group_d where MAC_address = @MAC and hostname = @host)
and gr_type = @type
--fetch next from gr_cursor into @grname
--end
--close gr_cursor
--deallocate gr_cursor
INSERT INTO desktop_group_d
values(@MAC, @host, @gr_name, @type, getdate())
end
Notice the commented-out lines and the change to the DELETE statement? That's all it took to replace row-by-row (slow and CPU-intensive) processing of the cursor with set-based all-at-once processing.
Have a closer look at the DELETE statement. The part which was driven by the cursor has been replaced with an IN subquery. The list it returns is all gr_names matching on @MAC and @host, regardless of @type. However, the WHERE clause does filter on @type. The logic becomes, in words, "Delete rows where we've got a match (to the parameters passed in to the sproc) on MAC_address, hostname and gr_type and where gr_names is found in any row matching on MAC_address and hostname".
So if a row exists which matches on all four parameters - it will be deleted and inserted again. Rows which match on the three parameters @MAC, @host and @type will be deleted if a match is also found in the subquery. It would be more efficiently written like this:
DELETE FROM desktop_group_d
WHERE MAC_address = @MAC
and hostname = @host
AND gr_names <> @gr_name
and gr_names IN (select gr_names from desktop_group_d where MAC_address = @MAC and hostname = @host)
and gr_type = @type
UPDATE desktop_group_d
SET [whatever the date column is called] = getdate()
WHERE MAC_address = @MAC
AND hostname = @host
AND gr_names = @gr_name
AND gr_type = @type
IF @@ROWCOUNT = 0
INSERT INTO desktop_group_d (MAC_address, hostname, gr_names, gr_type, [whatever the date column is called])
values(@MAC, @host, @gr_name, @type, getdate())
The next two blocks check for existence in table Desktop_Info and relax the specificity of the WHERE clause in the DELETE part of the conditional block.
So what are the conclusions from this?
The code is unnecessarily expensive, using cursors to perform row-by-row processing when little effort is required to change it to set-based processing.
It's catch-all programming. The author wasn't really sure how to insert new data into the tables, so all possibilities are covered. Possibly.
Transactions aren't apparent in this sproc. Some client apps issue transactions - the whole sproc is wrapped in a transaction, including all of the work which might be done to "set the stage" (check for existence of data, fiddle with variables & cursors) for table writes. Resources may be locked unnecessarily and those which do need to be protected from other user's work are locked for longer than they need to be. It's better to use explicit transactions written into the sprocs exactly where they are needed.
Cheers
ChrisM
For better assistance in answering your questions, please read this[/url].
Hidden RBAR: Triangular Joins[/url] / The "Numbers" or "Tally" Table: What it is and how it replaces a loop[/url] Jeff Moden[/url]
April 21, 2011 at 2:46 am
Hi ChrisM, Thanks for your valuable replay..
I should asking s/w Dev team to modify all the procedure based on your suggestion. but all the procedure ( begin transaction and commit transaction not mention), Is it not mandatory for procedure?
Thanks
ananda
April 21, 2011 at 3:19 am
Hi Ananda
Your first point - rewriting the code - if the code you posted is a representative sample then I'd agree. Find the worst-performing code and begin there. Grant Fritchey describes how to do this here[/url].
Your second point - transactions are not absolutely necessary. It's possible at least in theory to write a very simple app which doesn't require transactions.
The purpose of a transaction is to protect a unit of work from interference from other processes, ensure it either completes fully or doesn't do anything at all, then release the protection. A unit of work can of course encompass a number of writes to tables. Looking at your sproc, only one of the conditional blocks will run. Each has two or three separate writes which only make sense if they all succeed - or all don't. So each conditional block should have a begin and end transaction around the writes. At this point you will notice that there's no error trapping in the sproc and hence no ROLLBACK. There's little point in wrapping a unit of work in a transaction without a means of backing out properly. I'll try to find an article for you which I've used in the past to show how to trap errors in SQL Server 7,2000.
Cheers
ChrisM
Edit - added link to article.
For better assistance in answering your questions, please read this[/url].
Hidden RBAR: Triangular Joins[/url] / The "Numbers" or "Tally" Table: What it is and how it replaces a loop[/url] Jeff Moden[/url]
April 21, 2011 at 3:26 am
Hi ChrisM,
In SQL SERVER 2000.
Blocked process not clear after restarting SQL SERVICES, please tell me what could be reason? once restart the service all transaction will be terminated automatically by SQL server 2000? why still keeping blocked transaction after restart the services?
Thanks
ananda
April 21, 2011 at 3:44 am
ananda.murugesan (4/21/2011)
Hi ChrisM,In SQL SERVER 2000.
Blocked process not clear after restarting SQL SERVICES, please tell me what could be reason? once restart the service all transaction will be terminated automatically by SQL server 2000? why still keeping blocked transaction after restart the services?
Thanks
ananda
Hi Ananda
I'm a developer, not a DBA. I don't get to restart servers. Here's a developer answer. When you restart the server, it will attempt to resolve (not terminate) any uncommitted transactions which were interrupted by the restart. This activity may in itself give rise to blocking.
For better assistance in answering your questions, please read this[/url].
Hidden RBAR: Triangular Joins[/url] / The "Numbers" or "Tally" Table: What it is and how it replaces a loop[/url] Jeff Moden[/url]
April 25, 2011 at 7:36 pm
ananda.murugesan (4/21/2011)
Hi ChrisM,In SQL SERVER 2000.
Blocked process not clear after restarting SQL SERVICES, please tell me what could be reason? once restart the service all transaction will be terminated automatically by SQL server 2000? why still keeping blocked transaction after restart the services?
Thanks
ananda
Stopping SQL Server does not terminate transactions.
As soon as the service is restarted it restarts all open transactions from transaction log and tries to complete them. You cannot drop transactions, you need either commit or rollback each of them.
You might wish to use this to cancel all transactions:
ALTER DATABASE [database name] SET SINGLE_USER WITH ROLLBACK IMMEDIATE
ALTER DATABASE [database name] SET MULTI_USER
_____________
Code for TallyGenerator
Viewing 7 posts - 1 through 6 (of 6 total)
You must be logged in to reply to this topic. Login to reply