slow proc - SQL Server Forums

Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

All Forums

SQL Server 2000 Forums

SQL Server Administration (2000)

slow proc

Author

Topic

raagi2000
Starting Member

6 Posts

Posted - 2005-12-01 : 17:14:52

We are on SQL2000 sp3A on win2003 sp1 . We are running the 3rd party application on sql database. sometimes it runs really slow and kills the flow of rest of the application. here is the example . what is really bothering here is it happens quite few times unpredictable but kills the apps , FYI tables has been indexed properly , maintenance work (reindex ,update statistics done every day). is there anything can be tuned on the procs .. suggestion welcome

execute pricingsearch_all 102, '''', '''', '''', 1, ''12/01/2005'', ''12/01/2005 11:59:59PM'', '''', '''', '''', '''', '''', '''', '''', '''', '''', '''', '''', '''', 0, ''|N|O|'', '''', '''''

here is the code for the proc

CREATE PROCEDURE PricingSearch_all
@bundleid_val int, @desc_val char(22), @tkr_val char(10), @cusip_val char(12), @datetype tinyint,
@timerecvd_val1 char(20), @timerecvd_val2 char(21), @status_val1 char(1), @status_val2 char(1),
@status_val3 char(1), @status_val4 char(1), @status_val5 char(1), @status_val6 char(1),
@status_val7 char(1), @status_val8 char(1), @status_val9 char(1), @uploaded_val char(1),
@Sedol_val char(12), @ISIN_val char(12), @status_vals varchar(100)
AS
/******Create temp table with statuses to search for********/
set nocount on
declare
@pos int
create table #status_vals (
status_val char(1)
)
create index main_index on #status_vals (status_val)
select @pos = charindex('|', @status_vals, 1)
while ((@pos > 0) and (datalength(substring(@status_vals, @pos + 1, 1)) > 0) and (substring(@status_vals, @pos + 1, 1) <> '|'))
begin
insert into #status_vals
select substring(@status_vals, @pos + 1, 1)
select @pos = charindex('|', @status_vals, @pos + 1)
end
set nocount off
/***********************************************************/
if @datetype = 0
begin
select distinct p.status, rtrim(s.description) + ' ' + s.desc2 'description', s.tkr, p.price, p.lastquote, p.tradedate, p.vol, p.bid, p.ask, p.timerecvd,
p.opening_price, p.historic_close, p.kassakurs, p.last2, p.priceid, s.cusip, s.sedol, s.bloomberg,
s.ric, s.isin, p.vendor, p.userid, p.timelock, s.timelock, s.cap_pointer, s.cap, p.modified,
p.bundleid, o.uploaded, b.name, p.fvm_factor
from prices p, securitymaster s, openpositions o, bundles b, #status_vals sv
where s.tkr = p.tkr and
s.tkr *= o.tkr and
p.bundleid = b.id and
p.bundleid <=
case
when (@bundleid_val = -1) then 1000
else @bundleid_val
end and
p.bundleid >=
case
when (@bundleid_val = -1) then 0
else @bundleid_val
end and
s.description like
case
when (@desc_val = '') then '%'
else rtrim(@desc_val)
end and
s.tkr like
case
when (@tkr_val = '') then '%'
else rtrim(@tkr_val)
end and
s.cusip like
case
when (@cusip_val = '') then '%'
else rtrim(@cusip_val)
end and
p.old = 'F' and
o.uploaded like
case
when(@uploaded_val = '') then '%'
else rtrim(@uploaded_val)
end and
s.sedol like
case
when(@Sedol_val = '') then '%'
else rtrim(@Sedol_val)
end and
s.ISIN like
case
when(@ISIN_val = '') then '%'
else rtrim(@ISIN_val)
end and
--p.status in (@status_val1, @status_val2, @status_val3, @status_val4, @status_val5, @status_val6, @status_val7,
-- @status_val8, @status_val9) and
p.status = sv.status_val and
o.id not in (select * from closedfunds)
order by s.tkr
end
else if @datetype = 1
begin

... the same as above with little modification...

thanks in advance

tkizer
Almighty SQL Goddess

38200 Posts

Posted - 2005-12-01 : 17:19:08

Since this is our third party stored procedure, you won't be able to make any modifications to it without violating the contract that you have with them. The only thing you can do is work with the vendor to improve their code or get a better server.

Tara Kizer
aka tduggan

Kristen
Test

22859 Posts

Posted - 2005-12-02 : 00:31:08

At a glance:

1) the table owner should be explicitly named - e.g. dbo.prices, dbo.securitymaster - otherwise the query plan may not be cached

2) Why would they do:

p.bundleid <=
case
when (@bundleid_val = -1) then 1000
else @bundleid_val
end

rather than pre-set @bundleid_val to 1000 if it is -1 ??

3) I wouldn't do

s.description like
case
when (@desc_val = '') then '%'
else rtrim(@desc_val)
end and

but instead pre RTrim @desc_val and then just use:

(@desc_val = '' OR s.description like @desc_val)

which should be optimised-out if @desc_val is blank

4) this is crap:

o.id not in (select * from closedfunds)

5) I would be inclined to put each:

else if @datetype = 1
begin
... the same as above with little modification...

into a separate SProc, and call it from here, so that each has a separate cached query plan

6) use JOINS instead of deprecated syntax:

from prices p, securitymaster s, openpositions o, bundles b, #status_vals sv
where s.tkr = p.tkr and
s.tkr *= o.tkr and
p.bundleid = b.id

7) but Tara's probably right!

Kristen

SQLServerDBA_Dan
Aged Yak Warrior

752 Posts

Posted - 2005-12-02 : 08:42:30

LOL. I'm sorry for your junkie code that was forced on your system. I'm always amazed that people still use non-ansi standards for their joins. Is that even in SQL2k5? Bookonline said that non-ANSI joins are no longer support way back in SQL 7. I'd guess MS will drop those within the next few releases of SQL Server...

You guys didnt notice this little token of love?


@status_vals varchar(100)
... ... ...
declare 
@pos int
create table #status_vals (
status_val char(1)
)
create index main_index on #status_vals (status_val)
select @pos = charindex('|', @status_vals, 1)
while ((@pos > 0) and (datalength(substring(@status_vals, @pos + 1, 1)) > 0) and (substring(@status_vals, @pos + 1, 1) <> '|'))
begin
insert into #status_vals
select substring(@status_vals, @pos + 1, 1)
select @pos = charindex('|', @status_vals, @pos + 1)
end

1) Use SET not SELECT to assign variables. (very minor but still annoying).
2) Use ANSI joins (minor but still annoying).
3) If you write junk for code, format the queries to make them easier for someone to read so they don't have to spend and extra 30min making sence of it. (very, very minor but still annoying)
4) Never pass in multiple values into the database in one variable. Think of a better way before implimenting a kludge. If this were done differently then you wouldnt need to do the looping...
5) Why add an index to a blank table just prior to adding rows into it? LOL. Why not after you add the rows? For that matter, why use an index on this tiny table? If each value is delimited by a "|" pipe then there can be at most 50 values and your example has only 2 values. Yes maybe an index will help but the cost to create it and do double inserts may be more than not having one at all. I wouldn't know without testing but I'm sure that would be something to look at too.
6) Use (NOLOCK) when selecting from tables if the database is slowing down due to locking. You can also set the transaction isolation to uncommitted and have the same affect. If I had to bet, I'd guess that this is where your problem is occurring with the slow-downs. You have quite a few tables joined very poorly and if you have a high amount of inserts into any one of those tables then you may be getting some locking.

Is your 3rd party app still under "warranty"? If so, I'd ask them to fix it and if not I'd fix it myself

but only after getting approval

.

Have fun,

Daniel, MCP, A+
SQL Server DBA
www.dallasteam.com

Kristen
Test

22859 Posts

Posted - 2005-12-02 : 11:12:28

"You guys didnt notice this little token of love?"

Yeah, I did but I was in a rush and didn't have time to type how to make a SPLITTER function! I also thought it wouldn't be too bad as the example only had a couple of delimited values, so even with a loop it would be pretty quick. But on a scale of 1 to 10 I wouldn't do it that way!

"Use SET not SELECT to assign variables. (very minor but still annoying)."

Not so minor if there are multiple @variables to be assigned - a single SELECT is demonstrably quicker than multiple SETs (or multiple SELECTs for that matter)

"Never pass in multiple values into the database in one variable."

Not sure I would say never on that one. Never store then un-normalised like that, in the database, but that might be the way the data is stored externally and the Sproc can make a better job of splitting than, say, doing multiple round-trip inserts into a staging table from the client!

"Why add an index to a blank table just prior to adding rows into it"

I would have used a PK (i.e. clustered, as per default) and a SPLITTER function, and insert them pre-sorted [out of the splitter function]. I figure that would be OK, index-wise, but I've never tested it per-se.

"Use (NOLOCK) when selecting from tables"

I'm not keen on that - the data will be changing under-foot, won't it?

Kristen

SQLServerDBA_Dan
Aged Yak Warrior

752 Posts

Posted - 2005-12-02 : 14:29:28

"Not so minor if there are multiple @variables to be assigned - a single SELECT is demonstrably quicker than multiple SETs (or multiple SELECTs for that matter)"

Grr... I should have been a little more detailed with that statement but I too was in a rush. Yes, indeed if you are doing several SET's at a time then use one select instead. In this case there were not multiple values to assign so I left it as what I did.

"Not sure I would say never on that one. Never store then un-normalised like that, in the database, but that might be the way the data is stored externally and the Sproc can make a better job of splitting than, say, doing multiple round-trip inserts into a staging table from the client!"

In this case, YES I will say "never". There is always a better way to impliment this type of thing. The problem here stems from the developer who designed this process in the first place. If he would have thought of a better way to design the gui or whatever is calling this proc then there would be no need for such sillyness. Now on the case of dealing with someone like Verizon or Walmart where they tell you how its going to be then you have no choice but I still stand by the fact that you should never do this because its those big companies that are in the wrong in that case. They should have thought of a better way too. **Dealing with mainframe data doesn't count**

"I'm not keen on that - the data will be changing under-foot, won't it?"

It may... but probably not; I don't know the entire design of this system and how much activity there is on it. The data changing is the risk you take but for that same matter you can't have a query that takes 7 minutes to run for a sales person who has someone on the phone to see if a product is available or what the price is. I would say that this needs to be decided by whoever makes their business rules. Don't forget that lock escalation can cause page level or even table level locking which would cause the database to seem very sluggish. When in fact one query has a table lock and 20 other queries are blocked in queue behind it. That would all be fixed with (NOLOCK) for simple db reads. Once again, I have not logged into that db so I have no idea if that's the cause but I bet there is some locking occurring if the system is doing a moderate amount of activity. So would you rather risk a dirty read or no one doing business with you because it takes so long for you to find out how much something costs? Think that statement was a bit over the top? Examples: If newegg.com took 20 seconds to pull up the hardware list every time I wanted to search for something I wouldn't shop there anymore. If google took 5 seconds to return search results I'd still be using yahoo. LOL could you imagine an internet search engine where people had to wait in line to get their search processed? Maybe it is a bit over the top but the point still remains that if your business can't do business because your db has locking problems then someone's in big trouble.

Have fun

Kristen
Test

22859 Posts

Posted - 2005-12-03 : 02:50:08

These are general points representing my opinion, rather than specific to the OP, but I'm interested in your view too 'coz its quite different to mine in some areas:

"a better way to design the gui or whatever is calling this proc ... "

The "Verizon or Walmart" case prevents me saying "Never"! But in fact we often do this ... for example, the GUI has some checkboxes on a web page that the browser presents to the back end as a delimited list - I just pass that delimited list straight through to the Sproc to handle. Not sure I want to mess around with it first in the application - and if I did how I would present it to SQL in a more useful form.

"It may... but probably not" that wouldn't work for me - the support call for a total that is wrong because a transaction caused a change to before/after the point the report had got to, and the fact the error couldn't be repeated, would be a nightmare for customer support - but there again so would "a query that takes 7 minutes to run for a sales person" so the latter we would have designed out of the system to avoid the former!

"If newegg.com took 20 seconds to pull up the hardware list every time I wanted to search for something I wouldn't shop there anymore"

Indeed, but using dirty reads and showing you a product that has the old description but the new picture ain't much good either! I would fix the system so it takes a few ms instead!

Kristen

Subscribe to SQLTeam.com

SQLTeam.com Articles via RSS

SQLTeam.com Weblog via RSS

- Advertisement -

Resources