summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRémy Coutable <remy@rymai.me>2018-03-05 15:26:44 +0000
committerRémy Coutable <remy@rymai.me>2018-03-05 15:26:44 +0000
commite02502f998bee8105d8171cd0df9dd8c1689088d (patch)
tree2043126193850ebddce64d5e5aefdbf07888fc41
parentdf23494cd98f12e4341c5cc12764364404eabfad (diff)
parent0a4ee10eda01e23ddea0e6b4f80f61df3ffaabde (diff)
downloadgitlab-ce-e02502f998bee8105d8171cd0df9dd8c1689088d.tar.gz
Merge branch '37430-projects-comparecontroller-show-is-calling-gitaly-n-1-times-per-request' into 'master'
Resolve "Projects::CompareController#show is calling Gitaly n+1 times per request" Closes #37430 See merge request gitlab-org/gitlab-ce!17439
-rw-r--r--app/controllers/projects/compare_controller.rb6
-rw-r--r--lib/gitlab/git/blob.rb14
-rw-r--r--spec/lib/gitlab/git/blob_spec.rb29
3 files changed, 40 insertions, 9 deletions
diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb
index 3cb4eb23981..2b0c2ca97c0 100644
--- a/app/controllers/projects/compare_controller.rb
+++ b/app/controllers/projects/compare_controller.rb
@@ -17,10 +17,8 @@ class Projects::CompareController < Projects::ApplicationController
def show
apply_diff_view_cookie!
- # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37430
- Gitlab::GitalyClient.allow_n_plus_1_calls do
- render
- end
+
+ render
end
def diff_for_path
diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb
index b2fca2c16de..eabcf46cf58 100644
--- a/lib/gitlab/git/blob.rb
+++ b/lib/gitlab/git/blob.rb
@@ -238,9 +238,9 @@ module Gitlab
self.__send__("#{key}=", options[key.to_sym]) # rubocop:disable GitlabSecurity/PublicSend
end
- @loaded_all_data = false
# Retain the actual size before it is encoded
@loaded_size = @data.bytesize if @data
+ @loaded_all_data = @loaded_size == size
end
def binary?
@@ -255,10 +255,15 @@ module Gitlab
# memory as a Ruby string.
def load_all_data!(repository)
return if @data == '' # don't mess with submodule blobs
- return @data if @loaded_all_data
- Gitlab::GitalyClient.migrate(:git_blob_load_all_data) do |is_enabled|
- @data = begin
+ # Even if we return early, recalculate wether this blob is binary in
+ # case a blob was initialized as text but the full data isn't
+ @binary = nil
+
+ return if @loaded_all_data
+
+ @data = Gitlab::GitalyClient.migrate(:git_blob_load_all_data) do |is_enabled|
+ begin
if is_enabled
repository.gitaly_blob_client.get_blob(oid: id, limit: -1).data
else
@@ -269,7 +274,6 @@ module Gitlab
@loaded_all_data = true
@loaded_size = @data.bytesize
- @binary = nil
end
def name
diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb
index a6341cd509b..67d898e787e 100644
--- a/spec/lib/gitlab/git/blob_spec.rb
+++ b/spec/lib/gitlab/git/blob_spec.rb
@@ -500,4 +500,33 @@ describe Gitlab::Git::Blob, seed_helper: true do
end
end
end
+
+ describe '#load_all_data!' do
+ let(:full_data) { 'abcd' }
+ let(:blob) { Gitlab::Git::Blob.new(name: 'test', size: 4, data: 'abc') }
+
+ subject { blob.load_all_data!(repository) }
+
+ it 'loads missing data' do
+ expect(Gitlab::GitalyClient).to receive(:migrate)
+ .with(:git_blob_load_all_data).and_return(full_data)
+
+ subject
+
+ expect(blob.data).to eq(full_data)
+ end
+
+ context 'with a fully loaded blob' do
+ let(:blob) { Gitlab::Git::Blob.new(name: 'test', size: 4, data: full_data) }
+
+ it "doesn't perform any loading" do
+ expect(Gitlab::GitalyClient).not_to receive(:migrate)
+ .with(:git_blob_load_all_data)
+
+ subject
+
+ expect(blob.data).to eq(full_data)
+ end
+ end
+ end
end