July 22, 2010 at 1:18 pm
MegaDBA,
I don't understand your response. If you don't want honest feedback then you shouldn't plop down hundreds of lines of code and ask for feedback. If you do in fact have a total mess on your hands that should be completely redesigned, you really don't want anyone to tell you so? There isn't an easy solution here and pretending that you can do X and everything will be fine would be to mislead you.
My honest suggestion is to hire a consultant if this is an important system. Whoever designed it made a lot of mistakes and it's possible that your management doesn't have an accurate assessment of the skill-sets available in-house. And if it's not an important system then make whatever quick changes you can and let it be and it doesn't really matter.
Aside from Joe's quip about updating the resume I fail to see people being unnecessarily harsh. And as for Joe, just look at his posting history ... he has something like that to say to everyone so don't take it personally.
July 22, 2010 at 1:27 pm
If recompiling it once helped but performance later degraded, maybe one way to band-aid the mess is WITH RECOMPILE so that the SP recompiles on every execution?
July 22, 2010 at 1:27 pm
A lot has been covered already, but I'll just add 2 quick notes: "sp_" as a name is a bad idea (system will look in master db), and as of a quick glance Im not sure why you'd need a temp table OR a table variable. All you do after building it is query it. Seems like a CTE or derived table would be fine.
But I think your biggest problem is just the complex business rules and how they are implemented. I really don't get the whole 'status group' field that you have to check with a nested case statement. You need to interpret the data in multiple tables to figure out what status the record is in?
In fact, the more I look at this, the more it looks like most of this spaghetti code comes down to 1 issue: There are 5000 datestamps, and checking whether they have values is your only way of determining the status of the record. You are using it to pick records, and then to classify them.
The datestamps can stay, but a couple of status fields getting updated as these datestamps get set would make this query about 10 times shorter.
July 22, 2010 at 1:43 pm
MegaDBA,
I don't understand your response. If you don't want honest feedback then you shouldn't plop down hundreds of lines of code and ask for feedback. -Dude no one comes out here to be battered, period. Honest feedback is one thing, harsh unnecessary banter is not productive and is all together different. I started the post by giving 3 important facts, A. It was terrible code, B. it was inherited, and C. ANY HELP would be appreciated...
If you do in fact have a total mess on your hands that should be completely redesigned, you really don't want anyone to tell you so? --What the hell are you talking about? If that's your advice, to rediesign as it has been my advice to the dev, than that's cool. no probs with that
There isn't an easy solution here and pretending that you can do X and everything will be fine would be to mislead you. --I haven't suggested in the least that there was a simple fix nor was I looking for a silver bullet
My honest suggestion is to hire a consultant if this is an important system. Whoever designed it made a lot of mistakes and it's possible that your management doesn't have an accurate assessment of the skill-sets available in-house. Thanks this is the best advice you have offered and although it is more than an obvious piece of advice, we were just seeing what we could tweak (if anything) in the meantime since it just began to rear its ugly head
And if it's not an important system then make whatever quick changes you can and let it be and it doesn't really matter. Again, what the hell are you talking about here?
Aside from Joe's quip about updating the resume I fail to see people being unnecessarily harsh. I think reading is fundamental and you should do more of it, I actually had no problem with what Joe said and agreed with his point, so again I have to ask, what the hell are you talking about?
And as for Joe, just look at his posting history ... he has something like that to say to everyone so don't take it personally. I think you just like to hear yourself talk
Aurora
July 22, 2010 at 1:50 pm
Journeyman & Grasshopper, Thanks awesome tips...
For the life of me, there are several things I see glaring as well, the first was the variable table vs the temp table.
I also am trying to understand what the heck they are doing with the statuses...
I also hate procs to be developed name "sp_", and I hate table names to be like this "dbo.table_bad_practice"
What i find 91% of the cost being spent on is the implied right outer join (as seen in query execution plan)
Aurora
July 22, 2010 at 2:02 pm
Looks like a stats problem to be, have you tried updating them?
Also, is your database creating statistics automatically? This could help, as I see missing statistics for the Clustered indexes scans of a few tables.
Cheers,
J-F
July 22, 2010 at 3:51 pm
MegaDBA (7/22/2010)
I think reading is fundamental and you should do more of it, ...I think you just like to hear yourself talk
Cool it. Insults are not called for, neither are derogatory comments. I know you're under pressure and probably stressed, but taking it out on people here is not going to get you help faster.
I stand by what I said earlier, get someone in to do a proper job. This is well beyond what's reasonably possible for free help on forums from people posting in their spare time.
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
July 22, 2010 at 3:54 pm
Looks like you are missing statistics and it looks like you may have tempdb contention. The hash join is probably consuming a lot of tempdb space and resources due to the size of the resultset, plus you are using a temp table of equal size. Make sure tempdb is configured optimally and try to align your indexes in the same sort order, if possible. At the top of the execution plan, you should see a hash join. If you can get that hash join to become a merge join, you will be in a much better spot. This not only makes the query faster but takes the pressure off tempdb.
http://msdn.microsoft.com/en-us/library/aa178419(SQL.80).aspx
July 23, 2010 at 8:12 am
Just my 2ct.
Someone must have had a "overload where clause" button as we can see in queries like this one.
" FROM (
SELECT loannum
, MIN(apprid) AS myMin
FROM tbl_ldf_appraisals
WHERE ( uwexclude = 0 )
AND ( inactive = 0 )
AND ( orderdate IS NOT NULL )
AND ( appr_canceldate IS NULL )
OR ( uwexclude = 1 )
AND ( inactive = 0 )
AND ( orderdate IS NOT NULL )
AND ( appr_canceldate IS NULL )
AND ( apprvalue IS NULL
OR apprvalue = 0
)
OR ( appraiserID = 13534 ) --Recycled Internally
GROUP BY loannum
) apprMin "
I hope the mix of AND and ... hmm ... plus OR operators without proper bracket usage still delivers the correct results ....
- As already stated by the other replies this will be a challenge to optimize;
- Do you really expect 30M rows in the (unordered) result set ?
- What kind of box is this supposed to run on ? poc/ram/spindles
- what kind of response time do you expect for this to be acceptable ?
- there are query optimizer tools that might help you in the short term. (e.g. the one that comes with Quest Foglight or some other vendors ). Maybe just download a trial ...
It may even pay off getting the product in house to support your companies growth, its dba as well as its dev team(s). Not only for firefighting - as you are doing right now - but even planning ahead and performing/predicting the need for optimizations in the long run.
Johan
Learn to play, play to learn !
Dont drive faster than your guardian angel can fly ...
but keeping both feet on the ground wont get you anywhere :w00t:
- How to post Performance Problems
- How to post data/code to get the best help[/url]
- How to prevent a sore throat after hours of presenting ppt
press F1 for solution, press shift+F1 for urgent solution 😀
Need a bit of Powershell? How about this
Who am I ? Sometimes this is me but most of the time this is me
July 23, 2010 at 9:42 am
ALZDBA,
Thanks I have cleaned up the brackets (good grief) and you betcha we a have an open rec for a senior level dba developer... (whew)
I appreciate the advice, I think the overall dB design or lack thereof is the biggest problem here...in addition to no coding standards being enforced...
Thanks again, i am cutting it down bit by bit and i will let you guys know the outcome of this t-ragedy!
Aurora
July 23, 2010 at 10:11 am
Looks like a "catch-all query":
...
FROM @init init
JOIN tbl_ldf_mtgtype mtg
ON init.loannum = mtg.loannum
JOIN tbl_co_employees emp
ON mtg.processor_id = emp.id
AND ( ISNULL(@EmployeeId, -1) = -1
OR emp.id = @EmployeeId
)
JOIN tbl_co_team_sch_proc pTeam
ON pTeam.tsch_id = emp.procteamid
JOIN tbl_co_team_grouping grp
ON pTeam.tsch_id = grp.teamid
AND ( ISNULL(@TeamId, -1) = -1
OR pTeam.tsch_id = @TeamId
)
JOIN tbl_co_team_category cat
ON grp.categoryid = cat.categoryid
AND cat.categorytype = 'Processing'
AND ( ISNULL(@CategoryId, -1) = -1
OR cat.categoryid = @CategoryId
)
...
Adding WITH RECOMPILE or coding this as dynamic sql should help a bit.
Here you can find here a great article by MVP Erland Sommarskog on how to handle that at best: http://www.sommarskog.se/dyn-search.html.
Also I wouldn't filter on the parameters in the JOIN clause, even if it's INNER JOIN. If it's a filter, put it in the WHERE clause and let the optimizer take a "standard" path to perform the join.
Don't give up, it can be tuned. Lots of people here will help you, or at least will try. Don't give up.
-- Gianluca Sartori
July 23, 2010 at 10:42 am
I see a few things I might consider fixing:
Why are you using a table variable at all? Couldn't that just be a CTE or a subquery? I'm not sure how much of a difference it will make, but it should give the optimizer a better chance rather than explicitly breaking it down into two steps.
This right here:
JOIN tbl_co_employees emp ON mtg.processor_id= emp.id AND (ISNULL(@EmployeeId,-1) = -1 OR emp.id = @EmployeeId)
JOIN tbl_co_team_sch_proc pTeam ON pTeam.tsch_id = emp.procteamid
JOIN tbl_co_team_grouping grp ON pTeam.tsch_id=grp.teamid AND (ISNULL(@TeamId,-1)=-1 OR pTeam.tsch_id=@TeamId)
JOIN tbl_co_team_category cat ON grp.categoryid=cat.categoryid AND cat.categorytype = 'Processing' AND (ISNULL(@CategoryId,-1) = -1 OR cat.categoryid = @CategoryId)
is a prime example of when dynamic SQL should be used. Dynamic SQL would allow you to skip the join altogether if the parameters don't require it.
I agree that I wouldn't do a JOIN ON @parameter = field. Those belong in a WHERE clause, either in a subquery or the main query. In this case, I think you need to put the WHERE in a subquery, resulting in a smaller join.
Other than that, the sheer number of JOINs is just plain going to cost you time. Besides pre-caching some of the data in a flattened state, I'm not sure there's much you can do about that...
July 23, 2010 at 2:58 pm
Broke this up -- So after creating a temp table (as opposed to @table) I wrot the first portion like this:
Select init.loannum,
init.submitdate,
initext.init_RushPurchase,
init.init_express_flow,
init.init_doc_order_dt,
initext.init_lockreceived,
init.init_reviewed_by_funding,
init.init_review_date,
init.init_file_name,
init.init_dtp_date,
init.init_submit_uw_date,
init.init_doc_signdt,
init.init_package_sent_date,
init.init_package_rcvd_date,
init.init_to_rework_date,
init.init_from_rework_date,
init.init_to_renog_date,
init.init_from_renog_date,
init.init_to_resell_date,
init.init_from_resell_date,
init.init_clear_to_fund,
init.init_intro_call_date,
init.init_to_funding_date,
init.init_est_close,
init.current_est_COE_dt,
init.init_uw_approval_date,
init.statuscode,
init.init_cema,
init.appdate,
init.uw_received_date
FROM tbl_ldf_loaninit_ext initext
INNER JOIN tbl_ldf_loaninit init
ON initext.loannum = COALESCE (init.loannum , init.loannum)
WHERE init.init_fund_dt IS NULL
AND init.init_dtp_date IS NOT NULL
AND init.init_temp_loan >= @GetTempLoans
AND init.init_temp_loan <= @GetTempLoans
AND init.init_prov_funded_date IS NULL
AND init.init_cancel_date IS NULL
AND init.init_denied_date IS NULL
AND init.init_submit_to_cancel_dt IS NULL
AND ((init_to_rework_date IS NULL
OR init_from_rework_date IS NOT NULL)
AND (init_to_resell_date IS NULL
OR init_from_resell_date IS NOT NULL)
AND (init_to_renog_date IS NULL
OR init_from_renog_date IS NOT NULL)
or init.init_to_rework_date IS NOT NULL
AND init.init_from_rework_date IS NULL
OR init_to_resell_date IS NOT NULL
AND init_from_resell_date IS NULL
OR init_to_renog_date IS NOT NULL
AND init_from_renog_date IS NULL)
---Now I am working on all that business logic, and/or blah, blah, blah...
I want you to know all your tips helped out, you guys rock!
Aurora
Viewing 13 posts - 16 through 27 (of 27 total)
You must be logged in to reply to this topic. Login to reply