[erikdarlingdata]

Claude posting for Erik Darling.

Fixes #5656.

The CPU scheduler collector reported one "Offline CPUs" and one extra NUMA node on every healthy instance. This fixes the SQL in both places it lives: the shared collector (PerformanceMonitor.Collectors/CpuSchedulerStatsCollector.cs, used by Lite and Darling) and the Dashboard install script (install/17_collect_cpu_scheduler_stats.sql).

The three faults

1. The DAC node was left out by node_id <> 32767, but the DAC node's id is 64. It now gets left out by its state (node_state_desc NOT LIKE N'%DAC%').
2. A node counted as online only when its state was exactly ONLINE. The state can combine values (for example ONLINE DAC, IDLE, IDLE_READY, ONLINE THREAD_RESOURCES_LOW, ONLINE HOT ADDED). Online is now every state except OFFLINE (NOT LIKE N'OFFLINE%').
3. offline_cpus counted nodes that were not exactly online. The warning flag counted schedulers with is_online = 0. The two disagreed on some boxes. Both now read one count: visible (user) schedulers with status VISIBLE OFFLINE, in one derived table. Hidden schedulers no longer feed the flag. The Dashboard debug message used to say "offline nodes" and now says "offline CPUs". The config.collection_log row the same flag writes said CRITICAL: Offline CPUs detected in NUMA nodes. It now says CRITICAL: Offline CPUs detected.

No schema change, no column renames, no backfill. Rows already stored keep their old values. The readers need no change. A grep of offline_cpu_count and OfflineCpu shows they only read the stored column. The readers are PerformanceMonitor.Common/CpuSchedulerMetrics.cs, the Darling viewer, the Lite data service and MCP tool, and install/47_create_reporting_views.sql.

Per-instance check (read-only, DMV SELECTs only)

I ran the old and new node and scheduler predicates as plain SELECTs on each test instance. Every instance has node 0 ONLINE and node 64 ONLINE DAC, and no offline schedulers.

Instance	Old total nodes	Old online nodes	Old offline CPUs	New total nodes	New online nodes	New offline CPUs
2016	2	1	1	1	1	0
2017	2	1	1	1	1	0
2019	2	1	1	1	1	0
2022	2	1	1	1	1	0
2025	2	1	1	1	1	0

I also ran the full new collector query (as a SELECT, not the procedure) on all five. It returned total_node_count 1, nodes_online_count 1, offline_cpu_count 0 and offline_cpu_warning 0 everywhere. Only DMVs were read.

Combined node states, run on the 2019 instance against a VALUES list:

State	Counted in total	Counted online (new)	Counted online (old)
ONLINE	yes	yes	yes
ONLINE DAC	no (left out)	n/a	no
IDLE	yes	yes	no
IDLE_READY	yes	yes	no
ONLINE THREAD_RESOURCES_LOW	yes	yes	no
ONLINE HOT ADDED	yes	yes	no
OFFLINE	yes	no	no

Test plan

• New tests in Lite.Tests/CpuSchedulerStatsCollectorDefinitionTests.cs pin both copies. They check the state-based DAC filter and the OFFLINE% rule. They check there is no 32767 and no = N'ONLINE'. They check one VISIBLE OFFLINE predicate, with the count and the warning reading the same column, and no is_online. They check the debug message wording and the config.collection_log text (the install script no longer contains in NUMA nodes). The 32767 and is_online checks used to read the whole install script. They now read only its collection SELECT, sliced between max_workers_count = osi.max_workers_count and OPTION(RECOMPILE);. Both anchors are asserted, so the slice cannot go empty.
• build.yml: - 'install/**/!(*.md)' added to the lite filter and to its byte-for-byte shard copy (lite_shard). A PR that edits only an install script now runs Lite.Tests. This also covers the existing DmvResourceDatabaseSentinelTests read of install/56. CrossAppGuardCiGateTests (including the shard-copy pin), the CI select/map guards, LiteReadsTraitGuardTests in Darling.Tests and the Census and Ratchet classes pass.
• The pinning test fails on the old SQL. I planted the old collector and install files and 4 of 8 tests in the class failed. Those were the 3 new ones and the verbatim query contract. All 8 pass on the fix.
• The existing verbatim-query contract in the same class is updated to the new query text.
• Lite.Tests: every Census and Ratchet class plus the CPU scheduler classes, 87 run, 0 failed. Build 0 warnings, 0 errors.
• Darling.Tests: every Census and Ratchet class, the CPU scheduler classes and the schema generator classes, 555 run, 0 failed, 2 skipped. Build 0 warnings, 0 errors.
• Full suites not run on this laptop. CI decides.
• Not run: a box with an offline node or an affinity-masked CPU (none available). The VALUES run above covers the state strings.
• I did not run the install script because it is a procedure. Its SELECT has the same shape as the collector query that I did run.

