Repository navigation
Darling reads a wait stored under both spellings as one wait under its clean name - #4941
Merged
erikdarlingdata merged 4 commits intoOct 2, 2026
Merged
Conversation
…-space-history-darling
…s clean name SQL Server reports four wait names with a trailing space, and from 3.9 the collector stores them trimmed (#4884). History from before the upgrade keeps the spaced name, so the same wait could be stored under two spellings. Reads that group by wait now key on rtrim(wait_type): the viewer's wait picker, wait trends, current-waits trend and FinOps category summary, the MCP wait stats, wait types and current-waits trend, the daily summary's top wait, the analysis wait facts and the anomaly wait contributors. Lookups by name keep the column bare and match both spellings: the MCP wait trend and the viewer's queries-by-wait drill-down use wait_type IN ($n, $n || ' '). Custom Views group the three SQL Server wait-name dimensions on the trimmed name and widen each filter value instead of wrapping the column. pg_wait_stats (PostgreSQL wait events) is unchanged.
…-space-history-darling
…ellings on rtrim rtrim in GROUP BY ran once per row and cost 22-30% on the 7-day wait stats and 10-day daily summary reads. The six wide grouping reads now keep the old per-name aggregation inside and merge the two spellings over those groups, which measures within a few percent of the read before #4941. The FinOps wait categories read the category from the clean name. Tests: gte/lt guard cases and a top-N time series with "(other)" in the live spaced-history class; the SQL text tests follow the two-level shape.
erikdarlingdata
marked this pull request as ready for review
October 2, 2026 09:46
erikdarlingdata
deleted the
fix/wait-name-trailing-space-history-darling
branch
October 2, 2026 09:46
erikdarlingdata
added a commit
that referenced
this pull request
Oct 2, 2026
The 3.9.0 entry for the wait-name fix (#4884) now covers #4939 and #4941, which completed it after the release was cut: its references gain both PRs in the index and the archive, rows stored with the trailing space before the upgrade are described as reading as the clean name in both apps, and the entry says Lite's wait views hide the two newly ignored waits in history from before the upgrade while Darling shows that history until retention removes it. The archive census carries the new 3.9.0 prose hash; the entry count is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes
From 3.9 the collector trims wait names before it stores them (#4884). SQL Server reports four wait names with a trailing space. A wait that accrued time under both spellings has its history from before the upgrade stored as
NAMEand its newer rows stored asNAME. Darling's reads keyed on the stored text, so the same wait showed as two rows or two series, each with part of its time. A top-wait pick compared those parts, not the whole wait, and a lookup by the clean name lost the older history.Now every Darling read that groups, filters or looks up by SQL Server wait name treats both spellings as one wait. It shows that wait under the clean name. There is no schema, migration or continuous-aggregate change.
wait_stats: two levels. The inner query sums per stored name, exactly as before. The outer query adds those sums up perrtrim(wait_type), so the trim runs once per group, not once per row. "Grouping cost" below has the measurements.waiting_tasks:rtrim(wait_type) AS wait_typein the select list, andrtrim(wait_type)inGROUP BYandPARTITION BY.wait_type IN ($n, $n || ' '), and a list of several names adds the spaced form of each one.TrailingSpaceHistory, marks the three SQL Server wait-name dimensions (wait_stats,waiting_tasksandquery_snapshots). The compiler groups them onrtrim(f.wait_type)and keeps the column bare in every filter. It widens the value instead (details below). Thepg_wait_statsdimension holds PostgreSQL wait events, so it does not change.The trailing space
A SELECT-only check of
sys.dm_os_wait_statson SQL Server 2022 and 2025 found these four names. Each one ends in exactly one trailing character, U+0020:EXTERNAL_GOVERNANCE_ATTR_SYNC_BACKGROUND,EDC_DOPP_LOCK,EDC_DOPP_BACKGROUNDandSQP_STATS_REPORTING. No other name in that DMV ends in whitespace, sonameandname + ' 'cover every spelling the store can hold.The same check found five names that end in a comma and one name with a space inside it. They were checked and are out of scope. No list in the code holds any of them. The trim does not change them, so the store holds each one with a single spelling.
Scope
Observed:
SQP_STATS_REPORTINGat collection throughIgnoredWaitDefaults.All, so it never stores a clean twin of that one. By design, Darling applies its ignore list at collection only, and its reads have none. So stored history of the spaced form is unchanged by this PR. The history still shows, and it is now labelled with the clean name.Inferred:
EXTERNAL_GOVERNANCE_ATTR_SYNC_BACKGROUND,EDC_DOPP_LOCKorEDC_DOPP_BACKGROUNDgaining wait time on a server both before and after its upgrade.Every read checked
Changed (13 reads, plus Custom Views). "Two levels" means
GROUP BY wait_typeinside, thenrtrim(wait_type) AS wait_typeandGROUP BY rtrim(wait_type)over those groups.ViewerDataService.DistinctWaitTypesSqlViewerDataService.WaitTrendsSqlwait_type IN ($4, $4 || ' ', $5, $5 || ' ', ...).rtrimin the select list and theLAGPARTITION BY, so the outer grouping reads the clean nameViewerDataService.WaitingTaskTrendSqlrtrimin the select list,GROUP BYandORDER BYViewerDataService.WaitCategorySummarySqlCASEnow reads the clean name, so both spellings always land in one category. None of the four names matches a category, so they land in Other, as beforeViewerDataService.QuerySnapshotsByWaitTypeSqlwait_type IN ($4, $4 || ' ')get_wait_statsDarlingDataReader.WaitStatsSqlget_wait_typesDarlingDataReader.DistinctWaitTypesSqlget_wait_trend, both the per-collection and the bucketed formDarlingDataReader.WaitRawCtewait_type IN ($2, $2 || ' '). One wait per call, so theLAGneeds no partition and runs across the spelling changeget_current_waits_trendDarlingDataReader.WaitingTaskTrendSqlrtrimin the select list,GROUP BYandORDER BYDailySummarySql.RangeSqlwait_per_spellingsums per day and stored name, thenwait_per_typeadds those sums up per day andrtrim(wait_type). The routed forms swap only the queries CTE, so they get it tooPgFactCollector.WaitStatsSqlPgAnomalyDetector.WaitContribWindowSqlwait_stats,waiting_tasksandquery_snapshotsComposeCompiler(GroupRef,BuildFilterClause)GROUP BY, the top-N membership test, and the series select list andGROUP BYusertrim(f.wait_type). Filters keepf.wait_typebareCustom Views filters on a wait-name dimension:
eqandneqbind each value and the value plus one space:f.wait_type = ANY($n)andf.wait_type <> ALL($n).likebecomes(f.wait_type LIKE $n OR f.wait_type LIKE $n || ' '), so an exact pattern also matches the spaced spelling.gtbecomes(f.wait_type > $n AND f.wait_type <> $n || ' '), andltebecomes(f.wait_type <= $n OR f.wait_type = $n || ' ').gteandltstay as they were, because a name with one trailing space sorts directly after its clean form. Two guard tests hold this in place (see "Tests"). CI creates its PostgreSQL cluster without--locale(build.yml:1005), and the managed store'sinitdbpasses--locale=C(DarlingManagedPostgres.cs:3440). The order was checked under the C, musl, glibcen_US.UTF-8, ICU and Windows collations.Checked, no change:
LCK%only:ViewerDataService.LockWaitTrendSql,DarlingBlockingTrendReader(LockWaitTrendSql,LockWaitTypesSql) andPgDrillDownCollector.LockModeBreakdownSql.DarlingAlertReadAdapter.PoisonWaitsSql(THREADPOOL,RESOURCE_SEMAPHORE*)WAITFOR,BACKUP*,XE_LIVE_TARGET_TVF,SP_SERVER_DIAGNOSTICS)PgAnomalyDetector.YoungBaselineBarPeakSqlQueryStoreClutter(QDS\_%only)DarlingSessionReader(ActiveQueriesSql,WaitingTasksSql)PgPileupSnapshotReaderPgDrillDownCollector.QueriesAtSpikeSqlSameStatementPileupDetectorTotalWaitTrendSqlwait_stats_interval_baselineand the olderwait_stats_baseline)signal_wait_pctcustom-alert templateDarlingDeltaCalculator.WaitStatsSeedSql: Wait names lose their trailing space, and two Hyperscale timer waits are ignored #4884 already accepts the first trimmed sample as a new baseline.dmv_blocking_snapshot: its collector never trimmed, so it holds one spelling before and after.DarlingFleetReader: it has no wait read. Its onlywait_typereference is the mute-rule column testm.wait_type_pattern IS NULL.ResolveStoredSnapshotForActualPlanSql: it does not touchwait_type.Skipped, because these are PostgreSQL wait events and not SQL Server wait names:
PgTargetAnomalyDetector,PgTargetBaselineProvider,PgTargetFactCollector.Waits,DarlingPgWaitReader,DarlingPostgresAlertReadAdapter, and the Custom Viewspg_wait_stats.wait_typedimension.Query plan for the lookup
wait_typehas no index of its own. The plan check used a fresh PostgreSQL 18.6 database with TimescaleDB 2.30.1:EDC_DOPP_LOCKwas stored with the space for the first 5 days and clean after that.The query is the
get_wait_trendread for one server over an 8-day window, run once with the oldwait_type = $2and once with the newwait_type IN ($2, $2 || ' ').Both forms take the same path on every chunk:
server_idindex.(server_id, collection_time)index.wait_typeis only ever a filter, and no plan has a sequential scan.A first run with 5 servers (576,200 rows) took another path on the compressed chunks, the same one for both forms. Each compressed chunk then held about 60 batch rows, so both forms read those small tables sequentially. The uncompressed chunks used the
collection_timeindex. With 50 servers, both forms use theserver_idindex shown here.The old form returns 1,441 rows, the clean days only. The new form returns 2,305, both spellings. With
plan_cache_mode = force_generic_planthe two forms again match each other. Compressed chunks use the same index, and uncompressed chunks use the chunk'scollection_timeindex, withserver_idandwait_typeas filters.Old form, trimmed to the scan nodes:
New form, trimmed to the scan nodes:
The viewer drill-down lookup uses the same
INform onquery_snapshots, whose indexes have the same shape:(server_id, collection_time)and(collection_time DESC).Grouping cost
rtriminGROUP BYruns once per row. The two-level form keeps the old per-chunk aggregation and trims once per group. Each form ran on the same 5,762,000-row database, for one server, withEXPLAIN (ANALYZE, BUFFERS)andplan_cache_mode = force_custom_plan. After one warm-up, each form ran 3 times, with the forms interleaved. The table shows the median:DarlingDataReader.WaitStatsSqlDailySummarySql.RangeSqlThat first set timed a two-level
WaitStatsSqlwith other inner column names. A second set used the exact shipped text and kept the same order. It gave 32.5, 39.9 and 31.3 ms forWaitStatsSql, and 45.1, 57.0 and 41.9 ms for the range read.The one-level form was more than 10% slower in both reads. So every wide grouping read over
wait_statsnow uses two levels:DistinctWaitTypesSqlreadsDarlingDataReader.WaitStatsSqlandPgFactCollector.WaitStatsSqlPgAnomalyDetector.WaitContribWindowSqlDailySummarySqlViewerDataService.WaitCategorySummarySqlEach outer aggregate is a sum of the inner sums, so it adds up to the same total. The one-level and two-level forms returned the same rows for all 50 servers. That is 2,000 rows for
WaitStatsSqland 500 for the range read, with no difference either way (EXCEPT ALLon the row text).No form has a VectorAgg node on this TimescaleDB 2.30.1 store. All three keep the per-chunk partial aggregation. Plan nodes from the second set, trimmed, with chunk numbers shown as N:
Still one level:
ViewerDataService.WaitTrendsSqlfilters on the chosen names first, so it trims only their rows.WaitingTaskTrendSqlreads groupwaiting_tasks, which holds only the tasks waiting at each collection, not a row for every wait type.Tests
New
WaitNameSpacedHistoryLivePostgresTests: 21 tests, all_AgainstDevPostgres, so CI's PostgreSQL jobs run them. They cover three things:They failed on the code without the fix, 16 of 16, each on its assertion:
The
gtandltecases came later, with the range-operator handling. They failed when the flag was turned off on thewait_statsdimension, which compiles the old filter SQL:The top-N time series test came last. A spaced and a clean row of one wait sit in the top N. With the flag off, the read ranks one spelling at a time. Each spelling (300 ms) loses the one slot to the rival (500 ms), and the whole wait folds into "(other)". It failed that way:
The
gteandltcases are guards, not red-first tests. Those two operators compile the same with the flag on or off, and both cases pass either way:gteon the clean name keeps its spaced history.lton the clean name drops both spellings of that name, and keeps a lower name,EDC_DOPP_BACKGROUND, in both spellings.With the fix, all 21 pass against a migrated PostgreSQL 18.6 database with TimescaleDB 2.30.1.
Also:
DarlingComposeTestscheck four things:rtrimeqandneqbind both spellingspg_wait_statshas nortrim, and the catalog flags exactly the three SQL Server wait-name dimensionsDarlingMcpDataToolsTests,ViewerDrillDownTests,ViewerWaitStatsTestsand the two top-N series tests inDarlingComposeTests.Darling.Testsrun without a PostgreSQL connection:CHANGELOG
No new entry: this amends the [3.9.0] #4884 line, and the release owner applies it.
Proposed sentence for Darling: