Friday, March 30, 2012

Help on this

Hello:

I am currently running a query and it is taking like 6 hours to run.

what I do is:

I loop through user's table (20000 users) using a cursor, for each user I call a stored procedure "EXEC ...", then in that called stored procedure, I have an a set of 6 IF statements, and in each if statement I have a call for a simple stored procedure,.

Can anyone tell me, how can i optimize that query?

thanks a lot

Don't use a cursor? We'd need to know more specifics about what it's actually doing to determine if you can do this as a set operation instead of row-by-row (cursor).

Marcie

|||

Hello Marcie:

Thank you for your reply.

I sent you an email from your Blog, it has some codes.

thanks a lot,

|||

Hi SNT2,

I don't have access to that email account during the day, sorry. Also, if you'll post the code here (or at least an overview of what it does), other people will be able to help you with your problem too.

Take care,

Marcie

|||

Hello, I will list the Stored Procedures here: As I said, there is a main procedure, which loops using a cursor through all the records, and upon getting each account I call a method, the first method is the general one:

CREATE PROCEDURE ActiveUserAnalysis_AnalyzeUsers( @.AnalysisPeriod int = 30, -- the number of days over which the refills are checked @.CustGrpId int = 101, -- the group that we want to analyze @.AnalysisId int out)ASBEGIN SET NOCOUNT ON --create the analysis record INSERT INTO ActiveUserAnalysis ( AnalysisTime, AnalysisPeriod, CustGrpId) VALUES ( getdate(), @.AnalysisPeriod, 101) SET @.AnalysisId = @.@.identityDECLARE @.UserCount int SET @.UserCount = 0 --loop over all active users and analyze them DECLARE cActiveCustomers CURSORREAD_ONLYFOR select customerid, username from activecustomersview where custgrpid=@.CustGrpIdDECLARE @.customerid intDECLARE @.username varchar(30)OPEN cActiveCustomersFETCH NEXT FROM cActiveCustomers INTO @.customerid, @.usernameWHILE (@.@.fetch_status <> -1)BEGINIF (@.@.fetch_status <> -2)BEGIN SET @.UserCount = @.UserCount + 1 EXEC ActiveUserAnalysis_AnalyzeActiveUser @.AnalysisId, @.AnalysisPeriod, @.CustomerId, @.UserName   ENDFETCH NEXT FROM cActiveCustomers INTO @.customerid, @.usernameENDCLOSE cActiveCustomersDEALLOCATE cActiveCustomers UPDATE ActiveUserAnalysis SET ActiveUserCount = @.UserCount WHERE AnalysisId = @.AnalysisId SELECT Classification, count(*) FROM ActiveUserAnalysis_Results WHERE AnalysisId = @.AnalysisId GROUP BY ClassificationENDGO


Now the method we are calling above for each user is:

