diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION
index ee1e4d2aee50f5b556f579ac7ae7b902708ab0cb..3ba7bd5ba8314c6449c210bf027b99fd32eaa58c 100644
--- a/GITALY_SERVER_VERSION
+++ b/GITALY_SERVER_VERSION
@@ -1 +1 @@
-0.122.0
+0.123.0
diff --git a/Gemfile b/Gemfile
index 35e83a530f07938634e521a8e350306326c8c5d7..dcb97d1e7ad650380f57f1d1d8ad43207b5beeb3 100644
--- a/Gemfile
+++ b/Gemfile
@@ -423,7 +423,7 @@ group :ed25519 do
end
# Gitaly GRPC client
-gem 'gitaly-proto', '~> 0.117.0', require: 'gitaly'
+gem 'gitaly-proto', '~> 0.118.1', require: 'gitaly'
gem 'grpc', '~> 1.11.0'
# Locked until https://github.com/google/protobuf/issues/4210 is closed
diff --git a/Gemfile.lock b/Gemfile.lock
index d8eaaac99b1a0e0a36e9dd7dc58473cd71b459ae..8e8abb05a6779c624022898fdd6aa345b00e9df8 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -276,7 +276,7 @@ GEM
gettext_i18n_rails (>= 0.7.1)
po_to_json (>= 1.0.0)
rails (>= 3.2.0)
- gitaly-proto (0.117.0)
+ gitaly-proto (0.118.1)
google-protobuf (~> 3.1)
grpc (~> 1.10)
github-linguist (5.3.3)
@@ -1031,7 +1031,7 @@ DEPENDENCIES
gettext (~> 3.2.2)
gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.3)
- gitaly-proto (~> 0.117.0)
+ gitaly-proto (~> 0.118.1)
github-linguist (~> 5.3.3)
gitlab-flowdock-git-hook (~> 1.0.1)
gitlab-gollum-lib (~> 4.2)
diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock
index ab35a4a399fc904e225e4d025d9842d712b9b99e..a7728af0ebe87d412ea76539ea50325880dcc4ca 100644
--- a/Gemfile.rails5.lock
+++ b/Gemfile.rails5.lock
@@ -279,7 +279,7 @@ GEM
gettext_i18n_rails (>= 0.7.1)
po_to_json (>= 1.0.0)
rails (>= 3.2.0)
- gitaly-proto (0.117.0)
+ gitaly-proto (0.118.1)
google-protobuf (~> 3.1)
grpc (~> 1.10)
github-linguist (5.3.3)
@@ -1040,7 +1040,7 @@ DEPENDENCIES
gettext (~> 3.2.2)
gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.3)
- gitaly-proto (~> 0.117.0)
+ gitaly-proto (~> 0.118.1)
github-linguist (~> 5.3.3)
gitlab-flowdock-git-hook (~> 1.0.1)
gitlab-gollum-lib (~> 4.2)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 4fecdb3c1ad6f72dca68034700a40f6b8617cdc7..a3a3ce179fc8ce7ca8e82f7ef94443e6f42d13f4 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -668,6 +668,14 @@ def blob_at_branch(branch_name, path)
end
end
+ def list_last_commits_for_tree(sha, path, offset: 0, limit: 25)
+ commits = raw_repository.list_last_commits_for_tree(sha, path, offset: offset, limit: limit)
+
+ commits.each do |path, commit|
+ commits[path] = ::Commit.new(commit, @project)
+ end
+ end
+
def last_commit_for_path(sha, path)
commit = raw_repository.last_commit_for_path(sha, path)
::Commit.new(commit, @project) if commit
diff --git a/changelogs/unreleased/37433-solve-n-1-in-refs-controller-logs-tree.yml b/changelogs/unreleased/37433-solve-n-1-in-refs-controller-logs-tree.yml
new file mode 100644
index 0000000000000000000000000000000000000000..04662a7cfe223ac71c530f69a658e5d603c20dbb
--- /dev/null
+++ b/changelogs/unreleased/37433-solve-n-1-in-refs-controller-logs-tree.yml
@@ -0,0 +1,6 @@
+---
+title: Adds support for Gitaly ListLastCommitsForTree RPC in order to make bulk-fetch
+ of commits more performant
+merge_request: 21921
+author:
+type: performance
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index 3d5a63bdbacd8a1d09b9f2253bb9f8e5cafabacc..45d42c7078f02345f06c9c167159dde370b79918 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -953,6 +953,12 @@ def find_commits_by_message(query, ref, path, limit, offset)
end
end
+ def list_last_commits_for_tree(sha, path, offset: 0, limit: 25)
+ wrapped_gitaly_errors do
+ gitaly_commit_client.list_last_commits_for_tree(sha, path, offset: offset, limit: limit)
+ end
+ end
+
def last_commit_for_path(sha, path)
wrapped_gitaly_errors do
gitaly_commit_client.last_commit_for_path(sha, path)
diff --git a/lib/gitlab/gitaly_client/commit_service.rb b/lib/gitlab/gitaly_client/commit_service.rb
index 07e5e204b68b1e39d69d4e2c4b890701648d5a4b..085b2a127a52be244aa82d360e2d7d3c1baa18d5 100644
--- a/lib/gitlab/gitaly_client/commit_service.rb
+++ b/lib/gitlab/gitaly_client/commit_service.rb
@@ -148,6 +148,24 @@ def commit_count(ref, options = {})
GitalyClient.call(@repository.storage, :commit_service, :count_commits, request, timeout: GitalyClient.medium_timeout).count
end
+ def list_last_commits_for_tree(revision, path, offset: 0, limit: 25)
+ request = Gitaly::ListLastCommitsForTreeRequest.new(
+ repository: @gitaly_repo,
+ revision: encode_binary(revision),
+ path: encode_binary(path.to_s),
+ offset: offset,
+ limit: limit
+ )
+
+ response = GitalyClient.call(@repository.storage, :commit_service, :list_last_commits_for_tree, request, timeout: GitalyClient.medium_timeout)
+
+ response.each_with_object({}) do |gitaly_response, hsh|
+ gitaly_response.commits.each do |commit_for_tree|
+ hsh[commit_for_tree.path] = Gitlab::Git::Commit.new(@repository, commit_for_tree.commit)
+ end
+ end
+ end
+
def last_commit_for_path(revision, path)
request = Gitaly::LastCommitForPathRequest.new(
repository: @gitaly_repo,
diff --git a/lib/gitlab/tree_summary.rb b/lib/gitlab/tree_summary.rb
index b05d408b1c0f2a94e935deebc7020dce6cef29a0..c2955cd374c84f719eb667bd4e0296391ebe4e25 100644
--- a/lib/gitlab/tree_summary.rb
+++ b/lib/gitlab/tree_summary.rb
@@ -73,25 +73,29 @@ def build_entry(entry)
end
def fill_last_commits!(entries)
- # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37433
- Gitlab::GitalyClient.allow_n_plus_1_calls do
- entries.each do |entry|
- raw_commit = repository.last_commit_for_path(commit.id, entry_path(entry))
+ # Ensure the path is in "path/" format
+ ensured_path =
+ if path
+ File.join(*[path, ""])
+ end
+
+ commits_hsh = repository.list_last_commits_for_tree(commit.id, ensured_path, offset: offset, limit: limit)
- if raw_commit
- commit = resolve_commit(raw_commit)
+ entries.each do |entry|
+ path_key = entry_path(entry)
+ commit = cache_commit(commits_hsh[path_key])
- entry[:commit] = commit
- entry[:commit_path] = commit_path(commit)
- end
+ if commit
+ entry[:commit] = commit
+ entry[:commit_path] = commit_path(commit)
end
end
end
- def resolve_commit(raw_commit)
- return nil unless raw_commit.present?
+ def cache_commit(commit)
+ return nil unless commit.present?
- resolved_commits[raw_commit.id] ||= ::Commit.new(raw_commit, project)
+ resolved_commits[commit.id] ||= commit
end
def commit_path(commit)
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index 7e1b7c355179a217dda4fb2f582f97f120706ab9..784d17e271e7089a6aa12b13efab0d84b734b555 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -188,6 +188,57 @@ def expect_to_raise_storage_error
end
end
+ describe '#list_last_commits_for_tree' do
+ let(:path_to_commit) do
+ {
+ "encoding" => "913c66a37b4a45b9769037c55c2d238bd0942d2e",
+ "files" => "570e7b2abdd848b95f2f578043fc23bd6f6fd24d",
+ ".gitignore" => "c1acaa58bbcbc3eafe538cb8274ba387047b69f8",
+ ".gitmodules" => "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9",
+ "CHANGELOG" => "913c66a37b4a45b9769037c55c2d238bd0942d2e",
+ "CONTRIBUTING.md" => "6d394385cf567f80a8fd85055db1ab4c5295806f",
+ "Gemfile.zip" => "ae73cb07c9eeaf35924a10f713b364d32b2dd34f",
+ "LICENSE" => "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863",
+ "MAINTENANCE.md" => "913c66a37b4a45b9769037c55c2d238bd0942d2e",
+ "PROCESS.md" => "913c66a37b4a45b9769037c55c2d238bd0942d2e",
+ "README.md" => "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863",
+ "VERSION" => "913c66a37b4a45b9769037c55c2d238bd0942d2e",
+ "gitlab-shell" => "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9",
+ "six" => "cfe32cf61b73a0d5e9f13e774abde7ff789b1660"
+ }
+ end
+
+ subject { repository.list_last_commits_for_tree(sample_commit.id, '.').id }
+
+ it 'returns the last commits for every entry in the current path' do
+ result = repository.list_last_commits_for_tree(sample_commit.id, '.')
+
+ result.each do |key, value|
+ result[key] = value.id
+ end
+
+ expect(result).to include(path_to_commit)
+ end
+
+ it 'returns the last commits for every entry in the current path starting from the offset' do
+ result = repository.list_last_commits_for_tree(sample_commit.id, '.', offset: path_to_commit.size - 1)
+
+ expect(result.size).to eq(1)
+ end
+
+ it 'returns a limited number of last commits for every entry in the current path starting from the offset' do
+ result = repository.list_last_commits_for_tree(sample_commit.id, '.', limit: 1)
+
+ expect(result.size).to eq(1)
+ end
+
+ it 'returns an empty hash when offset is out of bounds' do
+ result = repository.list_last_commits_for_tree(sample_commit.id, '.', offset: path_to_commit.size)
+
+ expect(result.size).to eq(0)
+ end
+ end
+
describe '#last_commit_for_path' do
shared_examples 'getting last commit for path' do
subject { repository.last_commit_for_path(sample_commit.id, '.gitignore').id }