Review round 1

• The config.collection_log text said "in NUMA nodes" although the count no longer counts nodes. Changed to CRITICAL: Offline CPUs detected and pinned. Planting the old text back made the new pin fail (1 of 9 in the class). The fix was restored and all 9 pass.
• The install/17 half of the pin lives in Lite.Tests. The lite filter in build.yml did not include install/**, so an install-only PR never ran it. Added the filter line in both the build job and its shard copy.
• The 32767 and is_online checks no longer read the whole install file, so an unrelated later use of either string cannot fail this test.

[claude]

Non-blocking: this debug line is now right, but the other message the same flag drives still frames the count as nodes — line 313 inserts N'CRITICAL: Offline CPUs detected in NUMA nodes' into config.collection_log, and that one is the row an operator actually reads back after the fact (the @debug = 1 RAISERROR only exists for an interactive run). offline_cpu_count no longer counts nodes at all, so "in NUMA nodes" is left over from the shape this PR removed.

Worth noting the new pin cannot see it: InstallScript_DebugMessage_NamesOfflineCpusNotNodes asserts DoesNotContain("offline nodes"), and line 313's wording is "in NUMA nodes", so it passes either way.

[claude]

Non-blocking, but it qualifies the "pin both copies so they cannot drift back" claim: this half of the pin lives in Lite.Tests and reads install/17, and install/** is in build.yml's installer filter (.github/workflows/build.yml:690), not in lite (:644-686). A future PR that touches only install/17_collect_cpu_scheduler_stats.sql — re-introducing node_id <> 32767, say — fires installer and sql-validation, neither of which runs Lite.Tests, so the drift-back is caught by the nightly rather than by the PR. That is the same "a guard only guards if CI runs it on the edit it exists to catch" shape CONTRIBUTING's cross-app-guard section is about; CrossAppGuardCiGateTests does not derive it here because it only tracks the Lite/Darling roots, and Lite.Tests/DmvResourceDatabaseSentinelTests.cs:200 already reads install/56 the same way, so this is a pre-existing gap you are inheriting, not one you introduced. One line (- 'install/**/!(*.md)') in the lite filter would close it for both.

Smaller point on the same block: Assert.DoesNotContain("32767", sql) and Assert.DoesNotContain("is_online", sql) run against the whole 457-line install file, not just this SELECT, so an unrelated later use of either string anywhere in the proc reds this test for a reason that has nothing to do with #5656.

[claude]

Verdict: LGTM

Claims

• VERIFIED — the fix lands in both copies, substantively identical — PerformanceMonitor.Collectors/CpuSchedulerStatsCollector.cs:168-191 vs install/17_collect_cpu_scheduler_stats.sql:193-216.
• VERIFIED — the DAC node is left out by state, not by a node id — collector :182 / install :207; neither file contains 32767 or node_id any more.
• VERIFIED — online is now every non-OFFLINE-prefixed state, replacing the exact = N'ONLINE' match — collector :176 / install :201.
• VERIFIED — count and warning read ONE VISIBLE OFFLINE derived table, and the is_online = 0 subquery is gone from both files — collector :154,160,184-191 / install :183,186,209-216.
• VERIFIED — the debug message now says "offline CPUs" — install :390.
• VERIFIED — no schema change and no reader change needed — PayloadColumns untouched (collector :268-296), positional readers still land on 25/26 (Lite/Services/LocalDataService.CpuScheduler.cs:215-216, Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.CpuScheduler.cs:230-231), and the shared label already reads "Offline CPUs" with nodes shown separately (PerformanceMonitor.Common/CpuSchedulerMetrics.cs:52-53).
• VERIFIED — the reader roster in the body is complete — my grep of offline_cpu/OfflineCpu returns exactly that set, plus install/47_create_reporting_views.sql:2673-2741; nothing in PerformanceMonitor.Alerting reads either column.
• VERIFIED — the verbatim contract was updated — Lite.Tests/CpuSchedulerStatsCollectorDefinitionTests.cs:100-197 matches collector :152-249 line for line.
• VERIFIED — the new tests assert what the body says — CpuSchedulerStatsCollectorDefinitionTests.cs:224-237,244-245, run against both copies by the collector/install theory.
• VERIFIED — "4 of 8 in the class" is arithmetically consistent — 6 [Fact] (:200,241,249,259,303,340) + a 2-case [Theory] (:217) = 8, and exactly four depend on the new SQL.
• UNVERIFIED — the DAC node's id is 64 and every instance shows node 0 ONLINE / node 64 ONLINE DAC — a live sys.dm_os_nodes read; the tree records neither.
• UNVERIFIED — the per-instance 2016-2025 old/new table — live DMV SELECTs.
• UNVERIFIED — the full new query returning 1/1/0/0 on five instances — a live run; sql-validation.yml only checks the proc exists (.github/sql/ci_validate_installation.sql:48) and never executes it.
• UNVERIFIED — the combined-state VALUES table (IDLE, IDLE_READY, ONLINE THREAD_RESOURCES_LOW, ONLINE HOT ADDED) — the whole basis of the two new predicates, and a read-only tree has no list of the states the DMV emits.
• UNVERIFIED — Lite.Tests 87/0 and Darling.Tests 555/0/2 — needs a Windows build.
• UNVERIFIED — "the pinning test fails on the old SQL, 4 of 8 red" — needs the planted run; the count checks out, the red does not reproduce from the tree.

