January 3, 2009 at 9:59 am
I finally managed to get everything working again, and have a look at your procs. There's a few things I noticed that could be potential problems.
fnGetTerrTree_2:
FROM Users
CROSS APPLY (
SELECT v.TerriId
FROM VarOffices v
WHERE
v.VarId = Users.VarId
AND (Users.VarId IS NOT NULL) AND (Users.TerriId IS NULL)
UNION ALL
SELECT Users.TerriId
WHERE (Users.TerriId IS NOT NULL) AND (Users.VarId IS NULL)
) n
The cross apply means that the subquery will be executed once for each row in the users table. How many rows is that?
How many varOffices are there for a user?
fnGetTerrTree:
FROM Territories with (index(PK_Territories))
It's not recommended to hint an index in most cases. Are you very sure that's the optimal index for this?
SELECT @TerriId,IsValidTerr
FROM Territories with (index(PK_Territories))
WHERE TerriId = @TerriId AND @IsNodeMultiVAR = 0
UNION ALL
SELECT v.TerriId,T.IsValidTerr --1 AS IsValidTerr
FROM VarOffices v
JOIN Territories T with (index(PK_Territories)) on v.TerriId = T.TerriId
JOIN Vars On Vars.VarId = v.VarId
WHERE
v.VarId = @TerriId AND (Vars.IsDeleted = 'N' OR Vars.IsDeleted IS NULL)
AND @IsNodeMultiVAR = 1
This is very likely going to cause a parameter sniffing problem, as the optimal execution plan differs completely for different values of @IsNodeMultiVAR. How is your stored proc calling the outermost function?
CROSS APPLY dbo.fnGetTerritoryTree_2(@loginToken,n.TerriId) T
This means that the function will be run once for each row in the subquery n. I think, from looking at the exec plan, that this is the join that doesn't have a predicate that Grant spotted.
Onto indexes:
I can't see anywhere that you posted indexes for any tables other than Opportunities, so I'm guessing a bit here.
The index IX_Path on Territories should be widened to have ValidTerritory and IsDeleted as index key columns (it looks like they're include columns at the moment). Preferably place them before the TerritoryPath column, as that one is used for range search.
The index IX_Office on Opportunities needs RepID and OppoIsLost adding to the index key. In addition, SQL's looking up an additional 13 columns from the cluster after seeking on that index, doing a total of 85000 seeks on the cluster to do so. The plan puts that at 7%, but I think that cost is low because of the inaccurate row estimate.
You may want to consider adding those to the include, but that makes the index very wide.
Does that help at all?
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
January 5, 2009 at 2:25 am
Hi All,
Thank you very much for your help and taking time to look into this problem. Finally, I was able to reduce the response times for Opportunity UDF from almost 35-56 secs to 14 secs and as per you replies, I believe there is still scope to bring it down further. I am looking into it.
My auto-update stats are ON by default ..But still I updated statistics with Full table scan on all tables in database (as Grant said) and then corrected IX_Office index to include other two columns (as per Gail's analysis)....It worked very well..
About query hints, I was just trying out few things and it didnt help....Now I am going to try other things that you mentioned and will get back to you for help 🙂
Thanks again guys !!
January 5, 2009 at 2:32 am
About correcting IX_Path index, when I try to add two more columns it gives me warning. This index is created on TerritoryPath column which is varchar(900) field...So when I try to add another column to this index, I get warning "Adding the selected columns will result in an index key with max length of 901 bytes..."
Do I need to change the size of TerritoryPath column to modify this index ? Any alternative ?
January 5, 2009 at 6:05 am
rajg (1/5/2009)
About correcting IX_Path index, when I try to add two more columns it gives me warning. This index is created on TerritoryPath column which is varchar(900) field...So when I try to add another column to this index, I get warning "Adding the selected columns will result in an index key with max length of 901 bytes..."Do I need to change the size of TerritoryPath column to modify this index ? Any alternative ?
Since you're working in SQL Server 2005, you could simple add the index as an INCLUDE column.
For what it's worth, indexing large text fields like this can be fairly problematic. If you really need to search on text fields this large, you might want to look into full-text indexing.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
January 5, 2009 at 9:55 am
Grant Fritchey (1/5/2009)
Since you're working in SQL Server 2005, you could simple add the index as an INCLUDE column.
From what I can tell from the exec plan, they're already include columns, at least they appear to be somewhere in the index (the filters are applied as predicates in the index seek, which implies that the columns were available, no key lookup was required, but they weren't sargable)
Since they are equality filters, to have any perf gain, they need to be part of the index key and they need to be ahead of the Path column, which is used for an inequality match (Like)
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
January 5, 2009 at 10:52 am
Ah, I didn't go back and look at the execution plan again.
"The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
- Theodore Roosevelt
Author of:
SQL Server Execution Plans
SQL Server Query Performance Tuning
January 6, 2009 at 9:37 am
Classic problem of using table valued functions: the optimizer can't get the estimations right. Note that nested loop plans are chosen because the estimation is very low (130 rows) with 85000 rows actual.
you can:
1) get rid of the TVFs and inline the code (MUCH preferred)
2) maybe put the output from the TVFs into temp tables and use that
Also it seems you may have a very large index on opportunities, but you are still doing a bookmark lookup. Can you make sure everything you need is in that covering index (IX_VarOffice).
TVFs can be truly awful for performance!!
Best,
Kevin G. Boles
SQL Server Consultant
SQL MVP 2007-2012
TheSQLGuru on googles mail service
January 6, 2009 at 12:22 pm
If the UDFs are the ones that you define a table variable for the output (called "multi-value UDFs"), then that's probably the biggest source of the performance problem. Those are notoriously slow, for a number of reasons.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
January 6, 2009 at 12:49 pm
GSquared (1/6/2009)
If the UDFs are the ones that you define a table variable for the output (called "multi-value UDFs"), then that's probably the biggest source of the performance problem. Those are notoriously slow, for a number of reasons.
All the functions that the OP posted were inline ones, so pretty much parametrised views
Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability
January 6, 2009 at 1:39 pm
GilaMonster (1/6/2009)
GSquared (1/6/2009)
If the UDFs are the ones that you define a table variable for the output (called "multi-value UDFs"), then that's probably the biggest source of the performance problem. Those are notoriously slow, for a number of reasons.All the functions that the OP posted were inline ones, so pretty much parametrised views
Yeah, somehow I missed a major part of the thread when I was reading it.
I'd go with moving the UDFs to temp tables in the calling proc and using that. Test that, see if it gets the improvement it should.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
Viewing 10 posts - 16 through 24 (of 24 total)
You must be logged in to reply to this topic. Login to reply