Fix pipeline test report timeout on large pipelines

Daniel Prauserequested to merge
fix-pipeline-test-report-timeout into master

What does this MR do and why?

Fixes a Rack::Timeout (60s) on the pipeline test-report path for large pipelines, observed on GitLab.com.

Ci::Pipeline#accessible_test_reports / #accessible_test_report_summary load every test-report build of the pipeline and its same-project descendants at once (.to_a) and evaluate the read_job_artifacts policy once per artifact. The per-artifact authorization is O(builds) DeclarativePolicy evaluations, so a very large pipeline (many parallel test jobs across child pipelines) exceeds the 60s request budget on a cache miss.

Note the regression is not query count — the pre-existing #test_reports path actually issues more queries, yet did not time out. Two things regressed: the O(builds) read_job_artifacts policy evaluations (CPU, locally measurable — the verification below isolates it), and the one-shot .to_a materialization of all builds plus heavy preloads (memory/GC — the production Rack::Timeout backtrace points into this phase).

Changes:

  • accessible_test_reports (full report) — stream builds with find_each and a per-build filter instead of materializing the whole set with .to_a. This path still loads and parses the artifacts.
  • test_report_readable_by? — memoize the per-artifact authorization by [user, project, accessibility, file_type] (the subject attributes Ci::JobArtifactPolicy reads), collapsing O(builds) policy evaluations to one per distinct combination. Used by the full-report path.
  • accessible_test_report_build_ids (summary path — where the Rack::Timeout backtrace lands) — never instantiate the builds: authorize one representative artifact per distinct (project, accessibility, file_type) and, when they are all readable (the common case), return the build ids directly.

Together these remove the two axes of the regression: the O(builds) read_job_artifacts policy evaluations (CPU), and — for the summary path — the one-shot .to_a materialization of every build that the backtrace points at.

References

  • Production Rack::Timeout on GitLab.com (Sentry) in Ci::Pipeline#accessible_test_report_builds.
  • Access control is unchanged — the same :read_job_artifacts policy is applied, only deduplicated and streamed.
  • Stable-branch backports for released versions (19.0 / 19.1 / 19.2) will follow as separate MRs targeting the canonical X-Y-stable-ee branches per the patch release process, once this MR is merged and deployed.

Screenshots or screen recordings

No UI changes.

How to set up and validate locally

