All new memory counters

[mfuller333]

I added the memory counters from the DMV sys.dm_exec_query_memory_grants, enhancing what was there from sys.sysprocesses. This fundamentally changes how we count memory and which DMV we are relying on for our memory counters. Instead of counting pages and calculating this x 8 KB to come up with our KB usage. I am exposing the pre-calculated memory counters measured in kb in the XML output once the @get_additional_info is enabled. ** See comments below, the counters are in the main output now.

The tags are as follows:

• requested_memory
• granted_memory
• required_memory
• max_used_memory
• DOP
• QueryCost

This shows us exactly who is requesting how much memory, further allowing us to focus on the problem queries with large memory grants but low actual usage. While updating the code, I noticed we are always adding our granted_memory to the memusage from sys.sysprocesses and displaying the results as used_memory in 8 KB pages. This confused me at first because it is reflected in 8 KB pages and secondly that it was reporting memory used by the query during a resource semaphore wait. I would expect to see zero based on the actual used or granted memory for the query based on the description.

Formatted: [used_memory] varchar NOT NULL Non-Formatted: [used_memory] [bigint] NOT NULL For an active request, total memory consumption for the current query For a sleeping session, total current memory consumption

So I changed the used_memory to show what was being actually used by the query in KB. I can change it back to the sys.sysprocesses and the granted_memory from the other DMV if you want. Or just the granted_memory, but I feel since SQL honors all requests, granted_memory might make sense in the output right next to used_memory. Then you could clearly see the memory bloating. Thoughts? I need some direction here.

[mfuller333]

I had a little time to think about this last night, and how to get the best of both worlds. What if I added a parameter called @advanced_memory_counters, when enabled it takes the current used_memory consisting of the sys.sysyprocesses memusage, and granted_memory from dm_exec_requests and splits them into 5 fields, requested_memory, granted_memory, and used_memory, and required_memory, and max_used_memory from dm_exec_query_memory_grants? Or six if you want to still want to report the memory used by the process itself from memusage.

[mfuller333]

Ok added it: Fun stuff. This adds the advanced memory counters to the main output instead of in the XML if you toggle the @advanced_memory_counters=1. The additional fields in the output are: requested_memory, granted_memory, required_memory, used_memory, max_used_memory, DOP, QueryCost

I left the used_memory mapped to the newer DMV used memory not (granted and process memused) . Note, I did not add the Deltas on all the new fields only left it on the used_memory, probably makes sense to add it to max_used_memory since it would vary between polls.

****Added the ability to set the default memory counter you want to look at with parameter @memory_default, defaults to used_memory. --used_memory=1, requested_memory=2, granted_memory=3, required_memory=4, max_used_memory=5

[mfuller333]

Next adding the 2017 and > SQL Sort Spill information. Also I will have to switch between views on Azure Synapse Analytics still. I will have to add a version condition and switch between sys.dm_pdw_nodes_exec_query_memory_grants and sys.dm_exec_query_memory_grants.

Looks like I am going to have to change more then just that one out in the case of Azure. sys.dm_pdw_exec_requests AS r

LEFT JOIN sys.dm_pdw_nodes_exec_query_memory_grants AS [mg] ON [mg].[session_id] = r.[spid]

It should still fold correctly without(below), ya know what I mean... LEFT JOIN sys.dm_pdw_sql_requests AS pr ON pr.request_id = r.request_id

We are really going to need these as well, as long as they map the same it should be straightforward.

So I am adding ideal_memory as well to the memory counters...saw a couple mentions of it in some great articles .

[mfuller333]

I can slide the ideal_memory in easily, and already have added the Azure Synapse Analytics DMV, will release once we discuss, and your back from your vacation.

[mfuller333]

I can't wait to add all the new memory stuff for Azure Synapse Analytics... think of the troubleshooting,...what fun...I know... I am such a geek. I can push the new stuff in like this, just not sure if it is ideal.

WHEN 
                (
                    @SQLversion not like '%Azure SQL Datawarehouse%'
                ) THEN
                    '
                    LEFT OUTER LOOP JOIN sys.dm_exec_requests AS r ON
                        sp.status <> ''sleeping''
                        AND r.session_id = sp.session_id
                        AND r.request_id = sp.request_id
                        AND
                        (
                            (
                                s.is_user_process = 0
                                AND sp.is_user_process = 0
                            )
                            OR
                            (
                                r.start_time = s.last_request_start_time
                                AND s.last_request_end_time <= sp.last_request_end_time
                            )
                    )  
                     LEFT JOIN 
                                sys.dm_exec_query_memory_grants mg 
                        ON 
                                mg.session_id= sp.session_id  
                                                   AND         mg.request_id = sp.request_id
                                '
                    ELSE
                    '
                    LEFT OUTER LOOP JOIN sys.dm_pdw_exec_requests  AS r ON
                        sp.status <> ''sleeping''
                        AND r.session_id = sp.session_id
                        AND r.request_id = sp.request_id
                        AND
                        (
                            (
                                s.is_user_process = 0
                                AND sp.is_user_process = 0
                            )
                            OR
                            (
                                r.start_time = s.last_request_start_time
                                AND s.last_request_end_time <= sp.last_request_end_time
                            )
                        )  
                        LEFT JOIN 
                                    sys.dm_pdw_nodes_exec_query_memory_grants mg 
                            ON 
                                    mg.session_id= sp.session_id  
                                                           AND         mg.request_id = sp.request_id
                                    '
                    END 
                        + '

