Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added

- **Analysis engine: two server-config issue-classification arms (priority boost + lightweight pooling) in the shared FactScorer (Tier-2 parity, batch-2b)** ([#1520]) — the shared `PerformanceMonitor.Analysis` scorer (ONE code path for Lite, the deprecated Dashboard, AND Darling) now classifies two dangerous `sp_configure` settings the Dashboard's `config.critical_issues` analyzer flagged as WARNINGs but the ported engine did not. Both are near-pure additions over data ALREADY collected: `server_config` accumulates the FULL `sys.configurations` set, so the two ACTIVE fact collectors (Lite `DuckDbFactCollector` + Darling `PgFactCollector`) simply add `'priority boost'` + `'lightweight pooling'` to their curated fact-emission list, emitting `CONFIG_PRIORITY_BOOST` / `CONFIG_LIGHTWEIGHT_POOLING` — **the deprecated Dashboard's `SqlServerFactCollector` is untouched** (it just never emits the facts). (1) **Priority boost enabled** (`install/50_configuration_issues_analyzer.sql:368`, WARNING) — above-normal Windows scheduling priority that starves OS-critical threads. (2) **Lightweight pooling / fiber mode enabled** (`install/50:401`, WARNING) — breaks OLEDB and other in-process components. Each scores the WARNING band (`0.9` in `FactScorer`) when `value_in_use == 1` — surfacing prominently, since these are rare, clearly-wrong settings rather than routine tuning choices like MAXDOP/CTFP (which stay at the low `0.4` config-advisory base) — and roots a standalone card (added to `InferenceEngine.ConfigAdvisoryRootKeys`). Sourced advice (the Dashboard messages + the `sp_configure ..., 0; RECONFIGURE;` remediation) added to `FactAdvice`. `FactScorerTests` in BOTH `Lite.Tests` and `Dashboard.Tests` pin the WARNING band when enabled and dormancy when disabled; the arms are silent on every existing fixture (no seeder enables either setting). Lands in Darling + Lite; the deprecated Dashboard is not developed further.
- **Analysis engine: two tempdb-deep issue-classification arms in the shared FactScorer (Tier-2 parity)** ([#1519]) — the #1517 companion: the shared `PerformanceMonitor.Analysis` scorer (ONE code path for Lite, the deprecated Dashboard, AND Darling) now classifies two tempdb problems the deprecated Dashboard's `report.tempdb_pressure` / `report.tempdb_contention_analysis` SQL flagged but the ported engine only partially covered. Both are PURE `FactScorer` additions over facts the collectors ALREADY emit — no collector, schema, or `install/*.sql` change — each citing its source SQL line in a code comment. (1) **Version-store pressure by ABSOLUTE size** — `TEMPDB_USAGE` previously scored the space-FRACTION only, blind to a multi-GB version store sitting in a barely-full tempdb (a long-running RCSI/snapshot transaction pinning row versions). It now scores the WORSE of space-fraction and version-store size, reading `max_version_store_mb` (already in the fact metadata — the same key `ComposeTempdbUsage` reads) and tiering it to `report.tempdb_pressure`'s `pressure_level` CASE (`install/47_create_reporting_views.sql` lines 1431-1433): > 5000 MB CRITICAL-tier, > 2000 MB HIGH, > 1000 MB MEDIUM; `tempdb_contention_analysis` corroborates the 1 GB bar (`version_store_high_warning`, install/47:2504). The tempdb recommendation card now names the version store as the driver whenever it clears the 1 GB bar — even if another consumer is nominally larger — mirroring the source view keying `pressure_level` purely on version-store size. (2) **Allocation / PFS-GAM-SGAM PAGELATCH contention** — `ScoreWaitFact` had PAGEIOLATCH + LATCH_EX/SH but no PAGELATCH arm. It now scores the `PAGELATCH_UP` wait fact by ABSOLUTE `wait_time_ms` (the same absolute-ms shape as the THREADPOOL gate), tripping at > 10000 ms -> MEDIUM per `tempdb_contention_analysis`'s `contention_level` CASE (install/47:2515) — read from the SAME server-wide `wait_stats` the view reads, so it is a faithful port; a new advice block (routed through the wait-family `ComposeWaitByKey`, so the card states the actual wait total) names the add-tempdb-files / TF 1118 fix (install/47:2522, 2535). Base severity maxes at 1.0 (WARNING) like every base fact — the CRITICAL band stays reserved for corroboration. NOTE: the view's stronger CRITICAL "allocation contention" signal (`allocation_contention_warning`, a tempdb-scoped `dm_os_waiting_tasks` snapshot computed in `install/34`) is NOT carried in any fact, so per the pure-scorer scope it was left to the PAGELATCH_UP wait rather than adding collection. `FactScorer` tests in BOTH `Lite.Tests` and `Dashboard.Tests` pin every tier (fires at the right band, stays silent below the bar, and the version-store arm never lowers a worse space-fraction score). No test-seeder fix was needed (unlike #1517's Query-Store default): the clean-server fixture seeds a 10 MB version store and no PAGELATCH waits, and the largest version store across all seeders is 900 MB — below the 1 GB bar. Behavior of the existing arms is unchanged.
- **Analysis engine: three more issue-classification arms in the shared FactScorer (Tier-2 parity)** ([#1517]) — the shared `PerformanceMonitor.Analysis` scorer/inference brain (ONE code path for Lite, the deprecated Dashboard, AND Darling) now classifies three problems the deprecated Dashboard's `report.*` / `config.critical_issues` SQL flagged but the ported engine had dropped. All three are pure `FactScorer` additions over facts the collectors ALREADY emit — no collector, schema, or `install/*.sql` change — so they light up in all three apps' recommendations at once, each citing its source SQL line in a code comment. (1) **Query Store disabled** — an INFO advisory folded into the existing `DB_CONFIG` finding, detected as `query_store_on_count < database_count` (user DBs only, system DBs excluded on both counts), mirroring `install/50_configuration_issues_analyzer.sql`'s INFO Query-Store-off issue (line 83); the DB-config recommendation now lists "N with Query Store off" and appends enable-QS guidance. (2) **Security-cache (TokenAndPermUserStore) growth** — a flat WARNING when the `USERSTORE_TOKENPERM` memory clerk reaches >= 1 GB, read from the otherwise context-only `MEMORY_CLERKS` fact (the clerk size rides in metadata keyed by clerk_type), mirroring `install/50`'s WARNING at `pages_kb / 1024 / 1024 >= 1.0` (lines 562, 583); a new recommendation names the `DBCC FREESYSTEMCACHE('TokenAndPermUserStore')` relief plus the ad-hoc / dynamic-SQL churn cause. (3) **Runnable-queue / worker pressure** — a STANDALONE arm rooting the collected `cpu_scheduler_stats` snapshot on `total_runnable_tasks_count`, tiered to `report.cpu_scheduler_pressure`'s `> 50 / > 20 / > 10` CASE plus the collector's `runnable_tasks_warning` small-box fallback (`install/47_create_reporting_views.sql` lines 1839-1844), additive to and distinct from the #1494 runnable-queue -> THREADPOOL amplifier (kept unchanged). Base severity maxes at 1.0 (WARNING) like every base fact — the CRITICAL band stays reserved for amplifier-corroborated cases (the runnable-queue -> THREADPOOL escalation). `FactScorer` tests in BOTH `Lite.Tests` and `Dashboard.Tests` pin every tier (fires when it should, stays silent below the bar, at the right band) and that a co-present worse `DB_CONFIG` issue is left unchanged. Behavior of the existing arms is unchanged.
- **Lite MCP: the 18 Darling-only tools ported (drains the MCP inventory ratchet to ZERO)** ([#1518]) — the mechanical `[McpServerTool]` companion to the #1504 / #1505 / #1509 viewer BUILDs: every tool the #1501 `CrossAppMcpToolInventoryPinTests` ratchet listed as Darling-only is now exposed by Lite's MCP host, so `KnownLiteMissingMcpTools` is **empty** (only the `get_blocked_process_reports` <-> `get_blocking` naming-drift pair remains, handled separately). The 18: `get_latch_stats`, `get_spinlock_stats`, `get_plan_cache_bloat`, `get_cpu_scheduler_pressure`, `get_resource_semaphore`, `get_default_trace_events`, `get_daily_summary`, `get_server_config_changes`, `get_database_config_changes`, `get_trace_flag_changes`, and the eight `get_health_parser_*` tools (system_health / severe_errors / io_issues / scheduler_issues / memory_conditions / cpu_tasks / memory_broker / memory_node_oom). Each is a thin wrapper over the reader Lite ALREADY had from the viewer builds — no new collection, no live-server hit: latch/spinlock wrap the latest-snapshot readers, plan-cache-bloat folds the true-total summary + composition grid + the shared `ClassifyPlanCacheBloat` banding, cpu-scheduler-pressure surfaces the latest scheduler snapshot with the collector's warning flags, daily-summary wraps the Performance Calendar aggregate (composite health band), the three config-change tools wrap the shared `ConfigChangeDiff` histories, and default-trace + the eight health-parser tools wrap the System Events parse-on-read readers (gated by the shared `SystemHealthSignificance` / `DefaultTraceEventSignificance` — the SAME significant set the viewer shows). The ONE reader added is `GetResourceSemaphoreSnapshotAsync` (a latest-snapshot read of the FULL semaphore ceiling columns — target / max-target / total workspace memory + cumulative timeout/forced-grant counts — which the existing per-pool `GetMemoryGrantChartDataAsync` chart reader doesn't surface), so Lite's `get_resource_semaphore` matches the Dashboard/Darling shape rather than duplicating `get_memory_grants`. A raw naive-UTC `EventTime` / `ChangeTime` accessor was added to the System Events + Config Changes viewer row DTOs so the tools emit ISO-8601 timestamps (matching every other Lite MCP tool) instead of the grids' display-local strings. All 18 are registered in `McpHostService` and documented in the MCP server instructions. Lite-only — no Darling, Full Dashboard, or `install/*.sql` changes.
Expand Down Expand Up @@ -316,6 +317,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
[#1513]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1513
[#1514]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1514
[#1515]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1515
[#1520]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1520
[#1519]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1519
[#1518]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1518
[#1516]: https://github.com/erikdarlingdata/PerformanceMonitor/pull/1516
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ AND configuration_name IN (
'max degree of parallelism',
'max server memory (MB)',
'min server memory (MB)',
'max worker threads'
'max worker threads',
'priority boost',
'lightweight pooling'
)
)
SELECT configuration_name, value_in_use
Expand Down Expand Up @@ -74,6 +76,8 @@ private async Task CollectServerConfigFactsAsync(AnalysisContext context, List<F
"max server memory (MB)" => "CONFIG_MAX_MEMORY_MB",
"min server memory (MB)" => "CONFIG_MIN_MEMORY_MB",
"max worker threads" => "CONFIG_MAX_WORKER_THREADS",
"priority boost" => "CONFIG_PRIORITY_BOOST",
"lightweight pooling" => "CONFIG_LIGHTWEIGHT_POOLING",
_ => null
};

Expand Down
49 changes: 49 additions & 0 deletions Dashboard.Tests/FactScorerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,55 @@ public void Score_MemoryClerks_NoSecurityCacheClerk_DoesNotScore()
Assert.Equal(0.0, facts.First(f => f.Key == "MEMORY_CLERKS").Severity, precision: 4);
}

// batch-2b — priority boost enabled is a Dashboard WARNING (install/50:368). CONFIG_PRIORITY_BOOST
// bands WARNING (0.9) when value_in_use == 1, and scores 0 when disabled.
[Fact]
public void Score_ConfigPriorityBoost_Enabled_BandsWarning()
{
var facts = new List<Fact>
{
new() { Source = "config", Key = "CONFIG_PRIORITY_BOOST", Value = 1, Metadata = new() { ["value_in_use"] = 1 } },
};
new FactScorer().ScoreAll(facts);
var f = facts.First(f => f.Key == "CONFIG_PRIORITY_BOOST");
Assert.True(f.Severity >= 0.75 && f.Severity < 1.5, "priority boost bands WARNING when enabled");
}

[Fact]
public void Score_ConfigPriorityBoost_Disabled_DoesNotScore()
{
var facts = new List<Fact>
{
new() { Source = "config", Key = "CONFIG_PRIORITY_BOOST", Value = 0, Metadata = new() { ["value_in_use"] = 0 } },
};
new FactScorer().ScoreAll(facts);
Assert.Equal(0.0, facts.First(f => f.Key == "CONFIG_PRIORITY_BOOST").Severity, precision: 4);
}

// Lightweight pooling enabled is a Dashboard WARNING (install/50:401) — same shape.
[Fact]
public void Score_ConfigLightweightPooling_Enabled_BandsWarning()
{
var facts = new List<Fact>
{
new() { Source = "config", Key = "CONFIG_LIGHTWEIGHT_POOLING", Value = 1, Metadata = new() { ["value_in_use"] = 1 } },
};
new FactScorer().ScoreAll(facts);
var f = facts.First(f => f.Key == "CONFIG_LIGHTWEIGHT_POOLING");
Assert.True(f.Severity >= 0.75 && f.Severity < 1.5, "lightweight pooling bands WARNING when enabled");
}

[Fact]
public void Score_ConfigLightweightPooling_Disabled_DoesNotScore()
{
var facts = new List<Fact>
{
new() { Source = "config", Key = "CONFIG_LIGHTWEIGHT_POOLING", Value = 0, Metadata = new() { ["value_in_use"] = 0 } },
};
new FactScorer().ScoreAll(facts);
Assert.Equal(0.0, facts.First(f => f.Key == "CONFIG_LIGHTWEIGHT_POOLING").Severity, precision: 4);
}

// ARM 1 — Query Store off on a user database is an INFO advisory carried by the DB_CONFIG fact,
// detected as query_store_on_count < database_count (install/50:83 severity=INFO). Low 0.3 base =
// INFO band, and DB_CONFIG roots at any positive severity (a ConfigAdvisoryRootKey).
Expand Down
49 changes: 49 additions & 0 deletions Lite.Tests/FactScorerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -639,6 +639,55 @@ public void Score_MemoryClerks_NoSecurityCacheClerk_DoesNotScore()
Assert.Equal(0.0, facts.First(f => f.Key == "MEMORY_CLERKS").Severity, precision: 4);
}

// batch-2b — priority boost enabled is a Dashboard WARNING (install/50:368). CONFIG_PRIORITY_BOOST
// bands WARNING (0.9) when value_in_use == 1, and scores 0 when disabled.
[Fact]
public void Score_ConfigPriorityBoost_Enabled_BandsWarning()
{
var facts = new List<Fact>
{
new() { Source = "config", Key = "CONFIG_PRIORITY_BOOST", Value = 1, Metadata = new() { ["value_in_use"] = 1 } },
};
new FactScorer().ScoreAll(facts);
var f = facts.First(f => f.Key == "CONFIG_PRIORITY_BOOST");
Assert.True(f.Severity >= 0.75 && f.Severity < 1.5, "priority boost bands WARNING when enabled");
}

[Fact]
public void Score_ConfigPriorityBoost_Disabled_DoesNotScore()
{
var facts = new List<Fact>
{
new() { Source = "config", Key = "CONFIG_PRIORITY_BOOST", Value = 0, Metadata = new() { ["value_in_use"] = 0 } },
};
new FactScorer().ScoreAll(facts);
Assert.Equal(0.0, facts.First(f => f.Key == "CONFIG_PRIORITY_BOOST").Severity, precision: 4);
}

// Lightweight pooling enabled is a Dashboard WARNING (install/50:401) — same shape.
[Fact]
public void Score_ConfigLightweightPooling_Enabled_BandsWarning()
{
var facts = new List<Fact>
{
new() { Source = "config", Key = "CONFIG_LIGHTWEIGHT_POOLING", Value = 1, Metadata = new() { ["value_in_use"] = 1 } },
};
new FactScorer().ScoreAll(facts);
var f = facts.First(f => f.Key == "CONFIG_LIGHTWEIGHT_POOLING");
Assert.True(f.Severity >= 0.75 && f.Severity < 1.5, "lightweight pooling bands WARNING when enabled");
}

[Fact]
public void Score_ConfigLightweightPooling_Disabled_DoesNotScore()
{
var facts = new List<Fact>
{
new() { Source = "config", Key = "CONFIG_LIGHTWEIGHT_POOLING", Value = 0, Metadata = new() { ["value_in_use"] = 0 } },
};
new FactScorer().ScoreAll(facts);
Assert.Equal(0.0, facts.First(f => f.Key == "CONFIG_LIGHTWEIGHT_POOLING").Severity, precision: 4);
}

// ARM 1 — Query Store off on a user database is an INFO advisory carried by the DB_CONFIG fact,
// detected as query_store_on_count < database_count (install/50:83 severity=INFO). Low 0.3 base =
// INFO band, and DB_CONFIG roots at any positive severity (a ConfigAdvisoryRootKey).
Expand Down
6 changes: 5 additions & 1 deletion Lite/Analysis/DuckDbFactCollector.Config.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ AND configuration_name IN (
'max degree of parallelism',
'max server memory (MB)',
'min server memory (MB)',
'max worker threads'
'max worker threads',
'priority boost',
'lightweight pooling'
)
)
SELECT configuration_name, value_in_use
Expand Down Expand Up @@ -72,6 +74,8 @@ FROM latest
"max server memory (MB)" => "CONFIG_MAX_MEMORY_MB",
"min server memory (MB)" => "CONFIG_MIN_MEMORY_MB",
"max worker threads" => "CONFIG_MAX_WORKER_THREADS",
"priority boost" => "CONFIG_PRIORITY_BOOST",
"lightweight pooling" => "CONFIG_LIGHTWEIGHT_POOLING",
_ => null
};

Expand Down
16 changes: 16 additions & 0 deletions PerformanceMonitor.Analysis/FactAdvice.cs
Original file line number Diff line number Diff line change
Expand Up @@ -957,7 +957,7 @@
string? best = null;
double bestMs = 0;
foreach (var kv in f.Metadata)
if (kv.Key.EndsWith("_ms") && !standard.Contains(kv.Key) && kv.Value > bestMs)

Check warning on line 960 in PerformanceMonitor.Analysis/FactAdvice.cs

View workflow job for this annotation

GitHub Actions / build

The behavior of 'string.EndsWith(string)' could vary based on the current user's locale settings. Replace this call in 'PerformanceMonitor.Analysis.FactAdvice.DominantLockMode(PerformanceMonitor.Analysis.Fact)' with a call to 'string.EndsWith(string, System.StringComparison)'. (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1310)
{
best = kv.Key;
bestMs = kv.Value;
Expand Down Expand Up @@ -2015,6 +2015,22 @@
Remediation:
"For tempdb-driven PAGEIOLATCH_EX, fix the spilling queries — update statistics with FULLSCAN to correct the cardinality estimates that produced too-small grants, or rewrite the operator that spills. For modification-workload pressure, batch large operations into smaller chunks so the buffer pool can flush between batches. If the storage itself is slow (write latency consistently above 10 ms on data, 2 ms on log), no amount of query tuning will recover it — the storage is the bottleneck and needs hardware-side investigation.");

t["CONFIG_PRIORITY_BOOST"] = new AdviceBlock(
Headline:
"Priority boost is enabled — SQL Server threads run at above-normal Windows scheduling priority, which can starve OS-critical threads",
Investigation:
"`sp_configure 'priority boost'` raises the Windows scheduling priority of SQL Server's threads above normal. Microsoft recommends against it for essentially all servers: it can starve OS threads (network, disk, cluster heartbeat), destabilize the instance, and cause connectivity or failover problems — with no reliable throughput gain. It is almost always a leftover from bad tuning advice rather than a deliberate, measured choice.",
Remediation:
"Disable it: `EXECUTE sp_configure 'priority boost', 0; RECONFIGURE;` — the change takes effect after a SQL Server service restart. There is effectively no workload for which priority boost is the right fix.");

t["CONFIG_LIGHTWEIGHT_POOLING"] = new AdviceBlock(
Headline:
"Lightweight pooling (fiber mode) is enabled — it breaks OLEDB and other in-process components and is not recommended",
Investigation:
"`sp_configure 'lightweight pooling'` (fiber mode) switches SQL Server to fiber-based scheduling. It disables features that assume thread-based scheduling — most OLEDB providers (linked servers, some CLR, extended stored procedures) fail or misbehave — and its narrow high-end NUMA / context-switch benefit almost never applies. Microsoft recommends leaving it off for essentially all workloads.",
Remediation:
"Disable it: `EXECUTE sp_configure 'lightweight pooling', 0; RECONFIGURE;` — takes effect after a service restart. Only consider re-enabling it under a specific, measured high-end OLTP/NUMA scenario with vendor guidance, and never with linked servers / OLEDB in use.");

t["RESOURCE_SEMAPHORE"] = new AdviceBlock(
Headline:
"RESOURCE_SEMAPHORE waits — queries are queueing for memory grants because the workspace pool is exhausted",
Expand Down
Loading
Loading