Slow stored procedure performance [duplicate]

Question

I have a stored procedure that used to run fine without any issues before we made this change last evening and now the performance is unacceptable.

The stored proc pulls data based on information in the contact_lookup table. I highlighted the areas of the code that we changed using **. Basically, when I added WHERE attempt = @attempt and contact_type = @contacttype (as opposed to just hardcoding the value of 1) the performance became unacceptable.

Can someone explain why the behavior changed and how to improve performance using the parameters passed through the stored proc.Also, can we get rid of the union and simplify the code a bit.

USE [Test]
GO
/****** Object:  StoredProcedure [dbo].[sp_ODBGetContactForSingleLocation]    Script Date: 04/21/2017 09:48:13 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO

ALTER PROCEDURE [dbo].[sp_ODBGetContactForSingleLocation]

@reportentityid [int]
, @attemptnumber [int]
, @contacttype [int]
, @clientid [int]
, @resultcode [int] OUT

AS

BEGIN    
    DECLARE @activerowcount [int]   -- This is what we will use to store the number of contacts return after each query.

    SELECT 
    cc.first_name 
    , cc.last_name
    , ISNULL(cc.title, '') AS [title]
    , cct.contact_type_description AS [contact_type]
    , UPPER(re.friendly_name) AS [location]
    , cl.attempt AS [attempt]
    , re.report_entity_id AS [report_entity_id]
    , (SELECT ct.description
            FROM Test.dbo.communication_type ct
            WHERE ct.communication_type_id = cc.preferred_method) AS [preferred_method]
    , am.Email AS [email]
    , cc.phone AS [phone]
    , cc.cell_phone AS [cell_phone]
    , cc.fax AS [fax]
    , cc.department AS [department]
    , cc.address1 AS [address1]
    , cc.address2 AS [address2]
    , cc.city AS [city]
    , cc.state AS [state]
    , cc.zip AS [zip]
    , cc.contact_id AS [contact_id]
    FROM Test.dbo.client_contacts cc
    INNER JOIN Test.dbo.contact_lookup cl
    ON cc.contact_id = cl.contact_id

    INNER JOIN Test.dbo.contact_type_filter ctf
    ON ctf.contact_lookup_id = cl.contact_lookup_id

    INNER JOIN Test.dbo.contact_responsibility_filter crf
    ON crf.contact_lookup_id = cl.contact_lookup_id

    INNER JOIN Test.dbo.client_contact_type cct
    ON cct.contact_type_id = ctf.contact_type_id

    INNER JOIN C3Test.C3.report_entity re
    ON re.report_entity_id = crf.report_entity_id

    INNER JOIN C3SecondaryTest.dbo.aspnet_Membership am
    ON am.UserId = cc.master_user_id

    WHERE 
    **--cl.attempt = @attemptnumber
    cl.attempt = 1**
    AND cl.active = 1
    **AND cct.contact_type_id = 1
    --AND cct.contact_type_id = @contacttype**
    AND cc.active = 1
    AND (re.report_entity_id IN (SELECT report_entity_id FROM C3Test.C3.fn_getParentIDsForEntity(@reportentityid)) OR 
    re.report_entity_id IN (SELECT report_entity_id FROM C3Test.C3.fn_getReportEntityIDsForParent(@reportentityid)))

    UNION 

SELECT 
    cc.first_name 
    , cc.last_name
    , ISNULL(cc.title, '') AS [title]
    , cct.contact_type_description AS [contact_type]
    , UPPER(re.friendly_name) AS [location]
    , cl.attempt AS [attempt]
    , re.report_entity_id AS [report_entity_id]
    , (SELECT ct.description
            FROM Test.dbo.communication_type ct
            WHERE ct.communication_type_id = cc.preferred_method) AS [preferred_method]
    , am.Email AS [email]
    , cc.phone AS [phone]
    , cc.cell_phone AS [cell_phone]
    , cc.fax AS [fax]
    , cc.department AS [department]
    , cc.address1 AS [address1]
    , cc.address2 AS [address2]
    , cc.city AS [city]
    , cc.state AS [state]
    , cc.zip AS [zip]
    , cc.contact_id AS [contact_id] FROM Test.dbo.client_contacts cc
    INNER JOIN Test.dbo.contact_lookup cl
    ON cc.contact_id = cl.contact_id

    INNER JOIN Test.dbo.contact_type_filter ctf
    ON ctf.contact_lookup_id = cl.contact_lookup_id

    INNER JOIN Test.dbo.client_contact_type cct
    ON cct.contact_type_id = ctf.contact_type_id

    INNER JOIN C3SecondaryTest.dbo.aspnet_Membership am
    ON am.UserId = cc.master_user_id

    LEFT OUTER JOIN Test.dbo.contact_responsibility_filter crf
    ON crf.contact_lookup_id = cl.contact_lookup_id

    LEFT OUTER JOIN C3Test.C3.report_entity re
    ON re.report_entity_id = crf.report_entity_id

    WHERE 
    **--cl.attempt = @attemptnumber
    cl.attempt = 1**
    AND cl.active = 1
    **AND cct.contact_type_id = 1
    --AND cct.contact_type_id = @contacttype**
    AND crf.contact_lookup_id IS NULL
    AND cc.client_id = @clientid
    AND cc.active = 1

    SET @activerowcount = @@ROWCOUNT

    -- Now check if the rowcount is > 0. If not you have no contact
    -- so check for the primary_contact in the client_details table
    IF @activerowcount > 0
        BEGIN 
            SET @resultcode = 100
            RETURN 1
        END
    ELSE
        BEGIN
            SELECT
            cc1.first_name 
            , cc1.last_name
            , ISNULL(cc1.title, '') AS [title]
            , 'PRIMARY' AS [contact_type]
            , 'DEFAULT' AS [location]
            , 0 AS [attempt]
            , NULL AS [report_entity_id]
            , (SELECT ct.description
                    FROM Test.dbo.communication_type ct
                    WHERE ct.communication_type_id = cc1.preferred_method) AS [preferred_method]

            , CASE ISNULL(am.Email, '') 
                WHEN '' THEN cc1.email2
                ELSE am.Email 
                END AS [email]

            , cc1.phone AS [phone]
            , cc1.cell_phone AS [cell_phone]
            , cc1.fax AS [fax]
            , cc1.department AS [department]
            , cc1.address1 AS [address1]
            , cc1.address2 AS [address2]
            , cc1.city AS [city]
            , cc1.state AS [state]
            , cc1.zip AS [zip]
            , cc1.contact_id AS [contact_id]            
            FROM Test.dbo.client_contacts cc1
            LEFT OUTER JOIN C3SecondaryTest.dbo.aspnet_Membership am
            ON cc1.master_user_id = am.UserId

            INNER JOIN Test.dbo.client_details cd
            ON cd.client_id = cc1.client_id

            WHERE cd.client_id = @clientid
            AND cc1.contact_id = cd.primary_contact
            AND cc1.active = 1      

            SET @activerowcount = @@ROWCOUNT
            IF @activerowcount > 0
                BEGIN 
                    SET @resultcode = 100
                    RETURN 1
                END 
            ELSE    
            -- Now check if we have a primary contact. If not, 
            -- escalate this to the AE
                BEGIN
                    SELECT 
                    cu.first_name,
                    cu.last_name
                    , 'ACCOUNT EXECUTIVE' AS [title]
                    , 'CCC AE' AS [contact_type]
                    , 'NOT ASSIGNED' AS [location]
                    , 0 AS [attempt]
                    , NULL AS [report_entity_id]
                    , 'EMAIL' AS [preferred_method]
                    , am.Email AS [email]
                    , cu.phone_main AS [phone]
                    , cu.phone_cell AS [cell_phone]
                    , cu.fax_main AS [fax]
                    , '' AS [department]
                    , '' AS [address1]
                    , '' AS [address2]
                    , '' AS [city]
                    , '' AS [state]
                    , '' AS [zip]
                    , 0 AS [contact_id]
                    FROM Test.dbo.C3_user cu
                    INNER JOIN C3SecondaryTest.dbo.aspnet_Membership am
                    ON cu.master_user_id = am.UserId

                    INNER JOIN Test.dbo.client_details cd
                    ON cd.client_id = @clientid

                    WHERE cu.c3_user_id = cd.account_executive

                SET @activerowcount = @@ROWCOUNT

                IF @activerowcount < 1
                    BEGIN
                        SET @resultcode = -130
                        RETURN -1
                    END
                ELSE
                    BEGIN
                        SET @resultcode = 100
                        RETURN 1
                    END
                END
            END

Answer 1

As sp_BlitzErik notes, the performance degradation is most likely due to what's called "parameter sniffing". If there are significant differences in the number of records matching the hard-coded values versus the number of records that some other value might identify, then the plan the server generates can be great for some values and lousy for others.

There are a number of different ways to handle parameter sniffing, and which way works best for you will depend on your specific query, the plans generated with the hardcoded values and without them, and your specific goals. Hence, pointing you to a useful link to help you understand how this works and how to resolve it, instead of just saying, "Try changing the framistat settings from 37.5 to 291.8."

As an initial step, I'd try running your query with a couple of different hard-coded values, and capture the query plans, see how they differ. Then, check out the query plan for the stored procedure. And, read up on parameter sniffing.

On to ways to improve your query:

The main query (with the UNION) may be able to be significantly simplified. However, it's important to realize what UNION does. UNION takes all the records in both queries, merges them into a single result set, de-duplicates them, and returns them. UNION ALL takes the all the records in both queries, merges them into a single result set, and returns them - no deduplication happens. You'll need to analyze the query and your data to determine if your really need the deduplification or not. if not, then simply switching to UNION ALL may improve performance.

Looking at both queries in the UNION, it appears that there is little difference between them - just parts of the WHERE clauses. I would try replacing this with a single query with the following WHERE clause:

    ...
    WHERE 
    --cl.attempt = @attemptnumber
    cl.attempt = 1
    AND cl.active = 1
    AND cct.contact_type_id = 1
    --AND cct.contact_type_id = @contacttype
    AND cc.active = 1
    AND (   re.report_entity_id IN (SELECT report_entity_id FROM C3Test.C3.fn_getParentIDsForEntity(@reportentityid))
         OR re.report_entity_id IN (SELECT report_entity_id FROM C3Test.C3.fn_getReportEntityIDsForParent(@reportentityid))
         OR (crf.contact_lookup_id IS NULL AND cc.client_id = @clientid)
        )

Doing this may eliminate the possibility of any duplicates (rows that match either report_entity check, and that also match the client_id and have no contact_lookup, will only show up once, not twice). However, if it turns out that there's still the potential for duplicates, add DISTINCT after the initial SELECT to remove them.

Next, I'd look at that report_entity part of the WHERE clause. Standard wisdom says that you should generally avoid using IN and a subquery, and use EXISTS instead. In this case, we can actual convert this to a LEFT JOIN, using a temp table.

Before the first query in your procedure, add this:

CREATE TABLE #report_entity_check (report_entity_id int);
INSERT INTO #report_entity_check (report_entity_id)
SELECT report_entity_id FROM C3Test.C3.fn_getParentIDsForEntity(@reportentityid)
UNION
SELECT report_entity_id FROM C3Test.C3.fn_getReportEntityIDsForParent(@reportentityid)
;

(If this query returns a large number of rows, you might make the column in the temp table its clustered primary key).

Then, in the FROM clause, add the following in after the INNER JOIN with report_entity:

...
LEFT OUTER JOIN #report_entity_check rec ON (re.report_entity_id = rec.report_entity_id)
...

Then, the WHERE clause becomes:

    ...
    WHERE 
    --cl.attempt = @attemptnumber
    cl.attempt = 1
    AND cl.active = 1
    AND cct.contact_type_id = 1
    --AND cct.contact_type_id = @contacttype
    AND cc.active = 1
    AND (   rec.report_entity_id IS NOT NULL
         OR (crf.contact_lookup_id IS NULL AND cc.client_id = @clientid)
        )

This should avoid multiple runs of the functions, which can slow things down, and may improve the ability to use indexes.

The other thing that stands out as a potential slowdown is the subquery in the SELECT list, to get preferred_method. This subquery would have to be run for every row returned; in most cases, it's better to make this a JOIN (LEFT OUTER JOIN is safest, as rows can be returned even if a matching record can't be found; however, depending on your data and database, an INNER JOIN might work too).

Note that none of these is guaranteed to get you back to whatever your original performance was, or even to improve performance at all. These are just standard recommendations; your data may turn out to work better with the code in it's present form (in whole or in past). You'll still almost certainly want to look into the possible parameter sniffing issue.