Commit 0a4ee10e authored by Alejandro Rodríguez's avatar Alejandro Rodríguez

Fix n+1 issue by not reloading fully loaded blobs

parent 369d34c7
...@@ -17,11 +17,9 @@ class Projects::CompareController < Projects::ApplicationController ...@@ -17,11 +17,9 @@ class Projects::CompareController < Projects::ApplicationController
def show def show
apply_diff_view_cookie! apply_diff_view_cookie!
# n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37430
Gitlab::GitalyClient.allow_n_plus_1_calls do
render render
end end
end
def diff_for_path def diff_for_path
return render_404 unless @compare return render_404 unless @compare
......
...@@ -238,9 +238,9 @@ module Gitlab ...@@ -238,9 +238,9 @@ module Gitlab
self.__send__("#{key}=", options[key.to_sym]) # rubocop:disable GitlabSecurity/PublicSend self.__send__("#{key}=", options[key.to_sym]) # rubocop:disable GitlabSecurity/PublicSend
end end
@loaded_all_data = false
# Retain the actual size before it is encoded # Retain the actual size before it is encoded
@loaded_size = @data.bytesize if @data @loaded_size = @data.bytesize if @data
@loaded_all_data = @loaded_size == size
end end
def binary? def binary?
...@@ -255,10 +255,15 @@ module Gitlab ...@@ -255,10 +255,15 @@ module Gitlab
# memory as a Ruby string. # memory as a Ruby string.
def load_all_data!(repository) def load_all_data!(repository)
return if @data == '' # don't mess with submodule blobs 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| # Even if we return early, recalculate wether this blob is binary in
@data = begin # 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 if is_enabled
repository.gitaly_blob_client.get_blob(oid: id, limit: -1).data repository.gitaly_blob_client.get_blob(oid: id, limit: -1).data
else else
...@@ -269,7 +274,6 @@ module Gitlab ...@@ -269,7 +274,6 @@ module Gitlab
@loaded_all_data = true @loaded_all_data = true
@loaded_size = @data.bytesize @loaded_size = @data.bytesize
@binary = nil
end end
def name def name
......
...@@ -500,4 +500,33 @@ describe Gitlab::Git::Blob, seed_helper: true do ...@@ -500,4 +500,33 @@ describe Gitlab::Git::Blob, seed_helper: true do
end end
end 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 end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment