From 44044049f8acbc3a9465d6adb8a598a84b7d52bd Mon Sep 17 00:00:00 2001 From: damanpreetsingh Date: Tue, 13 Oct 2026 19:19:01 +0530 Subject: [PATCH] db: aggregate run summaries with GROUP BY instead of row-by-row iteration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ListRunSummariesInRange previously returned one row per (run, test) pair and iterated in Go to count tests by state. For the dashboard's 30-day call this meant ~100k rows and parsing test result JSON for each. Switch to GROUP BY runs.id with count(*) FILTER (WHERE result->>'state' = '...') to aggregate test counts in SQL. This: - Reduces rows from O(tests) to O(runs) (~100k → ~3k for 30 days) - Pushes state classification to Postgres, eliminating JSON parsing in Go - Simplifies the iteration logic (one pass over runs, no seenRuns map) PackageSummary now stores PassedCount/FailedCount/SkippedCount integers instead of map[string][]uuid.UUID per state. The /run_summary page shows aggregate counts and links to runs for individual test details. Follow-up to the approach suggested in PR #16 review: use GROUP BY to push more computation to Postgres rather than extracting fields row-by-row. --- db/pg.go | 169 ++++++++++++++------------------ db/pg_test.go | 36 +++---- http/templates/run_summary.html | 108 +++++++------------- http/ui_test.go | 9 +- types.go | 33 +++---- 5 files changed, 145 insertions(+), 210 deletions(-) diff --git a/db/pg.go b/db/pg.go index e18d43c..2d1b64c 100644 --- a/db/pg.go +++ b/db/pg.go @@ -576,13 +576,11 @@ const runSummaryLookback = 24 * time.Hour // during [begin, end) into windows of `window` and summarises them per // package. See tester.RunSummary for the bucket semantics. // -// The query is a LEFT JOIN from runs to tests so a run without tests (an -// errored or killed run whose test binary never reported anything) still -// shows up in ErrorRunIDs. Only the columns the summary needs are selected -// and the test name and state are extracted from the result jsonb in SQL, -// rather than transferring and unmarshalling the whole document; for the -// dashboard's 30-day call that is the difference between ~40 MB and a few -// MB over the wire. +// The query uses GROUP BY with count(*) FILTER to aggregate test counts in +// SQL, pushing computation to Postgres rather than iterating row-by-row in +// Go. This reduces the result set from O(tests) to O(runs) and eliminates +// unmarshalling overhead. For the dashboard's 30-day call this means one +// row per run (~3k rows) instead of one row per test (~100k rows). func (p *PG) ListRunSummariesInRange(ctx context.Context, begin, end time.Time, window time.Duration) ([]*tester.RunSummary, error) { begin = begin.UTC() end = end.UTC() @@ -613,10 +611,7 @@ func (p *PG) ListRunSummariesInRange(ctx context.Context, begin, end time.Time, ps, ok := summary.PackageSummary[pkg] if !ok { ps = &tester.PackageSummary{ - Package: pkg, - PassedTests: make(map[string][]uuid.UUID), - FailedTests: make(map[string][]uuid.UUID), - SkippedTests: make(map[string][]uuid.UUID), + Package: pkg, } summary.PackageSummary[pkg] = ps } @@ -624,54 +619,51 @@ func (p *PG) ListRunSummariesInRange(ctx context.Context, begin, end time.Time, } err := p.tx(ctx, func(tx pgx.Tx) error { - q := psq.Select( - "runs.package", - "runs.id", - "runs.started_at", - "runs.finished_at", - "(runs.error IS NOT NULL) AS errored", - "tests.id", - "tests.result->>'name'", - "tests.result->>'state'", - ). - From("runs"). - // The tests of an errored run are not counted, so do not fetch them. - LeftJoin("tests ON tests.run_id = runs.id AND runs.error IS NULL"). - Where("runs.started_at IS NOT NULL"). - Where("runs.started_at >= ?", begin.Add(-runSummaryLookback)). - Where("runs.started_at < ?", end). - Where("(runs.finished_at IS NULL OR runs.finished_at >= ?)", begin). - OrderBy("runs.started_at ASC") - - query, args, err := q.ToSql() - if err != nil { - return err - } - - rows, err := tx.Query(ctx, query, args...) + // Use GROUP BY to aggregate test counts per run in SQL. This returns + // one row per run instead of one row per (run, test) pair, dramatically + // reducing data transfer and Go-side iteration. + // + // The count(*) FILTER clauses push the state classification to Postgres, + // eliminating the need to transfer and parse test result JSON. + query := ` + SELECT + runs.id, + runs.package, + runs.started_at, + runs.finished_at, + (runs.error IS NOT NULL) AS errored, + count(tests.id) FILTER (WHERE tests.result->>'state' = 'passed') AS passed_count, + count(tests.id) FILTER (WHERE tests.result->>'state' = 'failed') AS failed_count, + count(tests.id) FILTER (WHERE tests.result->>'state' = 'skipped') AS skipped_count + FROM runs + LEFT JOIN tests ON tests.run_id = runs.id AND runs.error IS NULL + WHERE runs.started_at IS NOT NULL + AND runs.started_at >= $1 + AND runs.started_at < $2 + AND (runs.finished_at IS NULL OR runs.finished_at >= $3) + GROUP BY runs.id + ORDER BY runs.started_at ASC + ` + + rows, err := tx.Query(ctx, query, begin.Add(-runSummaryLookback), end, begin) if err != nil { return err } defer rows.Close() - // Rows are one per (run, test) pair, or a single row with NULL test - // columns for a run without tests. Run ids are recorded once per run - // on its first row; tests are recorded on every row that has one. - seenRuns := make(map[uuid.UUID]struct{}) - + // Each row is one run with aggregated test counts. for rows.Next() { var ( - pkg string - runID uuid.UUID - startedAt time.Time - finishedAt sql.NullTime - errored bool - testID uuid.NullUUID - testName sql.NullString - testState sql.NullString - startBucket int + runID uuid.UUID + pkg string + startedAt time.Time + finishedAt sql.NullTime + errored bool + passedCount int + failedCount int + skippedCount int ) - err := rows.Scan(&pkg, &runID, &startedAt, &finishedAt, &errored, &testID, &testName, &testState) + err := rows.Scan(&runID, &pkg, &startedAt, &finishedAt, &errored, &passedCount, &failedCount, &skippedCount) if err != nil { return err } @@ -679,54 +671,43 @@ func (p *PG) ListRunSummariesInRange(ctx context.Context, begin, end time.Time, // The bucket the run started in may be before `begin` (the run // was already executing when the range starts), in which case it // is negative and its tests were counted by a previous range. - startBucket = bucketIndex(startedAt.UTC().Sub(begin), window) - - if _, seen := seenRuns[runID]; !seen { - seenRuns[runID] = struct{}{} + startBucket := bucketIndex(startedAt.UTC().Sub(begin), window) + + // A run is attributed to every bucket it overlapped with, as + // the half-open interval [started_at, finished_at). A run that + // is still running extends to now. + until := now + if finishedAt.Valid { + until = finishedAt.Time.UTC() + } + lastBucket := startBucket + if until.After(startedAt) { + lastBucket = bucketIndex(until.Sub(begin)-time.Nanosecond, window) + } + if lastBucket < startBucket { + lastBucket = startBucket + } - // A run is attributed to every bucket it overlapped with, as - // the half-open interval [started_at, finished_at). A run that - // is still running extends to now. - until := now - if finishedAt.Valid { - until = finishedAt.Time.UTC() - } - lastBucket := startBucket - if until.After(startedAt) { - lastBucket = bucketIndex(until.Sub(begin)-time.Nanosecond, window) - } - if lastBucket < startBucket { - lastBucket = startBucket + for b := max(startBucket, 0); b <= min(lastBucket, buckets-1); b++ { + ps := packageSummary(b, pkg) + switch { + case errored: + ps.ErrorRunIDs = append(ps.ErrorRunIDs, runID) + case !finishedAt.Valid: + ps.RunIDs = append(ps.RunIDs, runID) + ps.RunningRunIDs = append(ps.RunningRunIDs, runID) + default: + ps.RunIDs = append(ps.RunIDs, runID) } - for b := max(startBucket, 0); b <= min(lastBucket, buckets-1); b++ { - ps := packageSummary(b, pkg) - switch { - case errored: - ps.ErrorRunIDs = append(ps.ErrorRunIDs, runID) - case !finishedAt.Valid: - ps.RunIDs = append(ps.RunIDs, runID) - ps.RunningRunIDs = append(ps.RunningRunIDs, runID) - default: - ps.RunIDs = append(ps.RunIDs, runID) - } + // Tests are only counted in the bucket their run started in, + // so a run spanning several buckets is not counted several times. + if b == startBucket && startBucket >= 0 && startBucket < buckets { + ps.PassedCount += passedCount + ps.FailedCount += failedCount + ps.SkippedCount += skippedCount } } - - // Tests are only counted in the bucket their run started in, so - // a run spanning several buckets is not counted several times. - if !testID.Valid || startBucket < 0 || startBucket >= buckets { - continue - } - ps := packageSummary(startBucket, pkg) - switch tester.TBState(testState.String) { - case tester.TBStatePassed: - ps.PassedTests[testName.String] = append(ps.PassedTests[testName.String], testID.UUID) - case tester.TBStateFailed: - ps.FailedTests[testName.String] = append(ps.FailedTests[testName.String], testID.UUID) - case tester.TBStateSkipped: - ps.SkippedTests[testName.String] = append(ps.SkippedTests[testName.String], testID.UUID) - } } return rows.Err() }) diff --git a/db/pg_test.go b/db/pg_test.go index 035f538..b4c88e9 100644 --- a/db/pg_test.go +++ b/db/pg_test.go @@ -887,9 +887,9 @@ func TestPG_ListRunSummariesInRange(t *testing.T) { Package: "pkg-1", RunIDs: []uuid.UUID{pkg1run1.ID, pkg1run2.ID}, ErrorRunIDs: nil, - PassedTests: map[string][]uuid.UUID{"test-pass": {pkg1run1.Tests[0].ID, pkg1run2.Tests[0].ID}}, - FailedTests: map[string][]uuid.UUID{"test-fail": {pkg1run1.Tests[1].ID, pkg1run2.Tests[1].ID}}, - SkippedTests: map[string][]uuid.UUID{"test-skip": {pkg1run1.Tests[2].ID, pkg1run2.Tests[2].ID}}, + PassedCount: 2, // 2 runs × 1 passed test each + FailedCount: 2, // 2 runs × 1 failed test each + SkippedCount: 2, // 2 runs × 1 skipped test each }, }, }, summaries[0]) @@ -900,9 +900,9 @@ func TestPG_ListRunSummariesInRange(t *testing.T) { Package: "pkg-2", RunIDs: []uuid.UUID{pkg2run1.ID}, ErrorRunIDs: []uuid.UUID{pkg2run2.ID}, - PassedTests: map[string][]uuid.UUID{"test-pass": {pkg2run1.Tests[0].ID}}, - FailedTests: map[string][]uuid.UUID{}, - SkippedTests: map[string][]uuid.UUID{}, + PassedCount: 1, + FailedCount: 0, + SkippedCount: 0, }, summaries[2].PackageSummary["pkg-2"]) }) }) @@ -929,9 +929,9 @@ func TestPG_ListRunSummariesInRange(t *testing.T) { assert.Equal(t, &tester.PackageSummary{ Package: "pkg", ErrorRunIDs: []uuid.UUID{run.ID}, - PassedTests: map[string][]uuid.UUID{}, - FailedTests: map[string][]uuid.UUID{}, - SkippedTests: map[string][]uuid.UUID{}, + PassedCount: 0, + FailedCount: 0, + SkippedCount: 0, }, summaries[0].PackageSummary["pkg"]) assert.Empty(t, summaries[1].PackageSummary) }) @@ -999,9 +999,9 @@ func TestPG_ListRunSummariesInRange(t *testing.T) { Package: "pkg", RunIDs: []uuid.UUID{run.ID}, RunningRunIDs: []uuid.UUID{run.ID}, - PassedTests: map[string][]uuid.UUID{"test-pass": {test.ID}}, - FailedTests: map[string][]uuid.UUID{}, - SkippedTests: map[string][]uuid.UUID{}, + PassedCount: 1, + FailedCount: 0, + SkippedCount: 0, }, summaries[0].PackageSummary["pkg"], "start bucket has the run and its tests") for _, i := range []int{1, 2} { @@ -1009,9 +1009,9 @@ func TestPG_ListRunSummariesInRange(t *testing.T) { Package: "pkg", RunIDs: []uuid.UUID{run.ID}, RunningRunIDs: []uuid.UUID{run.ID}, - PassedTests: map[string][]uuid.UUID{}, - FailedTests: map[string][]uuid.UUID{}, - SkippedTests: map[string][]uuid.UUID{}, + PassedCount: 0, + FailedCount: 0, + SkippedCount: 0, }, summaries[i].PackageSummary["pkg"], "bucket %d has the run but not its tests", i) } assert.Empty(t, summaries[3].PackageSummary, "bucket after now is empty") @@ -1107,9 +1107,9 @@ func TestPG_ListRunSummariesInRange(t *testing.T) { assert.Equal(t, &tester.PackageSummary{ Package: "pkg", RunIDs: []uuid.UUID{overlapping.ID}, - PassedTests: map[string][]uuid.UUID{}, - FailedTests: map[string][]uuid.UUID{}, - SkippedTests: map[string][]uuid.UUID{}, + PassedCount: 0, // Tests from run that started before range are not counted + FailedCount: 0, + SkippedCount: 0, }, summaries[0].PackageSummary["pkg"]) assert.Empty(t, summaries[1].PackageSummary) }) diff --git a/http/templates/run_summary.html b/http/templates/run_summary.html index af0013d..c74f62a 100644 --- a/http/templates/run_summary.html +++ b/http/templates/run_summary.html @@ -8,11 +8,22 @@