CREATE procedure ActiveUserAnalysis_AnalyzeActiveUser( @.AnalysisId int, @.AnalysisPeriod int, @.customerid int, @.username varchar(30))asbegin DECLARE @.TokenType varchar(4) DECLARE @.Expires datetime DECLARE @.Value int DECLARE @.LastActivity datetime DECLARE @.RefillType varchar(30) DECLARE @.LastRefillDate datetime DECLARE @.RefillCount int DECLARE @.TBSUserName varchar(30) --First collect info EXEC ActiveUserAnalysis_GetUsageToken @.customerid, @.TokenType out, @.Value out, @.Expires out SELECT TOP 1 @.LastActivity = LastActivity FROM PPPAccount WHERE CustomerId = @.CustomerId --Classify user -- 1- Refill EXEC ActiveUserAnalysis_CheckUserLastRefills @.CustomerId, @.AnalysisPeriod, @.RefillType out, @.LastRefillDate out if @.@.rowcount > 0 BEGIN INSERT INTO ActiveUserAnalysis_Results (AnalysisId, customerid, classification, lastactivity, refilltype, lastrefilldate, tokentype, value, expires) VALUES (@.AnalysisId, @.customerid, 'Refill', @.LastActivity, @.RefillType, @.LastRefillDate, @.TokenType, @.Value, @.Expires) RETURN END -- 2- NoLogin: He hasn't logged in at all during period If datediff (d, @.LastActivity, getdate()) > @.AnalysisPeriod BEGIN INSERT INTO ActiveUserAnalysis_Results (AnalysisId, customerid, classification, lastactivity, refilltype, lastrefilldate, tokentype, value, expires) VALUES (@.AnalysisId, @.customerid, 'NoLogin', @.LastActivity, @.RefillType, @.LastRefillDate, @.TokenType, @.Value, @.Expires) RETURN END --3 Other. If we reached here, then the user does not fit with any of the above categories INSERT INTO ActiveUserAnalysis_Results (AnalysisId, customerid, classification, lastactivity, refilltype, lastrefilldate, tokentype, value, expires) VALUES (@.AnalysisId, @.customerid, 'Other', @.LastActivity, @.RefillType, @.LastRefillDate, @.TokenType, @.Value, @.Expires)ENDGO

I didn't include all cases, but i listed only 3 of them.

Then in every case I am calling a procedure too.

Can that be optimzed?

Thanks Marcie

|||

Yes, but it will take some significant work. You haven't included the stored procedures/views that this depends on, so I can't really give you code that will work 100%.

Start off by creating a view for many of the stored procedures that this thing depends on... Can you post the code for say...

ActiveUserAnalysis_CheckUserLastRefills? Hopefully that doesn't call any more stored procedures/views.

|||

Hello, thank you for helping me.

The SP is as follows:

CREATE procedure ActiveUserAnalysis_CheckUserLastRefills(@.CustomerId int,@.AnalysisPeriod int, -- in days,@.Refilltype char(5) out,@.LastRefillDate datetime out)asselect top 1 @.RefillType=c.ISKCatCode, @.LastRefillDate = datepurchasedfrom isk i inner join iskcategory c on i.iskcatid = c.iskcatidwhere customerid = @.customerid and datediff(d, datepurchased, getdate()) <= @.AnalysisPeriod and i.CreatedNewCustomer = 0order by datepurchased descGO

Thank you.|||

Hello Marcie:

Can you help in that please?

thanks

|||

SomeNewTricks2 wrote:

I am currently running a query and it is taking like 6 hours to run.

what I do is:

I loop through user's table (20000 users) using a cursor, foreach user I call a stored procedure "EXEC ...", then in that calledstored procedure, I have an a set of 6 IF statements, and in each ifstatement I have a call for a simple stored procedure,.

Can anyone tell me, how can i optimize that query?


That's an awful big haystack. Have you pinpointed thebottleneck? I'd suggest running the query from Query Analyzerwith Show Execution Plan turned on. This should give you an ideaof the slow parts.

Also, this will not likely make an appreciable difference in yourexecution time, but instead of a cursor I'd use a #temp table (see anexample of this approach in this post:http://forums.asp.net/511669/ShowPost.aspx).|||

SomeNewTricks2 wrote:

Hello Marcie:

Can you help in that please?

thanks

My gut feel is still that you could probably do all of this with one (large) Update statement, rather than looping through thousands of items and doing them one at a time. If that gets too complex (or is not possible), using a #temp table as Terri suggests will make this job easier, and much faster than the cursor approach.

Marcie

|||

Thank you all.

Do you think using a loop over a temp table, is better than cursor?

What about the "if" statements, is using them ok?

thanks

|||

SomeNewTricks2 wrote:

Do you think using a loop over a temp table, is better than cursor?


Yes I do, that's why I suggested it. Here's a little background information:SQL Server Temp Table Performance.

SomeNewTricks2 wrote:

What about the "if" statements, is using them ok?


