pull request for issue #162 - move all AG related queries to AlwaysOnDiagScript.sql by hacitandogan · Pull Request #177 · microsoft/DiagManager

hacitandogan · 2022-06-21T16:11:11Z

Fixes #162

Changes proposed in this pull request:
clearing the duplicates from MiscPssdiaginfo.sql and SQL Server Perf Stats Snapshot.sql and combining all AG related queries under AlwaysOnDiagScript.sql

How to test this code:

execute T-SQL directly against AG instances
build the pssdiag.zip and capture data from different AG versions
import output of this into Nexus

Has been tested on (remove any that don't apply):

Case-sensitive SQL Server instance
SQL Server 2012
SQL Server 2014
SQL Server 2016
SQL Server 2017
SQL Server 2019

+ first draft of making AlwaysOnDiagScript.sql dynamic SQL ,choosing columns per version. -missing points *confirm columns *import tests on Nexus side *add rowset definitions

working on the AlwaysOnDiagScript.sql modification for issue microsoft#162

DiagManager/CustomDiagnostics/AlwaysOn Basic Info/AlwaysOnDiagScript.sql

…Script.sql

PiJoCoder · 2022-06-22T22:15:42Z

DiagManager/CustomDiagnostics/AlwaysOn Basic Info/AlwaysOnDiagScript.sql

+IF (@sql_major_version >=14) --this exists SQL 2017 and above
+	BEGIN
+	  SET @sql = @sql + ',is_distributed_network_name'
+	END


I ran the script on a SQL 2019 and got this error:

-- AG_hadr_ag_listeners -- Msg 207, Level 16, State 1, Server NODE1LABCSS\SQL2019, Line 1 Invalid column name 'is_distributed_network_name'. Then ran `select * from sys.availability_group_listeners` and don't see this column. The suggestion is to remove this logic or double-check it and make sure it applies to this DMV

Suggested change

IF (@sql_major_version >=14) --this exists SQL 2017 and above

BEGIN

SET @sql = @sql + ',is_distributed_network_name'

END

Thank you @PiJoCoder , I see this is not there on the RTM and added later in the CUs and not failed for me since I have those CUs on my test box. I will modify the logic to check the builds as well per major version.
https://docs.microsoft.com/en-us/azure/azure-sql/virtual-machines/windows/availability-group-distributed-network-name-dnn-listener-configure?view=azuresql

Oh, @hacitandogan I see, it is the SYS.AVAILABILITY_GROUP_LISTENERS DMV that has this. I do have it on my SQL 2019 build, but was querying the sys.availability_groups instead incorrectly. And since the documentation does not include the column, I didn't know which one had it. Can you fully-qualify the column to help with this discovery - ag1.is_distributed_network_name? And perhaps for consistency these columns too: listener_id,dns_name,port,is_conformant,ip_configuration_string_from_cluster
Thank you

@PiJoCoder , thank you , I modified this to make names full-qualified for those columns and added the additional build check logic for is_distributed_network_name column.
--this column exists SQL 2019 CU8 ,SQL 2017 CU25,SQL 2016 SP3 and above

--this column exists SQL 2019 CU8 ,SQL 2017 CU25,SQL 2016 SP3 and above
Thanks for calling these out. Based on these, submitted an update request on
https://docs.microsoft.com/en-us/sql/relational-databases/system-catalog-views/sys-availability-group-listeners-transact-sql?view=sql-server-ver16

PiJoCoder · 2022-06-22T22:22:18Z

DiagManager/Pristine/Confirm-FileAttributes.ps1


    $expectedFileAttributes = @(
-         [PSCustomObject]@{Algorithm = "SHA512"; Hash = "5AC4409C459ACB9778E2731E641C517FDE86BAD84B5E9EA5DA3B09127578A3AA01D5AF2A9FCB3A2208B919E5D940E8B971A555DCCFCF7969D3740D01BDB5E513"; FileName = ".\AlwaysOnDiagScript.sql"; FileSize = 11905}
+         [PSCustomObject]@{Algorithm = "SHA512"; Hash = "769E5354390EECB184B23C1D4D212424185FF636F25AEE244E85C0BC82717136124A32EBEE7F53ABA238E9301974A62729F7B6097DCC6AD89204050AD90218F9"; FileName = ".\AlwaysOnDiagScript.sql"; FileSize = 19876}


Suggested change

[PSCustomObject]@{Algorithm = "SHA512"; Hash = "769E5354390EECB184B23C1D4D212424185FF636F25AEE244E85C0BC82717136124A32EBEE7F53ABA238E9301974A62729F7B6097DCC6AD89204050AD90218F9"; FileName = ".\AlwaysOnDiagScript.sql"; FileSize = 19876}

[PSCustomObject]@{Algorithm = "SHA512"; Hash = "1E98F8BC77D9F6952CB83C87780BEF0D61F40D243AF52C532DFE5C79FE0559EF230585230E32D1D0D5159FCB9651C56A823C35FD5BBB4A25571D528EC6C45E1A"; FileName = ".\AlwaysOnDiagScript.sql"; FileSize = 19898}

PiJoCoder · 2022-06-22T22:34:21Z

@hacitandogan , thank you for all your work on this. We made a small change where we added spaces in the rowset identifiers.
Also starting with the rowset with this title, none of the remaining rowsets have an identifier of the type '-- something --'

===========================
AlwaysOn_health DDL Events
===========================

Could you please create rowset identifiers for these. Perhaps the person who created them didn't realize those get imported in SQL Nexus. These probably never got imported in the first place.

Also, could you please remove these "headings". They do add some value in that they explain what the rowset is about, but people rarely read the raw file, and if they do, these headings actually make it harder to read the rowset content. So please remove them and replace them with regular rowset identifiers.

Thank you

PiJoCoder · 2022-06-22T23:00:35Z

@hacitandogan , could you also sync your forked repo with recent changes from the upstream repo. Go to Fetch Upstream and sync your Forked repo. Then pull the changes down to your local machine (git pull)

-added build check for is_distributed_network_name column -added missing identifiers for xevent data -removed headings -other minor modifications

hacitandogan · 2022-06-29T20:37:57Z

@PiJoCoder , thank you for all the feedbacks.

I did following modifications;
-added build check for is_distributed_network_name column
-added missing identifiers for xevent data
-removed headings
-other minor modifications

PiJoCoder · 2022-06-29T20:47:08Z

@PiJoCoder , thank you for all the feedbacks.

I did following modifications; -added build check for is_distributed_network_name column -added missing identifiers for xevent data -removed headings -other minor modifications

Great! I am testing these now.
Could you please do a merge from master - git pull upstream master - because I just merged a PR into master and it has been updated. THANK YOU

PiJoCoder · 2022-06-29T20:51:31Z

@hacitandogan one thing that I am discovering - in AG_AlwaysOn_health_availability_replica_manager_state_change output there could be a ton of rows. In fact the AG I just tested against returned over 5200 rows. Do you think we should get the latest 500 or something like that? Some limited number....?

-limited the row counts to TOP 500 for AlwaysOn_health* events -changed date order to DESC to get latest records

hacitandogan · 2022-06-30T14:46:22Z

Thank you @PiJoCoder , just checked these numbers on my side and I see similar row numbers even on my test box.
To have a logical limit for these row counts, I am adding TOP 500 to all AlwaysOn_health* ones and changing date order to DESC to get the latest records.
I hope this is fine.

recalculate the hash for AlwaysOnDiagScript.sql

PiJoCoder · 2022-06-30T15:45:20Z

Thank you @PiJoCoder , just checked these numbers on my side and I see similar row numbers even on my test box. To have a logical limit for these row counts, I am adding TOP 500 to all AlwaysOn_health* ones and changing date order to DESC to get the latest records. I hope this is fine.

Yes, this is good. At least we won't get a ton of rows that we don't need and bloat the output files.
Thank you. I will test and let you know if there's more feedback. If it looks good I'll merge into Master. I have assigned the SQL Nexus issue to you if you would like to update the Textrowset.xml file for these new items.

PiJoCoder · 2022-06-30T22:24:30Z

Thank you @hacitandogan ! Looks great - merged!

hacitandogan added 3 commits June 2, 2022 15:04

working on these files as a part of issue microsoft#162

7650f47

first draft of making AlwaysOnDiagScript.sql dynamic SQL

cfed758

+ first draft of making AlwaysOnDiagScript.sql dynamic SQL ,choosing columns per version. -missing points *confirm columns *import tests on Nexus side *add rowset definitions

for issue microsoft#162

e84ee84

working on the AlwaysOnDiagScript.sql modification for issue microsoft#162