The regression is the per-artifact authorization scaling, so the reproducible example counts read_job_artifacts policy evaluations while the number of test-report builds grows (1 → 5), through the MR-widget hot path (Ci::CompareTestReportsService, whose #get_report calls #accessible_test_reports on current code and #test_reports on the pre-security-fix baseline). The assertion is a straightforward N+1-style guard: the evaluation count must not grow with the build count (expect(scaled).to eq(control)).

Spec: spec/services/ci/compare_test_reports_service_authz_scaling_spec.rb (part of this MR)

All three states can be reproduced from this MR's branch without switching branches — only the three affected app files are checked out per state, the spec stays in the working tree:

# 1) with performance fix (this MR) → PASSES
bundle exec rspec spec/services/ci/compare_test_reports_service_authz_scaling_spec.rb

# 2) with security fix only (= current master) → FAILS
git checkout origin/master -- app/models/ci/pipeline.rb app/models/ci/build.rb app/services/ci/compare_test_reports_service.rb
bundle exec rspec spec/services/ci/compare_test_reports_service_authz_scaling_spec.rb

# 3) old baseline (pre security fix, parent of 6ea7ed71c1ede78c) → PASSES
git checkout 6ea7ed71~1 -- app/models/ci/pipeline.rb app/models/ci/build.rb app/services/ci/compare_test_reports_service.rb
bundle exec rspec spec/services/ci/compare_test_reports_service_authz_scaling_spec.rb

# restore
git checkout HEAD -- app/models/ci/pipeline.rb app/models/ci/build.rb app/services/ci/compare_test_reports_service.rb

Measured results (read_job_artifacts evaluations at 1 vs 5 test-report builds):

State 1 build 5 builds Result
old baseline (#test_reports, pre-security-fix) 0 0 PASS — no per-artifact authorization
with security fix (before this MR) 1 5 FAIL — O(builds) policy evaluations
with performance fix (this MR) 1 1 PASS — memoized, O(1)

Wall-clock of the authorization phase alone (time spent inside Ability.allowed?(:read_job_artifacts), local, N test-report builds):

N with security fix with performance fix
10 207 ms (10 evals) 189 ms (1 eval, cold-cache warmup)
100 396 ms (100 evals) 10 ms (1 eval)
300 1096 ms (300 evals) 12 ms (1 eval)

Linear vs flat, ~3.7 ms per evaluation locally at steady state. On GitLab.com the per-evaluation cost is higher (cold caches, complex memberships) and the affected pipelines have far more builds, which is what pushed the endpoint over the 60 s request budget. For hard wall-clock at production scale, a Rails-console #test_reports vs #accessible_test_reports timing on a large real pipeline plus the EXPLAIN (ANALYZE) targets in the Database review section below can be run on request.

Reproduce the wall-clock numbers yourself (throwaway benchmark, not part of the MR)

Save as spec/models/ci/perf_wallclock_spec.rb, run once on this branch and once with the master files checked out (same state-switching commands as above), then delete it:

# frozen_string_literal: true

require 'spec_helper'
require 'benchmark'

RSpec.describe 'test report wallclock bench', feature_category: :continuous_integration do
  let_it_be(:project) { create(:project, :repository) }
  let_it_be(:user) { create(:user, maintainer_of: project) }

  def pipeline_with(n)
    create(:ci_empty_pipeline, project: project).tap do |p|
      create_list(:ci_build, n, :test_reports, pipeline: p, project: project)
    end
  end

  it 'measures authz wall-clock' do
    [10, 100, 300].each do |n|
      p = pipeline_with(n)

      authz_time = 0.0
      authz_calls = 0
      allow(Ability).to receive(:allowed?).and_wrap_original do |original, *args|
        if args[1] == :read_job_artifacts
          authz_calls += 1
          t0 = Process.clock_gettime(Process::CLOCK_MONOTONIC)
          result = original.call(*args)
          authz_time += Process.clock_gettime(Process::CLOCK_MONOTONIC) - t0
          result
        else
          original.call(*args)
        end
      end

      total = Benchmark.realtime { p.accessible_test_reports(user) }

      puts "WALL n=#{n} | total=#{(total * 1000).round}ms | " \
        "authz=#{(authz_time * 1000).round}ms (#{authz_calls} evals)"
    end
  end
end
bundle exec rspec spec/models/ci/perf_wallclock_spec.rb

The authz= value is the interesting one: it grows linearly with n on master and stays flat (1 eval) with this MR. The total= is dominated by JUnit parsing locally, which is identical in both states.

Reproduce the wall-clock on a large real pipeline (Rails console)

Run against any sufficiently large pipeline (many test-report builds across child pipelines). The console runs the deployed code (= the security fix), so the old baseline (#test_reports) and the security fix can be timed directly, and the fix's logic is reconstructed inline (a faithful copy of accessible_test_report_build_ids; the shipped code cannot be exercised until this MR deploys). Everything is read-only.

require 'benchmark'

PIPELINE_ID = <PIPELINE_ID>

def fresh
  # Fresh instance each time to avoid strong_memoize / association caching.
  Ci::Pipeline.find(PIPELINE_ID)
end

user = fresh.user # or a specific user who can read the artifacts

# --- shape of the data ---
p0  = fresh
rel = p0.send(:latest_report_builds_in_self_and_project_descendants, Ci::JobArtifact.of_report_type(:test))
arts = Ci::JobArtifact.of_report_type(:test).where(job_id: rel.select(:id), partition_id: p0.partition_id)
puts "builds: #{rel.count} | artifacts: #{arts.count} | accessibility mix: #{arts.group(:accessibility).count}"

# --- 1) old baseline: test_report_summary uses .ids, no per-artifact authz ---
GC.start
puts "1) old baseline    : #{Benchmark.realtime { fresh.test_report_summary.total }.round(2)}s"

# --- 2) with security fix (deployed). WARNING: this is the timeout, minutes. ---
GC.start
puts "2) security fix    : #{Benchmark.realtime { fresh.accessible_test_report_summary(user).total }.round(2)}s"

# --- 3) with performance fix (summary path), reconstructed inline ---
GC.start
ids = nil
t_ids = Benchmark.realtime do
  builds = fresh.send(:builds_in_self_and_project_descendants)
    .with_existing_job_artifacts(Ci::JobArtifact.of_report_type(:test))
    .pluck(:id, :partition_id)
  build_ids = builds.map(&:first)
  ta = Ci::JobArtifact.of_report_type(:test).where(job_id: build_ids, partition_id: builds.map(&:last).uniq)

  # authorize one representative artifact per distinct (project, accessibility, file_type)
  readable = ta.group(:project_id, :accessibility, :file_type)
    .pluck(Arel.sql('MIN(id)'), :project_id, :accessibility, :file_type)
    .each_with_object({}) { |(rid, pid, a, f), m| m[[pid, a, f]] = Ability.allowed?(user, :read_job_artifacts, Ci::JobArtifact.find(rid)) }

  ids = readable.values.all? ? build_ids :
    ta.pluck(:job_id, :project_id, :accessibility, :file_type).group_by(&:first)
      .select { |_j, rs| rs.all? { |(_j, pid, a, f)| readable[[pid, a, f]] } }.keys
end
t_agg = Benchmark.realtime { fresh.send(:build_test_report_summary, ids).total }
puts "3) performance fix : #{(t_ids + t_agg).round(2)}s (#{ids.size} ids)"

Expected shape: the security fix scales with the build count and can exceed the 60 s budget on a large pipeline; the old baseline and the performance fix stay well under it, because the fix authorizes one representative artifact per distinct (project, accessibility, file_type) and never instantiates the builds.

Optional — split the security-fix time into materialization vs authorization:

require 'benchmark'
pipeline = Ci::Pipeline.find(<PIPELINE_ID>)
user     = pipeline.user

sql = 0.0; n = 0
ActiveSupport::Notifications.subscribe('sql.active_record') { |*, s, f, _, pl| next if pl[:name] == 'SCHEMA'; sql += f - s; n += 1 }

builds = nil
sql = 0.0; n = 0
t = Benchmark.realtime { builds = pipeline.send(:latest_test_report_builds_in_self_and_project_descendants).to_a }
puts "materialization: #{t.round(1)}s (sql #{sql.round(1)}s / #{n} q, #{builds.size} builds)"

sql = 0.0; n = 0
t = Benchmark.realtime { builds.select { |b| b.test_report_readable_by?(user) } }
puts "authorization  : #{t.round(1)}s (sql #{sql.round(1)}s, #{builds.size} evals)"

Database review

The summary path (accessible_test_report_build_ids) issues two read queries; the full-report path is unchanged.

1. Builds with test artifacts — pre-existing shape (indexed EXISTS on JUnit), now selecting only id + partition_id:

SELECT id, partition_id FROM p_ci_builds
WHERE type = 'Ci::Build' AND (retried = FALSE OR retried IS NULL)
  AND commit_id IN (<self + same-project descendant pipeline ids>)
  AND partition_id = <partition_id>
  AND EXISTS (
    SELECT 1 FROM p_ci_job_artifacts
    WHERE job_id = p_ci_builds.id AND partition_id = p_ci_builds.partition_id AND file_type = 4
  )

2. Distinct authorization inputs — new; one row per (project, accessibility, file_type):

SELECT MIN(id), project_id, accessibility, file_type FROM p_ci_job_artifacts
WHERE file_type = 4 AND job_id IN (<build ids from query 1>) AND partition_id = <partition_id>
GROUP BY project_id, accessibility, file_type

Query 2 is the one to validate at scale — the job_id IN (...) list is as large as the pipeline's test-report build count. EXPLAIN (ANALYZE, BUFFERS) on Database Lab against a large real pipeline:

EXPLAIN (ANALYZE, BUFFERS)
SELECT MIN(id), project_id, accessibility, file_type FROM p_ci_job_artifacts
WHERE file_type = 4 AND job_id IN (<build ids>) AND partition_id = <partition_id>
GROUP BY project_id, accessibility, file_type;

A representative artifact per group is then loaded with preload(job: :project) (bounded by the number of distinct groups, not builds), and the summary is aggregated from Ci::BuildReportResult.where(build_id: <readable ids>) (unchanged).

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Daniel Prause

Merge request reports