Bring back Rugged implementation of ListCommitsByOid
This brings back changes in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/20432. For users using Gitaly on top of NFS, accessing the Git data directly via Rugged may be faster than going through than Gitaly. This merge request introduces the feature flag `rugged_list_commits_by_oid` to activate the Rugged method. For one customer, we saw that ListCommitsByOid was the second highest used endpoint that may be causing increased load.
This commit is contained in:
parent
b0c0f81d54
commit
2fc4de6ab6
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Bring back Rugged implementation of ListCommitsByOid
|
||||||
|
merge_request: 27441
|
||||||
|
author:
|
||||||
|
type: performance
|
|
@ -80,6 +80,8 @@ most commonly-used RPCs can be enabled via feature flags:
|
||||||
* `rugged_get_tree_entries`
|
* `rugged_get_tree_entries`
|
||||||
* `rugged_tree_entry`
|
* `rugged_tree_entry`
|
||||||
* `rugged_commit_is_ancestor`
|
* `rugged_commit_is_ancestor`
|
||||||
|
* `rugged_commit_tree_entry`
|
||||||
|
* `rugged_list_commits_by_oid`
|
||||||
|
|
||||||
A convenience Rake task can be used to enable or disable these flags
|
A convenience Rake task can be used to enable or disable these flags
|
||||||
all together. To enable:
|
all together. To enable:
|
||||||
|
|
|
@ -21,6 +21,17 @@ module Gitlab
|
||||||
nil
|
nil
|
||||||
end
|
end
|
||||||
|
|
||||||
|
# This needs to return an array of Gitlab::Git:Commit objects
|
||||||
|
# instead of Rugged::Commit objects to ensure upstream models
|
||||||
|
# operate on a consistent interface. Unlike
|
||||||
|
# Gitlab::Git::Commit.find, Gitlab::Git::Commit.batch_by_oid
|
||||||
|
# doesn't attempt to decorate the result.
|
||||||
|
def rugged_batch_by_oid(repo, oids)
|
||||||
|
oids.map { |oid| rugged_find(repo, oid) }
|
||||||
|
.compact
|
||||||
|
.map { |commit| decorate(repo, commit) }
|
||||||
|
end
|
||||||
|
|
||||||
override :find_commit
|
override :find_commit
|
||||||
def find_commit(repo, commit_id)
|
def find_commit(repo, commit_id)
|
||||||
if Feature.enabled?(:rugged_find_commit)
|
if Feature.enabled?(:rugged_find_commit)
|
||||||
|
@ -29,6 +40,15 @@ module Gitlab
|
||||||
super
|
super
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
override :batch_by_oid
|
||||||
|
def batch_by_oid(repo, oids)
|
||||||
|
if Feature.enabled?(:rugged_list_commits_by_oid)
|
||||||
|
rugged_batch_by_oid(repo, oids)
|
||||||
|
else
|
||||||
|
super
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
extend ::Gitlab::Utils::Override
|
extend ::Gitlab::Utils::Override
|
||||||
|
|
|
@ -12,7 +12,7 @@ module Gitlab
|
||||||
module Repository
|
module Repository
|
||||||
extend ::Gitlab::Utils::Override
|
extend ::Gitlab::Utils::Override
|
||||||
|
|
||||||
FEATURE_FLAGS = %i(rugged_find_commit rugged_tree_entries rugged_tree_entry rugged_commit_is_ancestor rugged_commit_tree_entry).freeze
|
FEATURE_FLAGS = %i(rugged_find_commit rugged_tree_entries rugged_tree_entry rugged_commit_is_ancestor rugged_commit_tree_entry rugged_list_commits_by_oid).freeze
|
||||||
|
|
||||||
def alternate_object_directories
|
def alternate_object_directories
|
||||||
relative_object_directories.map { |d| File.join(path, d) }
|
relative_object_directories.map { |d| File.join(path, d) }
|
||||||
|
|
|
@ -380,7 +380,32 @@ describe Gitlab::Git::Commit, :seed_helper do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#batch_by_oid' do
|
shared_examples '.batch_by_oid' do
|
||||||
|
context 'with multiple OIDs' do
|
||||||
|
let(:oids) { [SeedRepo::Commit::ID, SeedRepo::FirstCommit::ID] }
|
||||||
|
|
||||||
|
it 'returns multiple commits' do
|
||||||
|
commits = described_class.batch_by_oid(repository, oids)
|
||||||
|
|
||||||
|
expect(commits.count).to eq(2)
|
||||||
|
expect(commits).to all( be_a(Gitlab::Git::Commit) )
|
||||||
|
expect(commits.first.sha).to eq(SeedRepo::Commit::ID)
|
||||||
|
expect(commits.second.sha).to eq(SeedRepo::FirstCommit::ID)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
context 'when oids is empty' do
|
||||||
|
it 'returns empty commits' do
|
||||||
|
commits = described_class.batch_by_oid(repository, [])
|
||||||
|
|
||||||
|
expect(commits.count).to eq(0)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '.batch_by_oid with Gitaly enabled' do
|
||||||
|
it_should_behave_like '.batch_by_oid'
|
||||||
|
|
||||||
context 'when oids is empty' do
|
context 'when oids is empty' do
|
||||||
it 'makes no Gitaly request' do
|
it 'makes no Gitaly request' do
|
||||||
expect(Gitlab::GitalyClient).not_to receive(:call)
|
expect(Gitlab::GitalyClient).not_to receive(:call)
|
||||||
|
@ -390,6 +415,16 @@ describe Gitlab::Git::Commit, :seed_helper do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '.batch_by_oid with Rugged enabled', :enable_rugged do
|
||||||
|
it_should_behave_like '.batch_by_oid'
|
||||||
|
|
||||||
|
it 'calls out to the Rugged implementation' do
|
||||||
|
allow_any_instance_of(Rugged).to receive(:rev_parse).with(SeedRepo::Commit::ID).and_call_original
|
||||||
|
|
||||||
|
described_class.batch_by_oid(repository, [SeedRepo::Commit::ID])
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
shared_examples 'extracting commit signature' do
|
shared_examples 'extracting commit signature' do
|
||||||
context 'when the commit is signed' do
|
context 'when the commit is signed' do
|
||||||
let(:commit_id) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' }
|
let(:commit_id) { '0b4bc9a49b562e85de7cc9e834518ea6828729b9' }
|
||||||
|
|
Loading…
Reference in New Issue