Skip to content
Snippets Groups Projects
Commit ff61a6b2 authored by Stan Hu's avatar Stan Hu
Browse files

Bring back Rugged implementation of GetTreeEntries

This brings back some of the changes in
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/20343.

For users using Gitaly on top of NFS, accessing the Git data directly
via Rugged is more performant than Gitaly. This merge request introduces
the feature flag `rugged_tree_entries` to activate the Rugged method.

Part of four Rugged changes identified in
https://gitlab.com/gitlab-org/gitlab-ce/issues/57317.
parent db70f74d
No related branches found
No related tags found
Loading
Pipeline #50250354 failed
---
title: Bring back Rugged implementation of GetTreeEntries
merge_request: 25674
author:
type: other
......@@ -10,7 +10,7 @@ module Gitlab
module Git
module RuggedImpl
module Repository
FEATURE_FLAGS = %i(rugged_find_commit).freeze
FEATURE_FLAGS = %i(rugged_find_commit rugged_tree_entries rugged_tree_entry rugged_commit_is_ancestor).freeze
def alternate_object_directories
relative_object_directories.map { |d| File.join(path, d) }
......@@ -41,6 +41,11 @@ def rev_parse_target(revspec)
obj = rugged.rev_parse(revspec)
Ref.dereference_object(obj)
end
# Lookup for rugged object by oid or ref name
def lookup(oid_or_ref_name)
rugged.rev_parse(oid_or_ref_name)
end
end
end
end
......
# frozen_string_literal: true
# NOTE: This code is legacy. Do not add/modify code here unless you have
# discussed with the Gitaly team. See
# https://docs.gitlab.com/ee/development/gitaly.html#legacy-rugged-code
# for more details.
module Gitlab
module Git
module RuggedImpl
module Tree
module ClassMethods
extend ::Gitlab::Utils::Override
override :tree_entries
def tree_entries(repository, sha, path, recursive)
if Feature.enabled?(:rugged_tree_entries)
tree_entries_from_rugged(repository, sha, path, recursive)
else
super
end
end
def tree_entries_from_rugged(repository, sha, path, recursive)
current_path_entries = get_tree_entries_from_rugged(repository, sha, path)
ordered_entries = []
current_path_entries.each do |entry|
ordered_entries << entry
if recursive && entry.dir?
ordered_entries.concat(tree_entries_from_rugged(repository, sha, entry.path, true))
end
end
ordered_entries
end
def get_tree_entries_from_rugged(repository, sha, path)
commit = repository.lookup(sha)
root_tree = commit.tree
tree = if path
id = find_id_by_path(repository, root_tree.oid, path)
if id
repository.lookup(id)
else
[]
end
else
root_tree
end
tree.map do |entry|
new(
id: entry[:oid],
root_id: root_tree.oid,
name: entry[:name],
type: entry[:type],
mode: entry[:filemode].to_s(8),
path: path ? File.join(path, entry[:name]) : entry[:name],
commit_id: sha
)
end
rescue Rugged::ReferenceError
[]
end
end
end
end
end
end
......@@ -18,6 +18,10 @@ class << self
def where(repository, sha, path = nil, recursive = false)
path = nil if path == '' || path == '/'
tree_entries(repository, sha, path, recursive)
end
def tree_entries(repository, sha, path, recursive)
wrapped_gitaly_errors do
repository.gitaly_commit_client.tree_entries(repository, sha, path, recursive)
end
......@@ -95,3 +99,5 @@ def contributing?
end
end
end
Gitlab::Git::Tree.singleton_class.prepend Gitlab::Git::RuggedImpl::Tree::ClassMethods
......@@ -3,7 +3,10 @@
describe Gitlab::Git::Tree, :seed_helper do
let(:repository) { Gitlab::Git::Repository.new('default', TEST_REPO_PATH, '', 'group/project') }
context :repo do
# Tree#flat_path was introduced for Gitaly in
# https://gitlab.com/gitlab-org/gitaly/issues/530 to reduce N+1 queries.
# It's not needed by Rugged, and GitLab can function without it.
shared_examples :repo do |skip_flat_path|
let(:tree) { Gitlab::Git::Tree.where(repository, SeedRepo::Commit::ID) }
it { expect(tree).to be_kind_of Array }
......@@ -12,6 +15,17 @@
it { expect(tree.select(&:file?).size).to eq(10) }
it { expect(tree.select(&:submodule?).size).to eq(2) }
it 'returns an empty array when called with an invalid ref' do
expect(described_class.where(repository, 'foobar-does-not-exist')).to eq([])
end
it 'returns a list of tree objects' do
entries = described_class.where(repository, SeedRepo::Commit::ID, 'files', true)
expect(entries.count).to be > 10
expect(entries).to all(be_a(Gitlab::Git::Tree))
end
describe '#dir?' do
let(:dir) { tree.select(&:dir?).first }
......@@ -20,9 +34,13 @@
it { expect(dir.commit_id).to eq(SeedRepo::Commit::ID) }
it { expect(dir.name).to eq('encoding') }
it { expect(dir.path).to eq('encoding') }
it { expect(dir.flat_path).to eq('encoding') }
it { expect(dir.mode).to eq('40000') }
it '#flat_path', skip: skip_flat_path do
expect(dir.flat_path).to eq('encoding')
end
context :subdir do
let(:subdir) { Gitlab::Git::Tree.where(repository, SeedRepo::Commit::ID, 'files').first }
......@@ -31,7 +49,10 @@
it { expect(subdir.commit_id).to eq(SeedRepo::Commit::ID) }
it { expect(subdir.name).to eq('html') }
it { expect(subdir.path).to eq('files/html') }
it { expect(subdir.flat_path).to eq('files/html') }
it '#flat_path', skip: skip_flat_path do
expect(subdir.flat_path).to eq('files/html')
end
end
context :subdir_file do
......@@ -42,7 +63,10 @@
it { expect(subdir_file.commit_id).to eq(SeedRepo::Commit::ID) }
it { expect(subdir_file.name).to eq('popen.rb') }
it { expect(subdir_file.path).to eq('files/ruby/popen.rb') }
it { expect(subdir_file.flat_path).to eq('files/ruby/popen.rb') }
it '#flat_path', skip: skip_flat_path do
expect(subdir_file.flat_path).to eq('files/ruby/popen.rb')
end
end
end
......@@ -79,9 +103,17 @@
end
end
describe '#where' do
it 'returns an empty array when called with an invalid ref' do
expect(described_class.where(repository, 'foobar-does-not-exist')).to eq([])
describe '.where with Gitaly enabled' do
it_behaves_like :repo, false
end
describe '.where with Rugged enabled', :enable_rugged do
it 'calls out to the Rugged implementation' do
allow_any_instance_of(Rugged).to receive(:lookup).with(SeedRepo::Commit::ID)
described_class.where(repository, SeedRepo::Commit::ID, 'files', false)
end
it_behaves_like :repo, true
end
end
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment