New sql server queries (Azure) - refactoring and formatting#8186
New sql server queries (Azure) - refactoring and formatting#8186ssoroka merged 26 commits intoinfluxdata:masterfrom
Conversation
|
It would be better to put formatting style and behavior changes in separate PRs. The code behavior changes are important for a reviewer to check but putting them in with formatting/style changes make reviewing like finding a needle in a haystack. @denzilribeiro Can you review this? |
I'm aware of that, but those are just small things that got lost in the big PR of splitting the different sections of the plugin. |
denzilribeiro
left a comment
There was a problem hiding this comment.
Made 2 comments that are accross the board
a. Do we need deadlock priority? These are DMVs not updatable really
b. Error message change to include DB_Name as well as just a logical server on Azure DB in connection string won't help identify what connection string is in the wrong section
Ex:
DECLARE @errorMessage AS nvarchar(500) = 'Telegraf - Connection string Server:"'+ @@ServerName + ',Database:' + DB_NAME() +'" is not an Azure Managed Instance. Check the database_type parameter in the telegraf configuration.';
| ORDER BY | ||
| end_time DESC | ||
| const sqlAzureDBResourceStats string = ` | ||
| SET DEADLOCK_PRIORITY -10; |
There was a problem hiding this comment.
What is the nead for Deadlock_priority - given DMVs they are memory locations not updatable. I had removed many of them :)... do you see any explicit need for them?
There was a problem hiding this comment.
my bad, I placed it everywhere when I added the check on the engine edition (I mad the code block once and then pasted/replaced it everywhere)
There was a problem hiding this comment.
I had a look at this, DEADLOCK_PRIORITY was present in 11 queries out of 17 for the Azure part and in 9 out of 9 for the on-prem.
I'll remove it in case queries use only dmv
There was a problem hiding this comment.
I've removed some DEADLOCK_PRIORITY (where only dmv were referenced), it is now used only in DatabaseIO and PerfCounters queries (both azure DB and MI)
|
@ssoroka can you merge this? |
…ta#8186) * sqlAzureDBResourceStats - formatting & engine check * sqlAzureDBResourceGovernance - formatting & engine check * sqlAzureDBWaitStats - formatting and engine check * sqlAzureDBDatabaseIO - formatting and engine check * sqlAzureDBProperties - formatting and engine check * sqlAzureDBProperties - fix [engine_edition] * sqlAzureDBOsWaitStats - formatting and engine check * sqlAzureDBOsWaitStats - fix DBMIRROR_QUEUE string * sqlAzureDBMemoryClerks - formatting and engine check * sqlAzureDBRequests - formatting and engine check * sqlAzureDBSchedulers - moved near other queries * sqlAzureMIRequests - added [session_db_name] * sqlAzureDBPerformanceCounters - formatting and engine check * sqlAzureMIProperties - formatting and engine check * sqlAzureMIResourceStats - formatting and engine check * sqlAzureMIResourceGovernance - formatting and engine check * sqlAzureMIDatabaseIO - formatting and engine check * sqlAzureMIMemoryClerks - formatting and engine check * sqlAzureMIOsWaitStats - formatting and engine check * sqlAzureDBPerformanceCounters - fix check edition * sqlAzureMIPerformanceCounters - formatting and engine check * sqlAzureMIRequests - formatting and engine check * sqlAzureMISchedulers - engine check * Changed error msg to include DB_NAME * removed DEADLOCK_PRIORITY from most queries * sqlAzureDBSchedulers - added engineedition check
Required for all PRs:
I've done some formatting also for the Azure queries part.
There are no breaking changes, but a pair of edits to align some queries with the on-prem ones.
Some testing, especially for the Managed instance section is needed (I have an Azure SQL DB but no Managed Instance)
@denzilribeiro