Riskiest lines

1. PerformanceMonitor.Collectors/CpuSchedulerStatsCollector.cs:190 — status = N'VISIBLE OFFLINE' is strictly narrower than the is_online = 0 it replaces (no HIDDEN OFFLINE), so it redefines when the warning and install/17's CRITICAL log row fire, on both SKUs at once — pinned as a string by NodeAndOfflineCpuCounts_UseStateRulesAndOneSchedulerPredicate, UNPINNED on value.
2. PerformanceMonitor.Collectors/CpuSchedulerStatsCollector.cs:182 — the only thing keeping the DAC node out of the node counts is now a DMV string nothing in the tree documents; same string-only pin, UNPINNED on value. (Corollary, unreachable on a real instance so not filed: this WHERE can now empty the set, and SUM over no rows is NULL, which reader.GetInt32(23) at :331 would throw on.)
3. install/17_collect_cpu_scheduler_stats.sql:183 and :277 — the install copy's new CONVERT and GROUP BY term; sql-validation.yml does bind them at ALTER PROCEDURE time on 2017-2025, but only if this PR targets dev, and nothing executes the proc — UNPINNED on value.

Traps near this diff

• "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — applies to the structure, mitigated here — the C# literal and the T-SQL proc cannot be aliased, and the new theory pins both against the same independent predicates rather than against each other (CpuSchedulerStatsCollectorDefinitionTests.cs:214-238).
• "OPTION(RECOMPILE) on collector queries. A statement added to an existing batch needs its own hint." — does not apply — no statement added; the derived table sits inside the one INSERT … SELECT, which keeps its hint at install :278 / collector :249.
• "Integer division in a rate column … truncates every rate under 1/s to 0." — does not apply — the only ratio here is the untouched runnable_pct, already decimal with a NULLIF denominator (collector :214-229); the new column is a COUNT_BIG widened to integer.
• "CASE WHEN interval > 0 THEN delta / interval ELSE 0 END spells 'unknowable' as 0." — does not apply — point-in-time snapshot, no interval and no delta group (WatermarkColumn => null, :255; the no-delta pin at test :340-362); the touched ELSE 0 arms are a count and a flag.
• "Lite and Darling ship in parity: a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit." — does not apply — the fix is in the shared definition both SKUs run (collector :81), and no reader, tool or description changed on either side.

Divergence (blind read vs PR body)

Three, none of them a defect in the SQL:

