Commit b12be5c7 authored by Thong Kuah's avatar Thong Kuah

Remove need to call gitaly in ProjectPipelineStatus

Calling commit (gitaly) is un-nessary just to obtain the sha, for the
cache key. In fact, the cache key does not need to have the sha as the
sha is part of the cache value. The new cache key should be unique
enough for each project.

This saves us 20 gitaly calls on the dashboard page (assuming we have
one full page of projects).
parent fbc32fad
---
title: Remove need to call commit (gitaly call) in ProjectPipelineStatus
merge_request: 33712
author:
type: performance
...@@ -49,7 +49,6 @@ module Gitlab ...@@ -49,7 +49,6 @@ module Gitlab
def load_status def load_status
return if loaded? return if loaded?
return unless commit
if has_cache? if has_cache?
load_from_cache load_from_cache
...@@ -66,6 +65,8 @@ module Gitlab ...@@ -66,6 +65,8 @@ module Gitlab
end end
def load_from_project def load_from_project
return unless commit
self.sha, self.status, self.ref = commit.sha, commit.status, project.default_branch self.sha, self.status, self.ref = commit.sha, commit.status, project.default_branch
end end
...@@ -114,7 +115,7 @@ module Gitlab ...@@ -114,7 +115,7 @@ module Gitlab
end end
def cache_key def cache_key
"#{Gitlab::Redis::Cache::CACHE_NAMESPACE}:project:#{project.id}:pipeline_status:#{commit&.sha}" "#{Gitlab::Redis::Cache::CACHE_NAMESPACE}:project:#{project.id}:pipeline_status"
end end
def commit def commit
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::Cache::Ci::ProjectPipelineStatus, :clean_gitlab_redis_cache do RSpec.describe Gitlab::Cache::Ci::ProjectPipelineStatus, :clean_gitlab_redis_cache do
let!(:project) { create(:project, :repository) } let_it_be(:project) { create(:project, :repository) }
let(:pipeline_status) { described_class.new(project) } let(:pipeline_status) { described_class.new(project) }
let(:cache_key) { pipeline_status.cache_key } let(:cache_key) { pipeline_status.cache_key }
...@@ -77,6 +77,30 @@ RSpec.describe Gitlab::Cache::Ci::ProjectPipelineStatus, :clean_gitlab_redis_cac ...@@ -77,6 +77,30 @@ RSpec.describe Gitlab::Cache::Ci::ProjectPipelineStatus, :clean_gitlab_redis_cac
end end
describe '#load_status' do describe '#load_status' do
describe 'gitaly call counts', :request_store do
context 'not cached' do
before do
expect(pipeline_status).not_to be_has_cache
end
it 'makes a Gitaly call' do
expect { pipeline_status.load_status }.to change { Gitlab::GitalyClient.get_request_count }.by(1)
end
end
context 'cached' do
before do
described_class.load_in_batch_for_projects([project])
expect(pipeline_status).to be_has_cache
end
it 'makes no Gitaly calls' do
expect { pipeline_status.load_status }.to change { Gitlab::GitalyClient.get_request_count }.by(0)
end
end
end
it 'loads the status from the cache when there is one' 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(:has_cache?).and_return(true)
expect(pipeline_status).to receive(:load_from_cache) expect(pipeline_status).to receive(:load_from_cache)
......
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