I think using them is OK, but as far as I understand it, when SQLServer compiles the stored procedure it will use the execution planbased on whichever parameters were passed to it the first timethrough. So the execution plan may not be optimized for otherparameters. But I have also read that SQL Server often recompilesstored procedures, so I don't know how much of an issue that truly is.

Again, I urge you to identifiy your bottleneck(s). You havepresented a huge haystack to look through, and it could be that onesmall part needing optimization is causing all of the performanceissues. We don't really have enough information to be able tohelp you.

|||

Sorry, been away for a few days... I didn't see that you were stuffing some of values from ActiveUserAnalysis_GetUsageToken in your inserts, could you post that code? As long as it's pretty simple, I'll have some working code for ya that should be magnitudes faster than what you are currently doing by using set-based rather than cursor-if based logic.

Cursoring over a temp table won't be faster than cursoring over a query, however, you will have less transaction collisions (possibly). The article the above poster referenced, is saying temp tables are faster if by using one you can avoid a cursor and use set-based logic, which is what you'll probably have to do, but possibly not, but it doesn't eliminate the need to move away from the cursor-if logic.

|||

First of all, thank you Terri for your help.

Here is the code:

create procedure ActiveUserAnalysis_GetUsageToken
(
@.customerid int,
@.TokenType varchar(4) out,
@.Value int out,
@.Expires datetime out
)
as
SELECT TOP 1 /* get only first (highest priority) token */
@.TokenType=t.TokenTypeId, @.Value=u.Value, @.Expires=u.Expires
FROM UsageToken u
INNER JOIN UsageTokenTemplate t
ON u.UsageTokenTemplId = t.UsageTokenTemplId
INNER JOIN TokenType y
ON t.TokenTypeId = y.TokenTypeId
WHERE u.customerid = @.customerid
AND u.status<100 /* could be frozen */
AND (u.Expires is null or u.Expires > getdate()) /*did not expire*/
AND (u.Value is null or u.Value>0)
ORDER BY y.Priority, u.Created


Thanks a lot.

|||

Ok, let's start here. First step is to unroll this stored procedure into a view so that we can reuse it later. Heh, it would figure that you would have this buried somewhere, this is definately the hardest part of unravelling your cursor logic, so here goes. I am going to make the assumption that a single user can not have two (or more) tokens created with the same priority created at the same "Created". If this assumption is not correct, please let me know what combination of fields would be required to ALWAYS return no more than a single record.

Coding by hand here, so might be a syntax error somewhere (sorry).

CREATE VIEW vw_CustomerUsageToken

AS

SELECT t1.customerid,t1.TokenTypeId,t1.Value,t1.Expires
FROM (SELECT u.customerid,t.TokenTypeId,u.Value,u.Expires,y.priority,u.created FROM UsageToken u
INNER JOIN UsageTokenTemplate t
ON u.UsageTokenTemplId = t.UsageTokenTemplId
INNER JOIN TokenType y
ON t.TokenTypeId = y.TokenTypeId
WHERE u.status<100 /* could be frozen */
AND (u.Expires is null or u.Expires > getdate()) /*did not expire*/
AND (u.Value is null or u.Value>0)) t1

LEFT JOIN (SELECT u.customerid,t.TokenTypeId,u.Value,u.Expires,y.priority,u.created FROM UsageToken u
INNER JOIN UsageTokenTemplate t
ON u.UsageTokenTemplId = t.UsageTokenTemplId
INNER JOIN TokenType y
ON t.TokenTypeId = y.TokenTypeId
WHERE u.status<100 /* could be frozen */
AND (u.Expires is null or u.Expires > getdate()) /*did not expire*/
AND (u.Value is null or u.Value>0)) t2 on t1.customerid=t2.customerid AND (t2.priority<t1.priority OR (t2.priority=t1.priority AND t2.created<t1.created))

WHERE t2.customerid IS NULLsql

No comments:

Post a Comment