Add back Rugged support for retrieving a commit tree entry
This brings back some of the changes in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/20176/diffs. We discovered another N+1 that hits Gitaly `TreeEntry` via the `RelativeLinkFilter`: https://gitlab.com/gitlab-org/gitlab-ce/issues/58657. When a blob is loaded with many relative links, `TreeEntry` is called for each link to scan the URI type. There are multiple paths that hit Gitaly `TreeEntry`, and https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/25706 did not cover all cases. This commit covers another common use case. 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_commit_tree_entry` to activate the Rugged method.
This commit is contained in:
parent
5566809c97
commit
4ee08fd1f7
5 changed files with 50 additions and 2 deletions
5
changelogs/unreleased/sh-rugged-commit-tree-entry.yml
Normal file
5
changelogs/unreleased/sh-rugged-commit-tree-entry.yml
Normal file
|
@ -0,0 +1,5 @@
|
||||||
|
---
|
||||||
|
title: Bring back Rugged implementation of commit_tree_entry
|
||||||
|
merge_request: 25896
|
||||||
|
author:
|
||||||
|
type: other
|
|
@ -314,11 +314,16 @@ module Gitlab
|
||||||
def tree_entry(path)
|
def tree_entry(path)
|
||||||
return unless path.present?
|
return unless path.present?
|
||||||
|
|
||||||
|
commit_tree_entry(path)
|
||||||
|
end
|
||||||
|
|
||||||
|
def commit_tree_entry(path)
|
||||||
# We're only interested in metadata, so limit actual data to 1 byte
|
# We're only interested in metadata, so limit actual data to 1 byte
|
||||||
# since Gitaly doesn't support "send no data" option.
|
# since Gitaly doesn't support "send no data" option.
|
||||||
entry = @repository.gitaly_commit_client.tree_entry(id, path, 1)
|
entry = @repository.gitaly_commit_client.tree_entry(id, path, 1)
|
||||||
return unless entry
|
return unless entry
|
||||||
|
|
||||||
|
# To be compatible with the rugged format
|
||||||
entry = entry.to_h
|
entry = entry.to_h
|
||||||
entry.delete(:data)
|
entry.delete(:data)
|
||||||
entry[:name] = File.basename(path)
|
entry[:name] = File.basename(path)
|
||||||
|
|
|
@ -43,6 +43,30 @@ module Gitlab
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
override :commit_tree_entry
|
||||||
|
def commit_tree_entry(path)
|
||||||
|
if Feature.enabled?(:rugged_commit_tree_entry)
|
||||||
|
rugged_tree_entry(path)
|
||||||
|
else
|
||||||
|
super
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Is this the same as Blob.find_entry_by_path ?
|
||||||
|
def rugged_tree_entry(path)
|
||||||
|
rugged_commit.tree.path(path)
|
||||||
|
rescue Rugged::TreeError
|
||||||
|
nil
|
||||||
|
end
|
||||||
|
|
||||||
|
def rugged_commit
|
||||||
|
@rugged_commit ||= if raw_commit.is_a?(Rugged::Commit)
|
||||||
|
raw_commit
|
||||||
|
else
|
||||||
|
@repository.rev_parse_target(id)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
def init_from_rugged(commit)
|
def init_from_rugged(commit)
|
||||||
author = commit.author
|
author = commit.author
|
||||||
committer = commit.committer
|
committer = commit.committer
|
||||||
|
|
|
@ -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).freeze
|
FEATURE_FLAGS = %i(rugged_find_commit rugged_tree_entries rugged_tree_entry rugged_commit_is_ancestor rugged_commit_tree_entry).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) }
|
||||||
|
|
|
@ -542,7 +542,7 @@ eos
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#uri_type' do
|
shared_examples '#uri_type' do
|
||||||
it 'returns the URI type at the given path' do
|
it 'returns the URI type at the given path' do
|
||||||
expect(commit.uri_type('files/html')).to be(:tree)
|
expect(commit.uri_type('files/html')).to be(:tree)
|
||||||
expect(commit.uri_type('files/images/logo-black.png')).to be(:raw)
|
expect(commit.uri_type('files/images/logo-black.png')).to be(:raw)
|
||||||
|
@ -561,6 +561,20 @@ eos
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe '#uri_type with Gitaly enabled' do
|
||||||
|
it_behaves_like "#uri_type"
|
||||||
|
end
|
||||||
|
|
||||||
|
describe '#uri_type with Rugged enabled', :enable_rugged do
|
||||||
|
it 'calls out to the Rugged implementation' do
|
||||||
|
allow_any_instance_of(Rugged::Tree).to receive(:path).with('files/html').and_call_original
|
||||||
|
|
||||||
|
commit.uri_type('files/html')
|
||||||
|
end
|
||||||
|
|
||||||
|
it_behaves_like '#uri_type'
|
||||||
|
end
|
||||||
|
|
||||||
describe '.from_hash' do
|
describe '.from_hash' do
|
||||||
let(:new_commit) { described_class.from_hash(commit.to_hash, project) }
|
let(:new_commit) { described_class.from_hash(commit.to_hash, project) }
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue