From e342350afae7e4be8bb231c27b394b3f2a2517db Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Wed, 16 Sep 2026 09:10:45 -0400 Subject: [PATCH] Preload commit statuses to avoid N+1 --- CHANGELOG.md | 6 ++++++ app/controllers/shipit/api/deploys_controller.rb | 2 +- app/controllers/shipit/api/tasks_controller.rb | 2 +- app/models/shipit/deploy.rb | 3 ++- test/models/deploys_test.rb | 13 +++++++++++++ 5 files changed, 23 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db1ed26b2..ba7bc6d98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,10 @@ # Unreleased +* (performance) Preload commit statuses and check runs when serializing deploys. `DeploySerializer` + embeds every commit of a deploy and `CommitSerializer` reads each commit's status, so rendering a + deploy issued two queries per commit. On a deploy spanning thousands of commits that is thousands of + round trips held open for the life of the request: in production `GET /api/stacks/:stack/tasks` + responses reached 119s and pushed web processes into their memory limit. The commits relation now + preloads, and both API index actions preload the task's user and commits. * (bugfix) Fix task output flickering and freezing on long logs. Clusterize sized its virtual-scroll spacers with an inline style attribute produced by `outerHTML`, which a `style-src` Content Security Policy without `'unsafe-inline'` refuses to apply. The spacers collapsed to zero height, diff --git a/app/controllers/shipit/api/deploys_controller.rb b/app/controllers/shipit/api/deploys_controller.rb index a20360802..04c78651f 100644 --- a/app/controllers/shipit/api/deploys_controller.rb +++ b/app/controllers/shipit/api/deploys_controller.rb @@ -6,7 +6,7 @@ class DeploysController < BaseController require_permission :deploy, :stack def index - render_resources(stack.deploys_and_rollbacks) + render_resources(stack.deploys_and_rollbacks.preload(:user, :until_commit, :since_commit)) end params do diff --git a/app/controllers/shipit/api/tasks_controller.rb b/app/controllers/shipit/api/tasks_controller.rb index 6d7ae4004..c0b587483 100644 --- a/app/controllers/shipit/api/tasks_controller.rb +++ b/app/controllers/shipit/api/tasks_controller.rb @@ -7,7 +7,7 @@ class TasksController < BaseController require_permission :deploy, :stack, only: %i[trigger abort] def index - render_resources(stack.tasks) + render_resources(stack.tasks.preload(:user, :until_commit, :since_commit)) end def show diff --git a/app/models/shipit/deploy.rb b/app/models/shipit/deploy.rb index 71dded7c1..8f07672fb 100644 --- a/app/models/shipit/deploy.rb +++ b/app/models/shipit/deploy.rb @@ -163,7 +163,8 @@ def currently_deployed? def commits return Commit.none unless stack - @commits ||= stack.commits.reachable.newer_than(since_commit_id).until(until_commit_id).order(id: :desc) + @commits ||= stack.commits.reachable.newer_than(since_commit_id).until(until_commit_id) + .order(id: :desc).preload(:statuses, :check_runs) end def commits_since diff --git a/test/models/deploys_test.rb b/test/models/deploys_test.rb index 7193d90ee..5d2df08b4 100644 --- a/test/models/deploys_test.rb +++ b/test/models/deploys_test.rb @@ -881,6 +881,19 @@ def generate_commits(amount:, stack_id:, user_id:, validate:) assert_predicate @deploy, :error? end + test "#commits preloads what serializers read, so rendering a deploy costs no query per commit" do + deploy = shipit_deploys(:shipit_pending) + commits = deploy.commits.to_a + assert_operator commits.size, :>, 1 + + assert_no_queries do + commits.each do |commit| + commit.statuses.to_a + commit.check_runs.to_a + end + end + end + test "#chunk_output fetches from Redis if logs not rolled up" do assert_equal Shipit.redis.get(@deploy.send(:output_key)), @deploy.chunk_output refute @deploy.rolled_up