From 4ee08fd1f71caca88243833f08c2b1a59bd0fa9e Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Thu, 7 Mar 2019 13:53:22 -0800 Subject: 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. --- .../unreleased/sh-rugged-commit-tree-entry.yml | 5 +++++ lib/gitlab/git/commit.rb | 5 +++++ lib/gitlab/git/rugged_impl/commit.rb | 24 ++++++++++++++++++++++ lib/gitlab/git/rugged_impl/repository.rb | 2 +- spec/models/commit_spec.rb | 16 ++++++++++++++- 5 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/sh-rugged-commit-tree-entry.yml diff --git a/changelogs/unreleased/sh-rugged-commit-tree-entry.yml b/changelogs/unreleased/sh-rugged-commit-tree-entry.yml new file mode 100644 index 00000000000..bcefa2c7112 --- /dev/null +++ b/changelogs/unreleased/sh-rugged-commit-tree-entry.yml @@ -0,0 +1,5 @@ +--- +title: Bring back Rugged implementation of commit_tree_entry +merge_request: 25896 +author: +type: other diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb index 491e4b47196..e5bbd500e98 100644 --- a/lib/gitlab/git/commit.rb +++ b/lib/gitlab/git/commit.rb @@ -314,11 +314,16 @@ module Gitlab def tree_entry(path) 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 # since Gitaly doesn't support "send no data" option. entry = @repository.gitaly_commit_client.tree_entry(id, path, 1) return unless entry + # To be compatible with the rugged format entry = entry.to_h entry.delete(:data) entry[:name] = File.basename(path) diff --git a/lib/gitlab/git/rugged_impl/commit.rb b/lib/gitlab/git/rugged_impl/commit.rb index 251802878c3..f6777dfa0c3 100644 --- a/lib/gitlab/git/rugged_impl/commit.rb +++ b/lib/gitlab/git/rugged_impl/commit.rb @@ -43,6 +43,30 @@ module Gitlab 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) author = commit.author committer = commit.committer diff --git a/lib/gitlab/git/rugged_impl/repository.rb b/lib/gitlab/git/rugged_impl/repository.rb index fe0120b1199..c0a91f59ab9 100644 --- a/lib/gitlab/git/rugged_impl/repository.rb +++ b/lib/gitlab/git/rugged_impl/repository.rb @@ -12,7 +12,7 @@ module Gitlab module Repository 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 relative_object_directories.map { |d| File.join(path, d) } diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index baad8352185..9d4e18534ae 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -542,7 +542,7 @@ eos end end - describe '#uri_type' do + shared_examples '#uri_type' 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/images/logo-black.png')).to be(:raw) @@ -561,6 +561,20 @@ eos 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 let(:new_commit) { described_class.from_hash(commit.to_hash, project) } -- cgit v1.2.1