January 14, 2024 at 2:10 pm
Hi.
I have a query with erractic run times. Either 6 secs or up to 6mins. I am trying to determine why the difference and to ensure if always runs quickly. Data in the database is updated nightly. All tables have clustered primary keys and the execution plan shows these are used. I discovered however that these were quite fragmented so I now have a weekly job to rebuild indexes where necessary. Combining this with updating statistics every other has improved reliability i.e the query runs more often than not in 6 secs.
I have query store enabled so I have been able able to see that when the query runs quickly a particular plan is used and when slowly a quite different plan is used. I have forced use of the plan but it doesn'y always use the fast plan. That itself is a little odd.
However, I have taken the query have worked through the plan and have been able to get the query execution time (when not forced) down to about 30secs by added a few new indexes. However, I just can't get the execution time any lower i.e. to match the 6 secs plan.
I've attached my work in progress plan and the fast plan.
The issue is when I add in the left join to the RevisionDaily table. Without this the query runs in 2-3 seconds. Adding this table takes the execution time up to 20-30secs.
Would be grateful for any suggestions on improving execution time.
Thanks.
(I'm using SQL Server 2019)
Work in progress
https://www.brentozar.com/pastetheplan/?id=ryKTvDZFT
Fast Plan
https://www.brentozar.com/pastetheplan/?id=B1oEdwWtp
January 14, 2024 at 2:30 pm
Quick guess - this is a stored procedure? If so, then the change in plan is due to change in parameters. It kind of looks like a parameter sniffing problem. Adding the query hint "OPTIMIZE FOR UNKNOWN" may help you get consistent performance on the query.
The thing that is causing different plans is this right here:
EP.companyId = @companyid
as @companyid is a variable value, I am guessing that it is causing a parameter sniffing problems.
Another thing that MAY help (may not) is to adjust the MAXDOP (add a query hint for MAXDOP 1 so your query doesn't go parallel) as the "fast plan" is single threaded, but your WIP plan is running parallel. SOMETIMES parallel queries get stuck waiting for resources (CPU threads or can cause slowness due to disk I/O) or end up doing self blocking which can impact performance. Not always, but it is a thing you can test. I've had queries that run in 2-3 minutes on average to complete in 30 seconds by using MAXDOP 1 query hint AND it fixed a self-blocking issue that happened randomly which could make the query run for hours in rare cases.
The above is all just my opinion on what you should do.
As with all advice you find on a random internet forum - you shouldn't blindly follow it. Always test on a test server to see if there is negative side effects before making changes to live!
I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.
January 14, 2024 at 3:50 pm
Hi Brian.
Yes, you are correct in that it's a stored procedure. It needs to be run for about 1000 companies either seems to take about 3 hours or 20 hours to run !
I've added "option optimize for unknown" and this makes the procedure consistently run in 26 secs however, this is not quick enough. I added "maxdop 1" but that made things worse. I tried both with and without the optimize for unknown hint.
Looking at the WIP plan, a lot of the estimate seem to be way out compared to the actual rows returned but I can't seem to change that by updating statistics.
Looking at the WIP plan, the highest cost of 93% is for a Hash Match (Left Outer Join) and the estimates are crazy. 63,493 of 153,639,000.
Some of these tables are pretty big in terms of row counts.
As mentioned, when I force the plan then the parameter doesn't seem to make any difference. I just think that it's probably not the best option to force a plan. Presumably, as the data changes this plan will become worse and then eventually I will have the probably with that plan too. That good plan also basically just used the pk indexes.
Taking a copy of the procedure and not forcing the plan I have been analysing the plan and adding indexes to try and improve things. I've pretty much eliminated the PK Index Scans and now have much more efficient (in this case) Index Seeks. I've added on or two filter indexes too which have really help but now when I look at that plan I don't know what else to do with it. I see only that the cardinality estimates are way off but I can't seem to improve them. And of course this WIP plan looks nothing like the "fast" plan.
Any further suggestions gratefully received.
January 14, 2024 at 4:27 pm
I would try and break it down into 2 (maybe 3) sql blocks by moving some of the "good plan joins" - those which have INNER joins - to a temp table - and from that temp table (left) join to the "bad" table ciqEstimateRevisionDaily.
the process around table ciqEstimateAnalysisData (done 3 times) - may be fine to add it to the new temp table mentioned above, or, depending on the number of rows on that table, create another table with estimateConsensusId as the PK, and a column for each of the other columns required (2*3) - this will only work if there is a single row for combination of estimateConsensusId + dataItemId
Tables to move to temp table flagged below
SELECT
ED.dataItemValue,
ED.effectiveDate,
periodEndDate,
ED.dataItemId,
fiscalYear,
Curr.ISOCode,
EC.estimateConsensusId,
EP.periodTypeId,
C.asOfDate AS revisionsAsOfDate,
C.numAnalysts,
C.numAnalystsUp,
C.numAnalystsDown,
D.asOfDate AS EPSSurpriseDate,
D.dataItemValue AS EPSSurprise,
E.asOfDate AS RevSurpriseDate,
E.dataItemValue AS RevSurprise,
F.asOfDate AS DPSSurpriseDate,
F.dataItemValue AS DPSSurprise
FROM
ciqdata..ciqEstimatePeriod EP -- move to temp table
JOIN
ciqdata..ciqEstimateConsensus EC ON EC.estimatePeriodId = EP.estimatePeriodId -- move to temp table
JOIN
ciqdata..ciqEstimateNumericData ED ON ED.estimateConsensusId = EC.estimateConsensusId -- move to temp table
left join ciqdata..ciqCurrency Curr on ED.currencyId=Curr.currencyId -- move to temp table
JOIN
ciqdata..ciqEstimateChain ECC ON ECC.companyId = EP.companyId -- move to temp table
AND ECC.periodTypeId = EP.periodTypeId
AND ECC.filingMode = 0
LEFT JOIN
ciqdata..ciqEstimateRevisionDaily C ON EC.estimateConsensusId = C.estimateConsensusId
AND ED.dataItemId = C.dataItemId
AND C.asOfDate BETWEEN ED.effectiveDate AND ED.toDate
AND C.estimateRevisionTypeId IN (2)
LEFT JOIN
ciqdata..ciqEstimateAnalysisData D ON ED.estimateConsensusId = D.estimateConsensusId -- potentially move to temp table as well
AND D.dataItemId = 100331
LEFT JOIN
ciqdata..ciqEstimateAnalysisData E ON ED.estimateConsensusId = E.estimateConsensusId -- potentially move to temp table as well
AND E.dataItemId = 100333
LEFT JOIN
ciqdata..ciqEstimateAnalysisData F ON ED.estimateConsensusId = F.estimateConsensusId -- potentially move to temp table as well
AND F.dataItemId = 100341
WHERE
EP.companyId = @companyid
AND EP.periodTypeId IN (1, 2)
AND ED.dataItemId IN (100180, 100187, 104090, 100215, 100173, 104097, 100178, 100331, 100333, 100341, 104055, 100250)
AND endDate = '20790606'
AND (EC.tradingItemId IS NULL OR EXISTS (SELECT 1 FROM ciqdata..ciqTradingItem TI WHERE TI.tradingItemId = EC.tradingItemId AND TI.primaryFlag = 1)) -- move to temp table
ORDER BY
EP.periodTypeId,
ED.effectiveDate DESC
January 14, 2024 at 4:36 pm
Disclaimer: this contains speculation and opinions
In broad terms, when considering performance, imo it makes sense to overlay the FROM clause with the execution plan. The FROM clause is read left-to-right from the word "FROM". The corresponding execution plan is read from right-to-left. Again, in broad strokes. Separately, the outer ORDER BY, which appears at the end of the query, is listed in the beginning (from left-to-right) of the execution plan and in this query it's quite costly (as it spans 2 different tables).
The rightmost area of the execution plan contains many index seeks (which are good) with skinny lines (which are good) and they correspond to the INNER JOIN's of the query. Where it goes downhill is when it gets to the LEFT JOINs as there are many, many index scans (denoted by fat lines, which are not good) and hash matches (denoted by fat lines, which are not good). Imo a superficial reading of the execution plan would conclude your tables don't have enough indexes. Superficial because it doesn't consider the cardinalities of the sub-element sets returned by the FROM clause. If a large number of rows are returned (for subsequent JOIN'ing etc.) then the lack of any index will result in hash match'ing as the DBEngine writes to temp space. A lot more interpretation and consideration could go on and on... It's worth looking into
Imo you could break the query apart: 1) allocate the INNER JOIN'ed elements of the query to a temp table, and 2) add indexes to the temp table to support the LEFT JOIN's and the ORDER BY
Aus dem Paradies, das Cantor uns geschaffen, soll uns niemand vertreiben können
January 14, 2024 at 5:08 pm
The fast query is not the same as the slow query - I would focus on the differences. For example - looks like the 'slow' query includes additional tables:
---- Some more tables
left join ciqdata..ciqTradingItem TI on TI.tradingItemId = EC.tradingItemId --and TI.primaryFlag = 1
left join ciqdata..ciqSecurity S on S.securityId = TI.securityId and S.primaryFlag = 1
The slow query includes this: c.dataItemId AS revisionDataItemId,
Looks like you are trying to move the exists into an outer join - which is not going to result in the same plan. The EXISTS will only look for a row to exist - if more rows exist it doesn't need to do anything else, however - with an outer join the value has to be checked for each row - excluding the NULL rows from the outer join and all other rows that don't match.
The slow query is also checking a value in ciqSecurity which is already checked in the join which really just forces an inner join type relationship - but since that isn't even in the 'fast' query it isn't clear what it should be doing.
With that said - on the 'slow' query I would convert the outer joins to ciqEstimateAnalysisData to an OUTER APPLY and hit the table once.
OUTER APPLY (
SELECT EPSSurprise = MAX(CASE WHEN ead.dataItemId = 100331 THEN ead.dataItemValue END)
, EPSSurpriseDate = MAX(CASE WHEN ead.dataItemId = 100331 THEN ead.asOfDate END)
, RevSurprise = MAX(CASE WHEN ead.dataItemId = 100333 THEN ead.dataItemValue END)
, RevSurpriseDate = MAX(CASE WHEN ead.dataItemId = 100333 THEN ead.asOfDate END)
, DPSSurprise = MAX(CASE WHEN ead.dataItemId = 100341 THEN ead.dataItemValue END)
, DPSSurpriseDate = MAX(CASE WHEN ead.dataItemId = 100341 THEN ead.asOfDate END)
FROM CIQData..ciqEstimateAnalysisData ead
WHERE ead.estimateConsensusId = EC.estimateConsensusId
AND ead.dataItemId IN (100331, 100333, 100341)
) AS ea
Jeffrey Williams
“We are all faced with a series of great opportunities brilliantly disguised as impossible situations.”
― Charles R. Swindoll
How to post questions to get better answers faster
Managing Transaction Logs
January 14, 2024 at 7:00 pm
Thanks alot guys, I think I've taken a bit from each of the 3 posts and have made progress. I've posted the new execution plan below. Seems to be consistently sub 10secs.
Steve, you mentioned adding indexes to support the LEFT JOINS. Could you clarify that please? Sorry to be a bit dense but I'm not sure what I should add indexes on. If this bit works I could potentially reduce the execution time still further. At the moment, it execute a table scan on the temp table.
https://www.brentozar.com/pastetheplan/?id=r1RKAiZKT
Thanks again.
January 14, 2024 at 7:22 pm
Just to play devil's advocate here - be careful adding indexes. Each new index causes some data duplication on disk. So the more indexes you have, the slower your DML (INSERT, UPDATE, DELETE) will be. The impact may be low enough that it doesn't matter, but you should be aware of that. The last thing you want when doing tuning with indexes is to create a bunch of unused or duplicate indexes as they will ONLY hurt performance.
Also, for a bit of a potential performance boost - if you don't need the data ordered in the SELECT statement, you can take that out and do the sorting of the data on the application side. I find a lot of times, end users like the ability to sort the data as they see fit in the tool that grabs the data. If you are in a similar situation, let the application sort it instead of SQL and you'll get better performance on the stored procedure. IF the data must be sorted before getting to the application though, then leave that in. Don't want the data to confuse the end users because the order it came back in no longer makes sense.
The above is all just my opinion on what you should do.
As with all advice you find on a random internet forum - you shouldn't blindly follow it. Always test on a test server to see if there is negative side effects before making changes to live!
I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.
January 14, 2024 at 7:35 pm
Thanks Brian, I am aware of the downside to adding indexes. I will be monitoring these and removing any that are not being used. At the moment, I think the sorting needs to stay in place but I will check with the users on this in case it can be removed.
January 14, 2024 at 9:08 pm
Thanks alot guys, I think I've taken a bit from each of the 3 posts and have made progress. I've posted the new execution plan below. Seems to be consistently sub 10secs.
Steve, you mentioned adding indexes to support the LEFT JOINS. Could you clarify that please? Sorry to be a bit dense but I'm not sure what I should add indexes on. If this bit works I could potentially reduce the execution time still further. At the moment, it execute a table scan on the temp table.
https://www.brentozar.com/pastetheplan/?id=r1RKAiZKT
Thanks again.
In the 2nd pastetheplan the temp table is created using SELECT INTO #innerjoins. I never use SELECT INTO. This sentence is a link to the MS Docs on SELECT INTO. Note the parts I bolded below in the Limitations and Restrictions section:
Limitations and Restrictions
You cannot specify a table variable or table-valued parameter as the new table.
You cannot use SELECT...INTO to create a partitioned table, even when the source table is partitioned. SELECT...INTO does not use the partition scheme of the source table; instead, the new table is created in the default filegroup. To insert rows into a partitioned table, you must first create the partitioned table and then use the INSERT INTO...SELECT...FROM statement.
Indexes, constraints, and triggers defined in the source table are not transferred to the new table, nor can they be specified in the SELECT...INTO statement. If these objects are required, you can create them after executing the SELECT...INTO statement.
Specifying an ORDER BY clause does not guarantee the rows are inserted in the specified order.
When a sparse column is included in the select list, the sparse column property does not transfer to the column in the new table. If this property is required in the new table, alter the column definition after executing the SELECT...INTO statement to include this property.
When a computed column is included in the select list, the corresponding column in the new table is not a computed column. The values in the new column are the values that were computed at the time SELECT...INTO was executed.
Specifying an ORDER BY using SELECT INTO is just a waste and the indexes would have to be created after the insert. I provided ChaptGPT-4 Plus with the following prompt:
Prompt: In SQL Server why is it typically faster to add indexes to a table before rows are inserted vs inserting the rows and then indexing?
Answer
ChatGPT-4 Plus:
In SQL Server, the performance difference between adding indexes before or after inserting rows into a table is due to the way indexes are built and maintained.
When you add an index to an empty table and then insert rows:
1. **Sequential Insertion**: As rows are inserted, the index is built incrementally. Since the table starts empty, each new row can be added to the index in order, which is a relatively fast operation.
2. **Minimal Overhead**: There is minimal overhead for index maintenance during the insertion process because the index is being built from scratch.
3. **Optimized Page Splits**: SQL Server can optimize the index page allocations as rows are inserted, reducing the number of page splits, which can be an expensive operation.
On the other hand, when you insert rows into a table without an index and then add the index:
1. **Bulk Operation**: Adding an index after the rows are inserted requires SQL Server to perform a bulk operation to read all the existing rows and build the index. This can be resource-intensive and time-consuming, especially for large tables.
2. **Locking and Blocking**: The process of building an index on a populated table can lead to locking and blocking, which can affect the performance of other operations on the table.
3. **Page Splits and Fragmentation**: If the table already has data and is subject to updates, the index may experience page splits and fragmentation, which can degrade performance. SQL Server has to rearrange the index pages to accommodate the new data, which can be more costly than the sequential insertion in an empty table.
4. **Resource Usage**: Building an index on a populated table can consume more CPU, I/O, and memory resources, as SQL Server has to sort and process all the existing data.
In practice, the decision to create indexes before or after inserting data may depend on the specific use case. For example, if you're performing a one-time data load into an empty table, it might be more efficient to create the indexes first. However, if you're dealing with a table that is constantly being updated, you might need to balance the performance impact of index maintenance during data insertion with the cost of building the index afterward.
ChatGPT's answer is Ok. Like everything else in SQL Server these things depend on all the other things. Test, test, and test.
A very general outline of a procedure or script could be something like this.
create table #innerjoins (...);
create index ndx_ij_periodTypeId_effectiveDate on #innerjoins(periodTypeId, effectiveDate);
create index ndx_ij_estConsensus on #innerjoins(estimateConsensusId);
create index ndx_ij_estConsensus_di_effdt_todt on #innerjoins(estimateConsensusId, dataItemId, effectiveDate, toDate);
create index ndx_other...
insert into #innerjoins
select ...
select ij.*, ...
from #innerjoins ij
left join ciqdata..ciqEstimateRevisionDaily C ON ij.estimateConsensusId = C.estimateConsensusId
AND ij.dataItemId = C.dataItemId
AND C.asOfDate BETWEEN ij.effectiveDate AND ij.toDate
AND C.estimateRevisionTypeId IN (2)
OUTER APPLY (SELECT EPSSurprise = MAX(CASE WHEN ead.dataItemId = 100331 THEN ead.dataItemValue END)
, EPSSurpriseDate = MAX(CASE WHEN ead.dataItemId = 100331 THEN ead.asOfDate END)
, RevSurprise = MAX(CASE WHEN ead.dataItemId = 100333 THEN ead.dataItemValue END)
, RevSurpriseDate = MAX(CASE WHEN ead.dataItemId = 100333 THEN ead.asOfDate END)
, DPSSurprise = MAX(CASE WHEN ead.dataItemId = 100341 THEN ead.dataItemValue END)
, DPSSurpriseDate = MAX(CASE WHEN ead.dataItemId = 100341 THEN ead.asOfDate END)
FROM CIQData..ciqEstimateAnalysisData ead
WHERE ead.estimateConsensusId = ij.estimateConsensusId
AND ead.dataItemId IN (100331, 100333, 100341)) AS ea
order by ij.periodTypeId, ij.effectiveDate DESC;
There are many things to consider for which I have no idea... For example, are your tables very wide? If so then you'll probably want to INCLUDE the columns you need in the index. Are there unique pairings or tuples? Constraints and indexes are related. Indexes are typically reflective of table constraints
Aus dem Paradies, das Cantor uns geschaffen, soll uns niemand vertreiben können
January 15, 2024 at 6:31 am
If you're going to ORDER the rows going into the temp table, you might as well have SQL create a clustered index based on that order so the final should not need a sort. Something like this:
--!!ADDED THIS!!
SELECT TOP (0) ED.dataItemValue,ED.effectiveDate ed,periodEndDate,
ED.dataItemId, ep.fiscalYear,
EC.estimateConsensusId,
EP.periodTypeId,
ED.currencyId,
effectiveDate,
todate,
tradingItemId,
curr.ISOCode
into #innerjoins
from ciqdata..ciqEstimatePeriod EP
-- Join other tables
join ciqdata..ciqEstimateConsensus EC on EC.estimatePeriodId = EP.estimatePeriodId
join ciqdata..ciqEstimateNumericData ED on ED.estimateConsensusId = EC.estimateConsensusId
left join ciqdata..ciqCurrency Curr on ed.currencyId = Curr.currencyId
---- More tables
join ciqdata..ciqEstimateChain ECC on ECC.companyId = EP.companyId
and ECC.periodTypeId = EP.periodTypeId
and ECC.filingMode = 0
--!!ADDED THIS!!
CREATE CLUSTERED INDEX [innerjoins__CL] ON #innerjoins ( periodTypeId, effectiveDate DESC );
--!!CHANGED THIS SLIGHTLY!!
INSERT into #innerjoins
SELECT ED.dataItemValue,ED.effectiveDate ed,periodEndDate,
ED.dataItemId, ep.fiscalYear,
EC.estimateConsensusId,
EP.periodTypeId,
ED.currencyId,
effectiveDate,
todate,
tradingItemId,
curr.ISOCode
from ciqdata..ciqEstimatePeriod EP
-- Join other tables
join ciqdata..ciqEstimateConsensus EC on EC.estimatePeriodId = EP.estimatePeriodId
join ciqdata..ciqEstimateNumericData ED on ED.estimateConsensusId = EC.estimateConsensusId
left join ciqdata..ciqCurrency Curr on ed.currencyId = Curr.currencyId
---- More tables
join ciqdata..ciqEstimateChain ECC on ECC.companyId = EP.companyId
and ECC.periodTypeId = EP.periodTypeId
and ECC.filingMode = 0
where EP.companyId = @companyid
and EP.periodTypeId IN (1,2)
and ED.dataItemId IN (100180,100187,104090,100215,100173,104097, 100178, 100331,100333,100341,104055,100250)
and ecc.endDate='20790606'
ORDER BY EP.periodTypeId,ED.effectiveDate DESC
OPTION (OPTIMIZE FOR UNKNOWN)
SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".
Viewing 11 posts - 1 through 10 (of 10 total)
You must be logged in to reply to this topic. Login to reply