Skip to content

Drop sys.dm_os_schedulers from memory_stats on Azure SQL DB (#857) #876

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erikdarlingdata merged 1 commit into from
Apr 21, 2026
Merged

Drop sys.dm_os_schedulers from memory_stats on Azure SQL DB (#857) #876

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 9 additions & 7 deletions Lite/Services/RemoteCollectorService.Memory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ Use sys.dm_os_sys_info committed_target_kb/committed_kb as approximations.
Azure MI (edition 8) HAS dm_os_sys_memory, sql_memory_model_desc, and behaves like on-prem. */
bool isAzureSqlDb = serverStatus.SqlEngineEdition == 5;

/*
* Azure SQL Database in an elastic pool (e.g. D365FO tenants) enforces
* VIEW SERVER PERFORMANCE STATE on sys.dm_os_schedulers regardless of
* the login's DB-scoped grants, so we skip the schedulers subquery on
* Azure SQL DB and report current_workers_count as NULL. The rest of
* the row — memory sizes from sys.dm_os_sys_info and the perf counters
* — succeeds for contained DB users. See #857.
*/
Comment on lines +33 to +40
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: comment says "report current_workers_count as NULL", but the reader at Memory.cs:164 coerces IsDBNull(11) ? 0 : reader.GetInt32(11), so the value landing in DuckDB memory_stats.current_workers_count is 0, not NULL. A maintainer who reads only this block will be misled about what downstream consumers see. Consider tightening to something like "…SQL returns NULL; the reader coerces it to 0 before the appender, so DuckDB sees 0."

Generated by Claude Code

string query = isAzureSqlDb
? @"
SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
Expand All @@ -46,7 +54,7 @@ Azure MI (edition 8) HAS dm_os_sys_memory, sql_memory_model_desc, and behaves li
buffer_pool_mb = CONVERT(decimal(18,2), pc_buffer.cntr_value / 1024.0),
plan_cache_mb = CONVERT(decimal(18,2), pc_plan.cntr_value * 8.0 / 1024.0),
max_workers_count = osi.max_workers_count,
current_workers_count = w.current_workers
current_workers_count = CONVERT(int, NULL)
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downstream UX consequence worth noting (not a blocker for this PR): FinOpsTab.xaml.cs:271 renders WorkerThreadsText.Text = $"{data.CurrentWorkersCount:N0} / {data.MaxWorkersCount:N0}". After this change, elastic-pool users will see 0 / 12288, which reads as "zero active workers" rather than "unavailable on this tier." A follow-up render guard in FinOpsTab (e.g. show — / 12288 or N/A when CurrentWorkersCount == 0 and engine edition is 5) would close the loop — or keep the DuckDB value nullable end-to-end. Your call; flagging so it doesn't get lost.

Generated by Claude Code

FROM sys.dm_os_sys_info AS osi
CROSS JOIN
(
Expand All @@ -73,12 +81,6 @@ FROM sys.dm_os_performance_counters
WHERE counter_name = N'Cache Pages'
AND object_name LIKE N'%:Plan Cache%'
) AS pc_plan
CROSS JOIN
(
SELECT current_workers = SUM(active_workers_count)
FROM sys.dm_os_schedulers
WHERE status = N'VISIBLE ONLINE'
) AS w
OPTION(RECOMPILE);"
: @"
SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
Expand Down
Loading