Commit abb5f765 authored by Yorick Peterse's avatar Yorick Peterse

Merge branch '27376-cache-default-branch-pipeline-on-project' into 'master'

Speed up several project lists

See merge request !9903
parents 8c5a3ffe 7d5b8993
...@@ -42,7 +42,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController ...@@ -42,7 +42,7 @@ class Dashboard::ProjectsController < Dashboard::ApplicationController
private private
def load_projects(base_scope) def load_projects(base_scope)
projects = base_scope.sorted_by_activity.includes(:namespace) projects = base_scope.sorted_by_activity.includes(:route, namespace: :route)
filter_projects(projects) filter_projects(projects)
end end
......
...@@ -2,7 +2,7 @@ class Explore::ProjectsController < Explore::ApplicationController ...@@ -2,7 +2,7 @@ class Explore::ProjectsController < Explore::ApplicationController
include FilterProjects include FilterProjects
def index def index
@projects = ProjectsFinder.new.execute(current_user) @projects = load_projects
@tags = @projects.tags_on(:tags) @tags = @projects.tags_on(:tags)
@projects = @projects.tagged_with(params[:tag]) if params[:tag].present? @projects = @projects.tagged_with(params[:tag]) if params[:tag].present?
@projects = @projects.where(visibility_level: params[:visibility_level]) if params[:visibility_level].present? @projects = @projects.where(visibility_level: params[:visibility_level]) if params[:visibility_level].present?
...@@ -21,7 +21,8 @@ class Explore::ProjectsController < Explore::ApplicationController ...@@ -21,7 +21,8 @@ class Explore::ProjectsController < Explore::ApplicationController
end end
def trending def trending
@projects = filter_projects(Project.trending) @projects = load_projects(Project.trending)
@projects = filter_projects(@projects)
@projects = @projects.sort(@sort = params[:sort]) @projects = @projects.sort(@sort = params[:sort])
@projects = @projects.page(params[:page]) @projects = @projects.page(params[:page])
...@@ -36,7 +37,7 @@ class Explore::ProjectsController < Explore::ApplicationController ...@@ -36,7 +37,7 @@ class Explore::ProjectsController < Explore::ApplicationController
end end
def starred def starred
@projects = ProjectsFinder.new.execute(current_user) @projects = load_projects
@projects = filter_projects(@projects) @projects = filter_projects(@projects)
@projects = @projects.reorder('star_count DESC') @projects = @projects.reorder('star_count DESC')
@projects = @projects.page(params[:page]) @projects = @projects.page(params[:page])
...@@ -50,4 +51,11 @@ class Explore::ProjectsController < Explore::ApplicationController ...@@ -50,4 +51,11 @@ class Explore::ProjectsController < Explore::ApplicationController
end end
end end
end end
protected
def load_projects(base_scope = nil)
base_scope ||= ProjectsFinder.new.execute(current_user)
base_scope.includes(:route, namespace: :route)
end
end end
...@@ -59,6 +59,24 @@ module CiStatusHelper ...@@ -59,6 +59,24 @@ module CiStatusHelper
custom_icon(icon_name) custom_icon(icon_name)
end end
def pipeline_status_cache_key(pipeline_status)
"pipeline-status/#{pipeline_status.sha}-#{pipeline_status.status}"
end
def render_project_pipeline_status(pipeline_status, tooltip_placement: 'auto left')
project = pipeline_status.project
path = pipelines_namespace_project_commit_path(
project.namespace,
project,
pipeline_status.sha)
render_status_with_link(
'commit',
pipeline_status.status,
path,
tooltip_placement: tooltip_placement)
end
def render_commit_status(commit, ref: nil, tooltip_placement: 'auto left') def render_commit_status(commit, ref: nil, tooltip_placement: 'auto left')
project = commit.project project = commit.project
path = pipelines_namespace_project_commit_path( path = pipelines_namespace_project_commit_path(
......
...@@ -159,6 +159,13 @@ module ProjectsHelper ...@@ -159,6 +159,13 @@ module ProjectsHelper
choose a GitLab CI Yaml template and commit your changes. #{link_to_autodeploy_doc}".html_safe choose a GitLab CI Yaml template and commit your changes. #{link_to_autodeploy_doc}".html_safe
end end
def project_list_cache_key(project)
key = [project.namespace.cache_key, project.cache_key, controller.controller_name, controller.action_name, current_application_settings.cache_key, 'v2.3']
key << pipeline_status_cache_key(project.pipeline_status) if project.pipeline_status.has_status?
key
end
private private
def repo_children_classes(field) def repo_children_classes(field)
......
...@@ -22,6 +22,7 @@ module Ci ...@@ -22,6 +22,7 @@ module Ci
validate :valid_commit_sha, unless: :importing? validate :valid_commit_sha, unless: :importing?
after_create :keep_around_commits, unless: :importing? after_create :keep_around_commits, unless: :importing?
after_create :refresh_build_status_cache
state_machine :status, initial: :created do state_machine :status, initial: :created do
event :enqueue do event :enqueue do
...@@ -328,6 +329,7 @@ module Ci ...@@ -328,6 +329,7 @@ module Ci
when 'manual' then block when 'manual' then block
end end
end end
refresh_build_status_cache
end end
def predefined_variables def predefined_variables
...@@ -369,6 +371,10 @@ module Ci ...@@ -369,6 +371,10 @@ module Ci
.fabricate! .fabricate!
end end
def refresh_build_status_cache
Ci::PipelineStatus.new(project, sha: sha, status: status).store_in_cache_if_needed
end
private private
def pipeline_data def pipeline_data
......
# This class is not backed by a table in the main database.
# It loads the latest Pipeline for the HEAD of a repository, and caches that
# in Redis.
module Ci
class PipelineStatus
attr_accessor :sha, :status, :project, :loaded
delegate :commit, to: :project
def self.load_for_project(project)
new(project).tap do |status|
status.load_status
end
end
def initialize(project, sha: nil, status: nil)
@project = project
@sha = sha
@status = status
end
def has_status?
loaded? && sha.present? && status.present?
end
def load_status
return if loaded?
if has_cache?
load_from_cache
else
load_from_commit
store_in_cache
end
self.loaded = true
end
def load_from_commit
return unless commit
self.sha = commit.sha
self.status = commit.status
end
# We only cache the status for the HEAD commit of a project
# This status is rendered in project lists
def store_in_cache_if_needed
return unless sha
return delete_from_cache unless commit
store_in_cache if commit.sha == self.sha
end
def load_from_cache
Gitlab::Redis.with do |redis|
self.sha, self.status = redis.hmget(cache_key, :sha, :status)
end
end
def store_in_cache
Gitlab::Redis.with do |redis|
redis.mapped_hmset(cache_key, { sha: sha, status: status })
end
end
def delete_from_cache
Gitlab::Redis.with do |redis|
redis.del(cache_key)
end
end
def has_cache?
Gitlab::Redis.with do |redis|
redis.exists(cache_key)
end
end
def loaded?
self.loaded
end
def cache_key
"projects/#{project.id}/build_status"
end
end
end
...@@ -1209,6 +1209,10 @@ class Project < ActiveRecord::Base ...@@ -1209,6 +1209,10 @@ class Project < ActiveRecord::Base
end end
end end
def pipeline_status
@pipeline_status ||= Ci::PipelineStatus.load_for_project(self)
end
def mark_import_as_failed(error_message) def mark_import_as_failed(error_message)
original_errors = errors.dup original_errors = errors.dup
sanitized_message = Gitlab::UrlSanitizer.sanitize(error_message) sanitized_message = Gitlab::UrlSanitizer.sanitize(error_message)
......
...@@ -6,17 +6,16 @@ ...@@ -6,17 +6,16 @@
- css_class = '' unless local_assigns[:css_class] - css_class = '' unless local_assigns[:css_class]
- show_last_commit_as_description = false unless local_assigns[:show_last_commit_as_description] == true && project.commit - show_last_commit_as_description = false unless local_assigns[:show_last_commit_as_description] == true && project.commit
- css_class += " no-description" if project.description.blank? && !show_last_commit_as_description - css_class += " no-description" if project.description.blank? && !show_last_commit_as_description
- cache_key = [project.namespace, project, controller.controller_name, controller.action_name, current_application_settings, 'v2.3'] - cache_key = project_list_cache_key(project)
- cache_key.push(project.commit&.sha, project.commit&.status)
%li.project-row{ class: css_class } %li.project-row{ class: css_class }
= cache(cache_key) do = cache(cache_key) do
.controls .controls
- if project.archived - if project.archived
%span.label.label-warning archived %span.label.label-warning archived
- if project.commit.try(:status) - if project.pipeline_status.has_status?
%span %span
= render_commit_status(project.commit) = render_project_pipeline_status(project.pipeline_status)
- if forks - if forks
%span %span
= icon('code-fork') = icon('code-fork')
......
---
title: Speed up project dashboard by caching pipeline status and eager loading routes
merge_request: 9903
author:
require 'spec_helper' require 'spec_helper'
RSpec.describe 'Dashboard Projects', feature: true do RSpec.describe 'Dashboard Projects', feature: true do
let(:user) { create(:user) }
let(:project) { create(:project, name: "awesome stuff") }
before do
project.team << [user, :developer]
login_as user
visit dashboard_projects_path
end
it 'shows the project the user in a member of in the list' do
visit dashboard_projects_path
expect(page).to have_content('awesome stuff')
end
describe "with a pipeline" do
let(:pipeline) { create(:ci_pipeline, :success, project: project, sha: project.commit.sha) }
before do before do
login_as(create :user) pipeline
end
it 'shows that the last pipeline passed' do
visit dashboard_projects_path visit dashboard_projects_path
expect(page).to have_xpath("//a[@href='#{pipelines_namespace_project_commit_path(project.namespace, project, project.commit)}']")
end
end end
it_behaves_like "an autodiscoverable RSS feed with current_user's private token" it_behaves_like "an autodiscoverable RSS feed with current_user's private token"
......
...@@ -16,4 +16,11 @@ describe CiStatusHelper do ...@@ -16,4 +16,11 @@ describe CiStatusHelper do
helper.ci_icon_for_status(failed_commit.status) helper.ci_icon_for_status(failed_commit.status)
end end
end end
describe "#pipeline_status_cache_key" do
it "builds a cache key for pipeline status" do
pipeline_status = Ci::PipelineStatus.new(build(:project), sha: "123abc", status: "success")
expect(helper.pipeline_status_cache_key(pipeline_status)).to eq("pipeline-status/123abc-success")
end
end
end end
...@@ -63,6 +63,46 @@ describe ProjectsHelper do ...@@ -63,6 +63,46 @@ describe ProjectsHelper do
end end
end end
describe "#project_list_cache_key" do
let(:project) { create(:project) }
it "includes the namespace" do
expect(helper.project_list_cache_key(project)).to include(project.namespace.cache_key)
end
it "includes the project" do
expect(helper.project_list_cache_key(project)).to include(project.cache_key)
end
it "includes the controller name" do
expect(helper.controller).to receive(:controller_name).and_return("testcontroller")
expect(helper.project_list_cache_key(project)).to include("testcontroller")
end
it "includes the controller action" do
expect(helper.controller).to receive(:action_name).and_return("testaction")
expect(helper.project_list_cache_key(project)).to include("testaction")
end
it "includes the application settings" do
settings = Gitlab::CurrentSettings.current_application_settings
expect(helper.project_list_cache_key(project)).to include(settings.cache_key)
end
it "includes a version" do
expect(helper.project_list_cache_key(project)).to include("v2.3")
end
it "includes the pipeline status when there is a status" do
create(:ci_pipeline, :success, project: project, sha: project.commit.sha)
expect(helper.project_list_cache_key(project)).to include("pipeline-status/#{project.commit.sha}-success")
end
end
describe 'link_to_member' do describe 'link_to_member' do
let(:group) { create(:group) } let(:group) { create(:group) }
let(:project) { create(:empty_project, group: group) } let(:project) { create(:empty_project, group: group) }
......
...@@ -1018,6 +1018,19 @@ describe Ci::Pipeline, models: true do ...@@ -1018,6 +1018,19 @@ describe Ci::Pipeline, models: true do
end end
end end
describe '#update_status' do
let(:pipeline) { create(:ci_pipeline, sha: '123456') }
it 'updates the cached status' do
fake_status = double
# after updating the status, the status is set to `skipped` for this pipeline's builds
expect(Ci::PipelineStatus).to receive(:new).with(pipeline.project, sha: '123456', status: 'skipped').and_return(fake_status)
expect(fake_status).to receive(:store_in_cache_if_needed)
pipeline.update_status
end
end
describe 'notifications when pipeline success or failed' do describe 'notifications when pipeline success or failed' do
let(:project) { create(:project, :repository) } let(:project) { create(:project, :repository) }
......
require 'spec_helper'
describe Ci::PipelineStatus do
let(:project) { create(:project) }
let(:pipeline_status) { described_class.new(project) }
describe '.load_for_project' do
it "loads the status" do
expect_any_instance_of(described_class).to receive(:load_status)
described_class.load_for_project(project)
end
end
describe '#has_status?' do
it "is false when the status wasn't loaded yet" do
expect(pipeline_status.has_status?).to be_falsy
end
it 'is true when all status information was loaded' do
fake_commit = double
allow(fake_commit).to receive(:status).and_return('failed')
allow(fake_commit).to receive(:sha).and_return('failed424d1b73bc0d3cb726eb7dc4ce17a4d48552f8c6')
allow(pipeline_status).to receive(:commit).and_return(fake_commit)
allow(pipeline_status).to receive(:has_cache?).and_return(false)
pipeline_status.load_status
expect(pipeline_status.has_status?).to be_truthy
end
end
describe '#load_status' do
it 'loads the status from the cache when there is one' do
expect(pipeline_status).to receive(:has_cache?).and_return(true)
expect(pipeline_status).to receive(:load_from_cache)
pipeline_status.load_status
end
it 'loads the status from the project commit when there is no cache' do
allow(pipeline_status).to receive(:has_cache?).and_return(false)
expect(pipeline_status).to receive(:load_from_commit)
pipeline_status.load_status
end
it 'stores the status in the cache when it loading it from the project' do
allow(pipeline_status).to receive(:has_cache?).and_return(false)
allow(pipeline_status).to receive(:load_from_commit)
expect(pipeline_status).to receive(:store_in_cache)
pipeline_status.load_status
end
it 'sets the state to loaded' do
pipeline_status.load_status
expect(pipeline_status).to be_loaded
end
it 'only loads the status once' do
expect(pipeline_status).to receive(:has_cache?).and_return(true).exactly(1)
expect(pipeline_status).to receive(:load_from_cache).exactly(1)
pipeline_status.load_status
pipeline_status.load_status
end
end
describe "#load_from_commit" do
let!(:pipeline) { create(:ci_pipeline, :success, project: project, sha: project.commit.sha) }
it 'reads the status from the pipeline for the commit' do
pipeline_status.load_from_commit
expect(pipeline_status.status).to eq('success')
expect(pipeline_status.sha).to eq(project.commit.sha)
end
it "doesn't fail for an empty project" do
status_for_empty_commit = described_class.new(create(:empty_project))
status_for_empty_commit.load_status
expect(status_for_empty_commit).to be_loaded
end
end
describe "#store_in_cache", :redis do
it "sets the object in redis" do
pipeline_status.sha = '123456'
pipeline_status.status = 'failed'
pipeline_status.store_in_cache
read_sha, read_status = Gitlab::Redis.with { |redis| redis.hmget("projects/#{project.id}/build_status", :sha, :status) }
expect(read_sha).to eq('123456')
expect(read_status).to eq('failed')
end
end
describe '#store_in_cache_if_needed', :redis do
it 'stores the state in the cache when the sha is the HEAD of the project' do
create(:ci_pipeline, :success, project: project, sha: project.commit.sha)
build_status = described_class.load_for_project(project)
build_status.store_in_cache_if_needed
sha, status = Gitlab::Redis.with { |redis| redis.hmget("projects/#{project.id}/build_status", :sha, :status) }
expect(sha).not_to be_nil
expect(status).not_to be_nil
end
it "doesn't store the status in redis when the sha is not the head of the project" do
other_status = described_class.new(project, sha: "123456", status: "failed")
other_status.store_in_cache_if_needed
sha, status = Gitlab::Redis.with { |redis| redis.hmget("projects/#{project.id}/build_status", :sha, :status) }
expect(sha).to be_nil
expect(status).to be_nil
end
it "deletes the cache if the repository doesn't have a head commit" do
empty_project = create(:empty_project)
Gitlab::Redis.with { |redis| redis.mapped_hmset("projects/#{empty_project.id}/build_status", { sha: "sha", status: "pending" }) }
other_status = described_class.new(empty_project, sha: "123456", status: "failed")
other_status.store_in_cache_if_needed
sha, status = Gitlab::Redis.with { |redis| redis.hmget("projects/#{empty_project.id}/build_status", :sha, :status) }
expect(sha).to be_nil
expect(status).to be_nil
end
end
describe "with a status in redis", :redis do
let(:status) { 'success' }
let(:sha) { '424d1b73bc0d3cb726eb7dc4ce17a4d48552f8c6' }
before do
Gitlab::Redis.with { |redis| redis.mapped_hmset("projects/#{project.id}/build_status", { sha: sha, status: status }) }
end
describe '#load_from_cache' do
it 'reads the status from redis' do
pipeline_status.load_from_cache
expect(pipeline_status.sha).to eq(sha)
expect(pipeline_status.status).to eq(status)
end
end
describe '#has_cache?' do
it 'knows the status is cached' do
expect(pipeline_status.has_cache?).to be_truthy
end
end
describe '#delete_from_cache' do
it 'deletes values from redis' do
pipeline_status.delete_from_cache
key_exists = Gitlab::Redis.with { |redis| redis.exists("projects/#{project.id}/build_status") }
expect(key_exists).to be_falsy
end
end
end
end
...@@ -1916,4 +1916,15 @@ describe Project, models: true do ...@@ -1916,4 +1916,15 @@ describe Project, models: true do
end end
end end
end end
describe '#pipeline_status' do
let(:project) { create(:project) }
it 'builds a pipeline status' do
expect(project.pipeline_status).to be_a(Ci::PipelineStatus)
end
it 'hase a loaded pipeline status' do
expect(project.pipeline_status).to be_loaded
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment