Speed up Lite.Tests: share one DuckDB fixture per analysis-heavy test class - #1693
Merged
Conversation
The seven classes behind CI's "Run Lite analysis-heavy tests" step each created a fresh DuckDB database inside every test and laid down the full Lite schema (36 collector tables + indexes, archive views, analysis schema - ~80 DDL statements) before doing any work. A new SharedDuckDbFixture (xUnit IClassFixture) builds the database once per class; each test resets DATA only, from the test class constructor - every base table emptied except the schema_version / analysis_schema_version stamps, so the database keeps reading as current (v48) rather than as a blank file needing migration. Per-class fixtures deliberately, NOT a collection fixture: a single shared collection would serialize the classes against each other and give back most of the win; per-class databases keep xUnit's cross-class parallelism intact. Isolation was proven, not assumed: the reset covers ALL base tables, including analysis_findings / analysis_muted, which TestDataSeeder's server-scoped clear never touched (persisted findings and mute rules can no longer leak between tests); none of the seven classes mutates schema, touches parquet, or resets the database file; and the full suite was run twice consecutively with identical results (1479/1479 both times). TestDataSeeder itself is untouched - its per-scenario clears are load-bearing for tests that re-seed mid-test. Also removes the ~60 per-test InitializeAnalysisSchemaAsync calls, which were duplicate work even before this change - InitializeAsync already ends by calling it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ure-speedup # Conflicts: # CHANGELOG.md
erikdarlingdata
enabled auto-merge
July 26, 2026 18:16
This was referenced Jul 26, 2026
pull Bot
pushed a commit
to ehtick/PerformanceMonitor
that referenced
this pull request
Jul 29, 2026
The heavy seven got SharedDuckDbFixture in erikdarlingdata#1693; a full-suite trx profile showed 400s+ of serial weight remaining in fast-bucket classes with the same per-test disease: a fresh DuckDB + full schema build per test, and in several, per-row single-connection seeding. Converted (fixture + data-only reset per test; per-instance shared seed connection where the class hand-rolls inserts): IndexObjectStats, PerformanceCalendarData, StoreRoundTrip, SystemEventsReader, BlockingStatsReader, ConfigChangesReader, CollectionHealthWindow, QueryStatsModuleAttributionReader, FinOps, FactScorer, FindingStore, AnalysisNotification, DatabaseFilter, DailySummaryHighCpu, McpStatusEnvelope, McpAnalysisFindingsCommand, and the four appender round-trip classes (CollectorPlanColumn, CpuSchedulerPlanCache, SessionSummaryStats, SystemHealthEvents), which previously built one database PER TABLE PER TEST. Special handling: - FindingStoreTests' analysis-schema v3->v4 migration test hand-creates the legacy table shape, so it keeps its own private database file; its 15 siblings share the fixture. - The two MCP classes keep a test-local temp dir for the ServerManager config directory; only the database is shared. - The [Collection("server-time-helper")] trio keeps its collection. Deliberately NOT converted (each needs a private database by design): ArchiveViewDedupTests (writes parquet + rebuilds archive views), MuteRulesSurviveResetTests (ArchiveAllAndResetAsync deletes the DB file), DuckDbSchemaTests (tests schema creation/idempotency itself), and the three TestAlertDataHelper consumers (AlertHistorySource, DismissedArchiveSidecar, DismissReliability - the helper writes parquet into the DB's archive dir and rebuilds views). Measured (full-suite trx, same box): total serial work 925s -> 760s; IndexObjectStats 60.6s -> 20.8s, SystemEventsReader 36.2s -> 13.8s, PerformanceCalendarData off the top-15. Full suite run twice consecutively: identical 1494/1494 results, zero warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pull Bot
pushed a commit
to ehtick/PerformanceMonitor
that referenced
this pull request
Jul 29, 2026
The split existed because the seven analysis classes rebuilt the full DuckDB schema inside every test and their subset alone cost ~9 CI minutes, so a narrower lite_analysis path gate let non-analysis Lite changes skip it. After the shared class fixtures (erikdarlingdata#1693, erikdarlingdata#1698) and batched seeding (erikdarlingdata#1694), that subset runs in ~66s on the same runner - the split no longer earns its second test-host spin-up, and the hand-maintained class-name filters were a drift risk (a renamed class would silently fall out of the heavy filter and into fast). One "Run Lite tests" step now runs the whole suite, gated on the lite path filter; the unconsumed lite_analysis filter block is removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Summary
Share one DuckDB database per test CLASS in the seven analysis-heavy Lite.Tests classes, instead of creating a database and laying down the full schema (36+ collector tables + indexes + archive views + analysis schema, ~80 DDL statements) inside every single test. A new
SharedDuckDbFixture(xUnitIClassFixture) builds the schema once per class; each test starts by resetting DATA only - every base table is emptied except theschema_version/analysis_schema_versionstamps, so the database keeps reading as current between tests rather than as a blank file needing migration.Deliberately
IClassFixture, NOT a collection fixture: a single shared collection would serialize the seven classes against each other and hand back most of the win. Each class owns its own database file, so xUnit's cross-class parallelism is untouched.Classes converted (exactly the CI "analysis-heavy" filter):
AnomalyDetectorTestsFactCollectorTestsFactCollectorMiseryTestsBaselineProviderTestsInferenceEngineTestsScenarioTestsAnalysisServiceTestsTestDataSeederis untouched: its per-scenarioClearTestDataAsync()calls are load-bearing (tests likeMuteFinding_ExcludesFromNextRunre-seed mid-test and rely on the scoped clear). The fixture's reset list is a runtimeinformation_schemasnapshot, not a hand-maintained list - the AG collector tables that landed on dev mid-branch were picked up with zero changes.Also removes the ~60 per-test
InitializeAnalysisSchemaAsync()calls, which were duplicate work even before this change (InitializeAsync()already ends by calling it). Net -187 lines.Measurements (local, Debug, --no-build)
The win scales with CPU contention, because what the fixture eliminates is 153 redundant schema builds whose cost balloons when cores are scarce:
On an idle desktop the per-test schema build only costs ~0.2-0.3s, so the remaining runtime is genuine per-test analysis work (row-by-row seeding + pipeline queries). The ~1.4s/test measured locally before, and CI's 556s / 153 = ~3.6s/test, are the contended regime - CI's small runner is permanently contended, so the "Run Lite analysis-heavy tests" step is where this change should show its real value. This PR's own CI run is the decisive number. Full-suite after-timings on the idle window were polluted by external load returning mid-measurement (the same binary swung 286s-442s run to run); pass counts, not local wall-clock, are the reliable local signal.
Isolation proof
Sharing a database is exactly what lets one test poison the next, so isolation was proven, not assumed:
ResetData()in the test class constructor (xUnit constructs the class per test and runs a class's tests serially, so the shared DB is never touched concurrently).analysis_findings/analysis_muted, which the seeder's server-scoped clear never touched - so persisted findings and mute rules cannot leak between tests.CredentialProfileTestsfailure appeared in a separate run - the known load-flaky credential-store family (no DuckDB involvement); passes 13/13 in isolation.ResetDatabaseAsync(verified by reading all seven classes end to end).Not changed, and why
.github/workflows/build.yml- deliberately untouched per the fast/heavy split being cheap.MuteRulesSurviveResetTestsdeletes the DB file viaResetDatabaseAsync;ArchiveViewDedupTestswrites parquet and rebuilds views). Follow-up material if the fast bucket ever becomes the bottleneck.Generated with Claude Code