1. The body's "the Dashboard debug message … now says offline CPUs" is true of install :390, but the message the same flag writes to config.collection_log — the one an operator reads back later — still says N'CRITICAL: Offline CPUs detected in NUMA nodes' (install :313), and the new pin cannot see it (it asserts DoesNotContain("offline nodes")). Commented inline.
2. "Pin both copies so they cannot drift back" is weaker for the install half: install/** is in build.yml's installer filter (:690), not lite (:644-686), so an install-only edit never runs Lite.Tests and a drift-back falls to the nightly. Pre-existing (Lite.Tests/DmvResourceDatabaseSentinelTests.cs:200 reads install/56 the same way) and closable with one filter line. Commented inline.
3. "Online is every state except OFFLINE" is implemented as a prefix test, so the rule is "no OFFLINE-prefixed state" — identical on the state list the body enumerates; a hypothetical … OFFLINE suffix would read as online.

Reviewer checklist

• Run the install/17 procedure (not its SELECT) with @debug = 1 on a real instance — the body says it was not run, and the proc is the only place the new CONVERT lands in an integer column through an INSERT.
• A box with an affinity mask excluding CPUs, or a genuinely offline NUMA node: confirm VISIBLE OFFLINE yields one row per offline CPU, and that nothing that used to raise the warning only through HIDDEN OFFLINE now goes silent.
• Confirm no sys.dm_os_nodes row survives NOT LIKE N'%DAC%' that the old node_id <> 32767 would have dropped — i.e. no internal node now enters total_node_count.
• Azure SQL Managed Instance, which both SKUs collect from (AppliesTo = !IsAzureSqlDb, collector :262): read node_state_desc and sys.dm_os_schedulers.status there.
• Full Lite.Tests + Darling.Tests on Windows and sql-validation on 2017/2019/2022/2025 — both out of reach from this tree.

[claude]

Verdict: LGTM

Claims

• VERIFIED — the config.collection_log text said "in NUMA nodes" and was changed to CRITICAL: Offline CPUs detected and pinned — install/17_collect_cpu_scheduler_stats.sql:313; new test InstallScript_CollectionLogText_NamesOfflineCpusNotNodes (Lite.Tests/CpuSchedulerStatsCollectorDefinitionTests.cs:249-255)
• VERIFIED — the lite filter in build.yml did not include install/**, so an install-only PR never ran Lite.Tests; the line was added to both the build job's lite filter and its shard copy lite_shard — .github/workflows/build.yml:653 and :2659; confirmed installer/sql-validation already matched install/** but lite/lite_shard did not before this commit
• VERIFIED — this also covers the existing DmvResourceDatabaseSentinelTests read of install/56 — install/56_collect_dmv_blocking_snapshot.sql exists and is read at Lite.Tests/DmvResourceDatabaseSentinelTests.cs:200, and the new glob is unscoped to any one file under install/
• VERIFIED — the 32767 and is_online checks no longer read the whole install file, only the collection SELECT between the two anchors — CollectionSelect helper (Lite.Tests/CpuSchedulerStatsCollectorDefinitionTests.cs:376-385), confirmed both anchor strings are unique (single occurrence each) in install/17
• UNVERIFIED — "Planting the old text back made the new pin fail (1 of 9 in the class). ... All 8 [Fact/Theory groups, 9 cases] pass on the fix" — a live test-run result; a read-only tree can confirm the test exists and the assertion direction (8 Fact/Theory attribute groups counted, one Theory with 2 InlineData cases = 9), but not the pass/fail outcome
• UNVERIFIED — "CrossAppGuardCiGateTests (including the shard-copy pin)... pass" — same reason, requires running the suite

Riskiest lines

1. .github/workflows/build.yml:653 (and its twin at :2659) — the new install/**/!(*.md) line must stay byte-identical across the build job's lite filter and the lite_shard copy or the shards silently stop matching the build job's scope — pinned by TheShardJobsFilters_AreByteForByteCopiesOfTheBuildJobs (Lite.Tests/CrossAppGuardCiGateTests.cs:1388)
2. install/17_collect_cpu_scheduler_stats.sql:313 — the collection_log text an operator reads after the fact — pinned by InstallScript_CollectionLogText_NamesOfflineCpusNotNodes
3. Lite.Tests/CpuSchedulerStatsCollectorDefinitionTests.cs:376-385 (CollectionSelect anchor slicing) — a future rename of either anchor string would need to fail loudly rather than silently narrowing to an empty slice that trivially passes every DoesNotContain — pinned by its own inline Assert.True(start >= 0, ...) / Assert.True(end > start, ...) guards, not a separate named test

Traps near this diff

• "dorny/paths-filter v4 evaluates each pattern as an INDEPENDENT predicate under predicate-quantifier: some... Keep the carve-out INSIDE the include: dir/**/!(*.md)." — applies to this diff's layer (new CI path filter), and is followed correctly: the new line is - 'install/**/!(*.md)' at build.yml:653/2659, carve-out inside the include, not a bare !**/*.md subtraction.
• "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — applies to the pre-existing lite/lite_shard duplicate structure this commit edits; the new line was added identically to both copies in the same relative position (right after the .ps1/.py entries), and the existing byte-for-byte pin (TheShardJobsFilters_AreByteForByteCopiesOfTheBuildJobs) still holds since the two lists' concatenation order is unchanged.

Divergence (blind read vs PR body)

None — the body's "Review round 1" account matches the diff on every claim graded above: the collection_log text fix, the install/** filter addition to both copies, and the narrowing of the 32767/is_online checks to the collection SELECT slice.

Reviewer checklist

• Run the Lite.Tests CpuSchedulerStatsCollectorDefinitionTests class (9 cases) and CrossAppGuardCiGateTests to confirm the pass counts the PR body reports, since a read-only tree cannot execute the suite.