I can change it all over this way, unless there is a better plan, it gets more complex with versioning though, unless we simplify and say > 2017 the new way....next up sys.sysprocesses??? Unless there is a better way?

[mfuller333]

So on the last commit I added the last memory counter, ideal_memory, and changed the formatting of QueryCost output, as well as changed the physical_reads, reads, and write so they do not return null values when we pull prior to having the request. Also correct some data types. That should about do it for now, starting the Azure Synapse Analytics adjustments. To see all the new counters below is the syntax.

--change what default memory counter you want to see sp_whoisactive @memory_default=6 go sp_whoisactive @advanced_memory_counters=1 go

[mfuller333]

I was thinking of adding additional memory counters to the XML output before adding the Azure Synapse Analytics. The DMV sys.dm_exec_query_memory_grants has some interesting ones. I can grab the query statistics memory stuff from sys.dm_exec_query_stats as well. All real straight forward. Out of those two views what would you like to see?

[amachanic]

Good start!

I left a few comments in the proc. A bigger issue is that there are no version checks on any of this stuff. The proc is supposed to be back-compatible to SQL Server 2005. 2008 would be fine at this point, but even then I think you're going to be in trouble with some of these DMVs. Can you check release versions for each of these and introduce some version checks?

I'm also not sure we need to introduce new top-level columns for all of this stuff, and I'm not sure the two separate input params are required. I'd like to see at most one memory-oriented control param, and I think it would be good to evaluate how things might look with an XML collection or similar. Do you really need to see these metrics that upfront?

[erikdarlingdata]

@mfuller333 I have a version here that hits dm_exec_query_memory_grants and produces an XML clickable column for memory grants, and includes a version check for the DMV.

If you ctrl + f through the code for @get_memory_grant_info, you can see all the places I made changes to include the result.

You're free to use any bits that you find useful. Also, if you think my version gets the memory info you want, I'm fine adding a pull request for it so you don't have to keep going back and forth here.

Thanks, Erik

[mfuller333]

Nice @erikdarlingdata, thanks! Different implementation than I was thinking, just slide the counters in. Pretty slick. I wanted to include a couple additional tables so we can pull more memory information. I am exposing them like this though.

                                FROM @sessions AS sp
                LEFT OUTER LOOP JOIN sys.dm_exec_sessions AS s ON
                    s.session_id = sp.session_id
                    AND s.login_time = sp.login_time
                LEFT OUTER LOOP JOIN sys.dm_exec_requests AS r ON
                    sp.status <> ''sleeping''
                    AND r.session_id = sp.session_id
                    AND r.request_id = sp.request_id
                    AND
                    (
                        (
                            s.is_user_process = 0
                            AND sp.is_user_process = 0
                        )
                        OR
                        (
                            r.start_time = s.last_request_start_time
                            AND s.last_request_end_time <= sp.last_request_end_time
                        )
                    )
                        LEFT JOIN sys.dm_exec_query_stats AS qs ON
                                 r.sql_handle = qs.sql_handle
                            AND  r.plan_handle = qs.plan_handle
                            AND  r.statement_start_offset = qs.statement_start_offset
                        LEFT JOIN sys.dm_exec_query_memory_grants mg ON
                                 mg.session_id = sp.session_id 
                            AND  mg.request_id = sp.request_id
                        LEFT JOIN sys.dm_exec_query_resource_semaphores rs ON
                                 mg.resource_semaphore_id = rs.resource_semaphore_id 
                            AND  mg.pool_id = rs.pool_id
                        LEFT JOIN sys.resource_governor_workload_groups wg ON
                                 s.group_id = wg.group_id
                        LEFT JOIN sys.resource_governor_resource_pools rp ON
                                 wg.pool_id = rp.pool_id

Is there a reason you didn't go this direction? I was thinking because the queries used_memory and max_used_memory are polled, we should handle it similar to used_memory_delta...just seeing what you’re thinking.

