Convert the Viewer read-path teardowns; ratchet 87 -> 19 (#1902, batch 2 of 3)

[erikdarlingdata]

Batch two of three for #1902. Does not close it — 19 sites remain and the ratchet tracks them.

The tranche

The whole Viewer* read-path family: 68 teardowns across 21 classes. Taken as one tranche because they are one shape — plant rows for a viewer query, read them back, delete them in a finally that ran on the body's connection, so a test that failed for a real reason reported its teardown's error instead.

Class	Sites		Class	Sites
ViewerQueriesTests	10		ViewerPlanHostTests	3
ViewerBlockingDepthTests	7		ViewerConfigChangesTests	2
ViewerFileIoBlockingTests	7		ViewerLatchSpinlockTests	2
ViewerDailyHealthTests	6		ViewerOverviewLanesTests	2
ViewerMemoryTests	5		ViewerWaitStatsTests	2
ViewerConfigurationTests	4		ViewerBlockingStatsTests	1
ViewerCpuTempDbTests	4		ViewerCpuSchedulerTests	1
ViewerQueriesRestTests	4		ViewerDeadlockSeverityStatsTests	1
ViewerPerfmonRunningJobsTests	3		ViewerFleetRollupTests	1
			ViewerPlanCacheTests	1
			ViewerRecommendationsTests	1
			ViewerSessionStatsTests	1

61 sites were the single-call shape, 7 delete from two tables. Both go through LiveStoreCleanup.RunAsync taking the cleanup connection and its token.

New Ceiling: 19 (was 87).

Every one of the 21 had the token gotcha

25 delete helpers reached for TestContext.Current.CancellationToken internally. Under LiveStoreCleanup — which passes CancellationToken.None deliberately, so a cancelled run still restores the shared store — such a helper re-signals itself on precisely the path it exists to survive, and skips the delete. All 25 take a token now.

None of those 21 files declares a local ct at all; they use the ambient token inline throughout. So their body call sites pass it explicitly rather than introducing a mixed idiom inside one method. Cleanup call sites take cleanupCt.

A flake this batch's runs surfaced, fixed here

StuckCompressionJobsSql_NeverRunJob_ReadsNullLastRunStartedAt failed on the second full-suite run, in TimescaleSupportTests — a file this batch does not otherwise touch (git diff --name-only matched it zero times before the fix).

It pins the #1760 sentinel: job_stats.last_run_started_at reads -infinity, not NULL, for a job that has never executed, and the production query must neutralise that or the stuck-Running arm fires on a job that never ran. The test created its policy unparked, and add_compression_policy creates the job SCHEDULED with no initial_start, so TimescaleDB launches it within a second or two (#1788). One background launch destroys the sentinel outright — unlike a chunk count, no amount of re-reading recovers it.

#1889 fixed exactly this for three sibling tests by creating and parking in ONE transaction, so the scheduler (a separate backend) cannot see the job until the row already reads scheduled = false. This was the fourth site, and it now uses the same helper — a one-line swap to a helper already in the file, written for this race.

#1888 predicted it when it raised CI's worker slots: "any other test that has been quietly relying on the scheduler being unable to run is going to start failing … whether others exist is unknown, and finding out is the actual work here."

Proven both ways rather than by re-running until green:

Shape	4-second scheduler window	Result
Old, unparked	yes	fails Expected: True / Actual: False — byte-identical to the intermittent full-suite failure
New, parked	yes, identical	passes 3/3

So the fix closes the window rather than narrowing it. It is a separate commit (3f56c20e) from the mechanical batch.

No permanent guard is added, and that is deliberate. Three other live tests in the same file create unparked policies BY DESIGN — an already-armed legacy policy is exactly what the converge tests converge — so any source-level rule would either fire on those or be narrowed until it pinned nothing. A brittle guard here would be worse than none.

Test plan

PostgreSQL 18.4 + TimescaleDB 2.28.1 from the repo's pg-runtime.zip, throwaway cluster on port 5967 with max_worker_processes = 24 (mirroring the product's sizing, per #1888 — which is why the sentinel flake was reproducible here at all), CI's connection-string shape.

• Three full gated-live runs on brand-new databases: 3999 passed, 0 failed, 10 skipped, exit code 0 read from dotnet test itself, zero collection-cleanup failures — Live tests can strand a continuous aggregate in the shared DARLING_TEST_PG store, and the best-effort DROP swallows the reason #1873's residue check stayed quiet across all 68 conversions. (Three rather than two: the first pair bracketed the sentinel flake, so the post-fix state was re-run from scratch.)
• Ratchet green at 19, both directions.
• Watched-red for the sentinel fix, above.
• Lite suite 1849/0. Both suites -t:Rebuild, 0 warnings, 0 errors. Installer.Tests not run.

Deviations and filings

Nothing filed. The one defect discovered was fixed here rather than deferred — it is a one-line mirror of a pattern already in the same file, and #1888's own text names finding these as the work. The remaining 19 sites are #1902's declared scope for batch three.

The skip count moved 9 → 10 against dev; that is a gated test dev added, not this branch.

Test-suite only — no shipped code changed.

[claude]

Review

Reviewed the full diff (23 files, 68 teardown conversions across 21 Viewer* classes + the TimescaleSupportTests flake fix). This is test-infrastructure-only — no shipped SQL/product code changed — so the "no missing-index DMV folklore" and T-SQL style-guide concerns don't really apply here; focused on correctness of the mechanical conversion, the ratchet, and the flake fix instead.

Verification performed (not just eyeballing the diff):

• Cross-checked every file's LiveStoreCleanup.RunAsync call count against the PR description's per-class table — all 21 match exactly (e.g. ViewerQueriesTests 10, ViewerBlockingDepthTests 7, ViewerFileIoBlockingTests 7, down to the 1-site classes).
• Grepped every bodySucceeded = true; occurrence (68 total) — in every single case it's the last statement in the try block, immediately before finally. No case where it's set before further-risky code follows, which would have silently defeated the masking rule in LiveStoreCleanup.RunAsync's catch when (!bodySucceeded).
• Confirmed LiveStoreCleanup.RunAsync/RunOwnedAsync themselves are untouched by this PR (pre-existing from Sixteen live-test finally blocks still clean up on the body's connection and throw, so a failing test can report the cleanup's error instead of its own #1896) — this batch only calls into them.
• Confirmed the TimescaleSupportTests change reuses the pre-existing AddCompressionPolicyParkedAsync helper (from Flaky on dev: the compression-isolation test races the background policy job add_compression_policy starts, failing as 2/3 or 1/0 #1889) rather than reintroducing new parking logic — it's genuinely a one-line swap as claimed, and the helper's "create + park in one transaction" behavior checks out against the Rollups serve only what they materialized: route by coverage, and back fill behind a disk preflight #1788 race it's closing.
• Confirmed the fully-qualified System.Threading.CancellationToken ct parameter style (rather than adding a using System.Threading;) matches the convention already used in batch-one files (DarlingMcpHealthToolsTests.cs, ViewerW2aTests.cs, etc.), so it's not a new inconsistency.
• Confirmed the ratchet mechanics: Ceiling dropped 87 → 19, and TheRatchet_IsNotLeftSlackAfterABatch enforces equality (not just <=), so this can't quietly leave slack.

Lite/Darling parity: No parity gap. Lite.Tests has no Viewer*Tests naming family at all — the Viewer* live-Postgres teardown classes are Darling-specific, so there's no Lite counterpart that also needed this conversion.

Correctness/security: Nothing found. Each DeleteRowsAsync-style helper's signature change (dropping the internal TestContext.Current.CancellationToken reach-in for a passed-in ct) is applied uniformly, and every call site correctly threads TestContext.Current.CancellationToken for setup-phase deletes (still on the test's own connection/token) vs. cleanupCt for the finally-phase deletes (via the fresh LiveStoreCleanup connection with CancellationToken.None underneath). No SQL string interpolation changes — the existing server_id = {id} interpolations are all int-typed args, not user input, so no injection surface introduced or changed.

No findings to raise. This is a clean, well-verified mechanical batch.