Run Results {{.RunSummary.Time | format

{{$pkg}} {{len $summary.RunIDs}} Runs {{if $summary.RunningRunIDs}}({{len $summary.RunningRunIDs}} running){{end}} {{if $summary.ErrorRunIDs}}({{len $summary.ErrorRunIDs}} erred){{end}}

- {{if or $summary.RunningRunIDs $summary.ErrorRunIDs}}

Runs

+ {{if $summary.RunIDs}} +
+ Completed +
+ {{range $summary.RunIDs}} + + {{.}} completed + + {{end}} +
+
+ {{end}} {{if $summary.RunningRunIDs}} -
+
Running
{{range $summary.RunningRunIDs}} @@ -24,7 +35,7 @@

Runs

{{end}} {{if $summary.ErrorRunIDs}} -
+
Erred
{{range $summary.ErrorRunIDs}} @@ -36,74 +47,31 @@

Runs

{{end}}
- {{end}} - -

Tests

-
-
- Passed -
    - {{range $name, $testIDs := $summary.PassedTests}} -
  • -
    - {{$name}} - {{len $testIDs}} -
    -
    -
    - {{range $testIDs}} -
    - {{.}} -
    - {{end}} -
    -
    -
    • - {{end}} -
-
- -
- Failed / Skipped -
    - {{range $name, $testIDs := $summary.FailedTests}} -
  • -
    - {{$name}} - {{len $testIDs}} -
    - -
    -
    - {{range $testIDs}} -
    - {{.}} -
    - {{end}} -
    -
    -
    • - {{end}} - {{range $name, $testIDs := $summary.SkippedTests}} -
  • -
    - {{$name}} - {{len $testIDs}} -
    - -
    -
    - {{range $testIDs}} -
    - {{.}} -
    - {{end}} -
    -
    -
    • - {{end}} -
+

Test Summary

+
+
+
+
+ {{$summary.NumPassedTests}} + Passed +
+
+ {{$summary.NumFailedTests}} + Failed +
+
+ {{$summary.NumSkippedTests}} + Skipped +
+
+ {{$summary.NumTotalTests}} + Total +
+
+

+ Click on a run above to view individual test results. +

diff --git a/http/ui_test.go b/http/ui_test.go index 19ff953..5af625a 100644 --- a/http/ui_test.go +++ b/http/ui_test.go @@ -21,17 +21,14 @@ import ( func uiTestSummaries(begin time.Time, window time.Duration) []*tester.RunSummary { newPkg := func() *tester.PackageSummary { return &tester.PackageSummary{ - Package: "pkg", - PassedTests: map[string][]uuid.UUID{}, - FailedTests: map[string][]uuid.UUID{}, - SkippedTests: map[string][]uuid.UUID{}, + Package: "pkg", } } results := newPkg() results.RunIDs = []uuid.UUID{uuid.New()} - results.PassedTests["TestA"] = []uuid.UUID{uuid.New()} - results.FailedTests["TestB"] = []uuid.UUID{uuid.New()} + results.PassedCount = 1 + results.FailedCount = 1 running := newPkg() running.RunIDs = []uuid.UUID{uuid.New()} diff --git a/types.go b/types.go index 2920165..73bc8e9 100644 --- a/types.go +++ b/types.go @@ -195,17 +195,18 @@ func (s *RunSummary) NumTotalTests() int { // RunIDs holds every run of the package that did not error and was executing // at some point during the bucket; RunningRunIDs is the subset of those that // has not finished yet. ErrorRunIDs holds the runs that errored; they are -// disjoint from RunIDs and their tests are not counted. The test maps are -// keyed by test name and only contain tests of runs that started in this -// bucket. +// disjoint from RunIDs and their tests are not counted. +// +// Test counts are aggregated in SQL using GROUP BY with FILTER clauses, +// pushing computation to Postgres rather than iterating row-by-row in Go. type PackageSummary struct { Package string RunIDs []uuid.UUID RunningRunIDs []uuid.UUID ErrorRunIDs []uuid.UUID - PassedTests map[string][]uuid.UUID - FailedTests map[string][]uuid.UUID - SkippedTests map[string][]uuid.UUID + PassedCount int + FailedCount int + SkippedCount int } func (s *PackageSummary) NumRuns() int { @@ -221,11 +222,7 @@ func (s *PackageSummary) NumRunningRuns() int { } func (s *PackageSummary) NumPassedTests() int { - var total int - for _, tests := range s.PassedTests { - total += len(tests) - } - return total + return s.PassedCount } func (s *PackageSummary) PercentPassedTests() float64 { @@ -233,11 +230,7 @@ func (s *PackageSummary) PercentPassedTests() float64 { } func (s *PackageSummary) NumFailedTests() int { - var total int - for _, tests := range s.FailedTests { - total += len(tests) - } - return total + return s.FailedCount } func (s *PackageSummary) PercentFailedTests() float64 { @@ -245,11 +238,7 @@ func (s *PackageSummary) PercentFailedTests() float64 { } func (s *PackageSummary) NumSkippedTests() int { - var total int - for _, tests := range s.SkippedTests { - total += len(tests) - } - return total + return s.SkippedCount } func (s *PackageSummary) PercentSkippedTests() float64 { @@ -257,5 +246,5 @@ func (s *PackageSummary) PercentSkippedTests() float64 { } func (s *PackageSummary) NumTotalTests() int { - return s.NumPassedTests() + s.NumFailedTests() + s.NumSkippedTests() + return s.PassedCount + s.FailedCount + s.SkippedCount }