As far as versioning, the lowest version I can find to test this on is SQL 2008 R2 SP3 10.50.6000.34, and it works like a charm with all the tables listed above.

I think combining our efforts makes perfect sense. If you don't mind, I would like to blend the ideas together and present it back without the new top-level outputs first. Combine that in a PR first and get the memory counters out there.

@amachanic I am still not completely sold on keeping the memory counters at top level the same; my reasoning is this has caused much confusion among DBA's. Most people I have asked think its used_memory_kb they are looking at all the time, and not memusage + the granted_memory measured in pages. So far the feedback I received is very positive from DBA's testing the memory counters and having them at the top level. Though I agree there are too many. Can we meet somewhere in the middle? Maybe set the @get_memory_grant_info=2 and you get some of the expanded memory fields in the top level output?

[mfuller333]

@erikdarlingdata I blended the code in this version with yours. Now we have access to all the memory counters from these tables:

1. dm_exec_query_stats
2. dm_exec_query_resource_semaphores
3. resource_governor_workload_groups
4. resource_governor_resource_pools

The polling appears to be a factor between the two methods with used_memory and max_used_memory. Let me know your thoughts about this approach. I am getting different results between the two.

[mfuller333]

Maybe a dumb question, but shouldn't we be treating used_memory and max_used_memory, like memusage at the session level from sys.sysprocesses? I think we would poll those counters in similar fashion since the values will increase depending on when its polled? This is what I was thinking initially, and I am still thinking this direction again after much consideration.

[erikdarlingdata]

Nice @erikdarlingdata, thanks! Different implementation than I was thinking, just slide the counters in. Pretty slick. I wanted to include a couple additional tables so we can pull more memory information. I am exposing them like this though.

                                FROM @sessions AS sp
              LEFT OUTER LOOP JOIN sys.dm_exec_sessions AS s ON
                  s.session_id = sp.session_id
                  AND s.login_time = sp.login_time
              LEFT OUTER LOOP JOIN sys.dm_exec_requests AS r ON
                  sp.status <> ''sleeping''
                  AND r.session_id = sp.session_id
                  AND r.request_id = sp.request_id
                  AND
                  (
                      (
                          s.is_user_process = 0
                          AND sp.is_user_process = 0
                      )
                      OR
                      (
                          r.start_time = s.last_request_start_time
                          AND s.last_request_end_time <= sp.last_request_end_time
                      )
                  )
                      LEFT JOIN sys.dm_exec_query_stats AS qs ON
                               r.sql_handle = qs.sql_handle
                          AND  r.plan_handle = qs.plan_handle
                          AND  r.statement_start_offset = qs.statement_start_offset
                      LEFT JOIN sys.dm_exec_query_memory_grants mg ON
                               mg.session_id = sp.session_id 
                          AND  mg.request_id = sp.request_id
                      LEFT JOIN sys.dm_exec_query_resource_semaphores rs ON
                               mg.resource_semaphore_id = rs.resource_semaphore_id 
                          AND  mg.pool_id = rs.pool_id
                      LEFT JOIN sys.resource_governor_workload_groups wg ON
                               s.group_id = wg.group_id
                      LEFT JOIN sys.resource_governor_resource_pools rp ON
                               wg.pool_id = rp.pool_id

Is there a reason you didn't go this direction? I was thinking because the queries used_memory and max_used_memory are polled, we should handle it similar to used_memory_delta...just seeing what you’re thinking.

As far as versioning, the lowest version I can find to test this on is SQL 2008 R2 SP3 10.50.6000.34, and it works like a charm with all the tables listed above.

I think combining our efforts makes perfect sense. If you don't mind, I would like to blend the ideas together and present it back without the new top-level outputs first. Combine that in a PR first and get the memory counters out there.

@amachanic I am still not completely sold on keeping the memory counters at top level the same; my reasoning is this has caused much confusion among DBA's. Most people I have asked think its used_memory_kb they are looking at all the time, and not memusage + the granted_memory measured in pages. So far the feedback I received is very positive from DBA's testing the memory counters and having them at the top level. Though I agree there are too many. Can we meet somewhere in the middle? Maybe set the @get_memory_grant_info=2 and you get some of the expanded memory fields in the top level output?

I went the way I did because it was quick and easy to get the small set of counters that I cared about for memory, and it was nicely summarized in an xml clickable column. I haven’t had a chance to look through your code and think about the differences. I’ll probably hold off on that until it’s more stable. You seem to still be making a lot of changes.

[mfuller333]

@erikdarlingdata I just finally finished all the formatting, unless there is something major missing I don't think there will be anymore changes. Let me know what you think.

[erikdarlingdata]

Some general notes:

