Extend the shared DuckDB fixture to 20 fast-bucket test classes - #1698
Merged
Conversation
…ed helper Profiling the analysis-heavy filter after the shared-fixture change showed the remaining cost was not schema DDL but seeding I/O: every seeded row opened a fresh DuckDB connection and auto-committed a single INSERT, measured at ~90ms/row, and AnomalyDetectorTests' 100-200-row seeds alone were the filter's entire critical path (154.5s serial of its ~154s wall). - AnomalyDetectorTests / BaselineProviderTests: a per-test-instance seed connection reused for every row; the seven SeedBaseline* loops in AnomalyDetectorTests each run inside one BEGIN/COMMIT. - TestDataSeeder: one lazily-opened connection per seeder instance, and every seed helper wraps its inserts in a SeedBatch (BEGIN on create, COMMIT on dispose) so a helper's rows share one WAL flush. ClearTestDataAsync deliberately stays un-batched: its per-table try/catch DELETEs would abort an explicit transaction on the first missing table and silently skip the rest of the clear. - TestDataSeeder is now IDisposable (it owns the connection); all 48 construction sites take `using var seeder`. Measured (local, idle box): analysis-heavy filter 153 tests 164s -> 19s; AnomalyDetectorTests 154.5s -> 5.9s serial; full suite 1494 tests 233s -> 78s. Full suite run twice consecutively with identical 1494/1494 results both times. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The heavy seven got SharedDuckDbFixture in #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>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
erikdarlingdata
enabled auto-merge
July 26, 2026 18:47
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
Round three of the Lite.Tests speed campaign (#1693 shared fixtures for the heavy seven, #1694 batched seeding): a full-suite trx profile showed 400s+ of serial weight remaining in fast-bucket classes with the same per-test disease - a fresh DuckDB and full schema build inside every test, and in several classes per-row single-connection seeding. This PR extends
SharedDuckDbFixtureto 20 more classes.Stacked on #1694; once it merges, this diff collapses to the fast-bucket commits.
Converted
Fixture + data-only reset per test, plus a 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, called out because each was a place this refactor could have gone wrong:
CREATE TABLE analysis_findingswould collide on the fixture's current schema). It now runs on its own private database file; its 15 siblings share the fixture.[Collection("server-time-helper")]trio keeps its collection attribute (class fixtures compose with named collections).Deliberately NOT converted
Each of these needs a private database by design, and converting them is exactly how a shared fixture poisons tests:
ArchiveViewDedupTests- writes parquet and rebuilds archive views.MuteRulesSurviveResetTests- drivesArchiveAllAndResetAsync, which deletes the database file (ResetDatabaseAsync).DuckDbSchemaTests- tests schema creation and init idempotency itself.AlertHistorySourceTests/DismissedArchiveSidecarTests/DismissReliabilityTests- theirTestAlertDataHelperwrites parquet into the database's archive directory and rebuilds views.LiteAlertForwardingTests- a single DB-touching test; not worth a fixture.Measured (full-suite trx, same box, contended-parallel conditions)
Local full-suite wall time is now parallelism-bound (~87-100s on a many-core box; the removed work was parallel slack). CI's 2-core runner is serial-bound, so the ~165s of eliminated work should translate there nearly 1:1 - the fast-tests step is the one to watch on this PR's CI run.
Isolation proof
Generated with Claude Code