• I would probably hold off on coding a hard stop based on version numbers like that. It will be a pain to maintain over time.
• If the version doesn't support memory counters, why not flip it back to 0 rather than erroring out, and adding an informational message to the output?
• Rather than return 0 AS max_memory_usage,, you may want to do something like CONVERT(int, NULL) AS max_memory_usage, -- people might think the query is actually using 0 for memory. I think NULL is a bit more honest because we don't know.
• There are some changes that seem outside the scope of this pull request, e.g.
  • There are also some very large chunks of red/green where it's not immediately clear what was changed. I think you made some commit comments about these, but I'm not sure if I need to review them.

That's about what I have time for at the moment. I'm only calling out the stuff I have questions/concerns about. Please don't take that as me being overly negative. Just short on time.

Thanks, Erik

[mfuller333]

Responses:

1. I would probably hold off on coding a hard stop based on version numbers like that. It will be a pain to maintain over time.

_The issue I am trying to avoid is someone running sp_whoisactive on Azure Synapse Analytics (where I will have to switch between the views sys.dm_pdw_nodes_exec_query_memorygrants) or a future version of SQL server that does not support the DMV’s such as sys.sysprocesses. If they come out with a new version it should be as easy as adding the version to the list and adjusting the dynamic code for that version with our > version blocks.

**updated, changed how I am handling versioning thanks for the input @erikdarlingdata

2. If the version doesn't support memory counters, why not flip it back to 0 rather than erroring out, and adding an informational message to the output?

Sure, that makes sense in SQL Server when enabling something that is not available, it just lets you even though the feature is not there. Thought I would just tell them if they were using SQL Server 2005, so they’re not confused as to why the new memory counters did not appear. If this is the consensus, I have no issue coding this way

3. Rather than return 0 AS max_memory_usage, you may want to do something like CONVERT (int, NULL) AS max_memory_usage, -- people might think the query is actually using 0 for memory. I think NULL is a bit more honest because we don't know.

   _The max_memory_usage is only calculated and available when the memory counters are engaged at the session level since we get different results depending on when the data is polled and how much of that query memory is being consumed. I set it to CONVERT(int, NULL) AS max_memory_usage and uploaded that in my last commit. The only issue is upstream the NULL gets removed from the COALESCE at the same time as used_memory. This brings up a bigger question; should we be returning NULL for all these values when we have a memory_request not granted? Or should they then be zero as they currently are since NULL values are not allowed for used_memoy? Usedmemory wouldn’t be null obviously because a process always will have memusage from sys.sysprocesses, but you bring up a great point what’s the bigger picture and what should we be displaying to the end user?

4. There are some changes that seem outside the scope of this pull request, e.g.

I thought I removed all this stuff, good catch, I saw a couple lingerers and took those out as well. When I merged the changes between the projects it looks like I incorporated some of the changes you were working on unintentionally.

5. There are also some very large chunks of red/green where it's not immediately clear what was changed. I think you made some commit comments about these, but I'm not sure if I need to review them.

The big chucks of code moved around were to fix all the formatting issues between the code and the output lining up when you execute the code:

sp_whoisactive @get_memory_grant_info=1, @show_own_spid=1 If you click your dynamic SQL

I am formatting the output results, so they all line up, and look pretty.

Also, I would like to note that I have some of the memory grant outputs at the top level only when the new counters so you could see the disparities between the granted and used memory without having to dig into the XML. I duplicated 4 of the grants intentionally because it makes more sense if you dig into the XML, you see everything in one place and do not have to flip back and forth between results…it seemed counter intuitive only having them at the top level once you dug into the XML.

Not negative, this all needs to be discussed, thanks for the feedback, you bring up some valid points.

Respectfully,

Mike

[mfuller333]

The latest versioning I added allows us to handle versioning with code blocks like below...until I can think of something more clever.

--my way SQL versioning...

DECLARE @sql_version INT = CONVERT(INT,LEFT(REPLACE(CAST(SERVERPROPERTY('ProductVersion') AS CHAR(15)),'.',''),7))

SELECT 
CASE
    WHEN
        (@sql_version >= 1106020 AND @sql_version < 1200000) OR
        (@sql_version >= 1205000 AND @sql_version < 1300000) OR
        @sql_version >= 1301601
    THEN
        1
    ELSE
        0
    END

or like this...

SELECT 
CASE
    WHEN
        (@sql_version BETWEEN 1106020 AND 1200000) OR
        (@sql_version BETWEEN 1205000 AND 1300000) OR
        @sql_version >= 1301601
    THEN
        1
    ELSE
        0
    END

Or I could use it to set a feature flag. Fun stuff...that should about do it for now. Let me know if you want any changes.

[mfuller333]

I am going to resubmit this request outlining just the final change in a PR

[mfuller333]

This has gotten messy with all the changes, closing this and creating an new PR