Commit 5d4c6c17 authored by Kamil Trzciński's avatar Kamil Trzciński Committed by Aleksei Lipniagov

Do not use `restored_project`

This removes the need for `restored_project`,
but rather drops all relation caches.

It saves one SQL query per-restored top-level
object, which if you have 40k MRs,
this saves 40k SQL queries.
parent 56987a07
...@@ -9,11 +9,9 @@ module EE ...@@ -9,11 +9,9 @@ module EE
private private
attr_accessor :project
override :remove_feature_dependent_sub_relations override :remove_feature_dependent_sub_relations
def remove_feature_dependent_sub_relations(relation_item) def remove_feature_dependent_sub_relations(relation_item)
if relation_item.is_a?(Hash) && ::Feature.disabled?(:export_designs, project, default_enabled: true) if relation_item.is_a?(Hash) && ::Feature.disabled?(:export_designs, @project, default_enabled: true)
relation_item.except!('designs', 'design_versions') relation_item.except!('designs', 'design_versions')
end end
end end
......
...@@ -6,19 +6,20 @@ module Gitlab ...@@ -6,19 +6,20 @@ module Gitlab
# Relations which cannot be saved at project level (and have a group assigned) # Relations which cannot be saved at project level (and have a group assigned)
GROUP_MODELS = [GroupLabel, Milestone].freeze GROUP_MODELS = [GroupLabel, Milestone].freeze
attr_reader :user
attr_reader :restored_project
def initialize(user:, shared:, project:) def initialize(user:, shared:, project:)
@path = File.join(shared.export_path, 'project.json') @path = File.join(shared.export_path, 'project.json')
@user = user @user = user
@shared = shared @shared = shared
@project = project @project = project
@project_id = project.id
@saved = true @saved = true
end end
def restore def restore
begin begin
json = IO.read(@path) @tree_hash = read_tree_hash
@tree_hash = ActiveSupport::JSON.decode(json)
rescue => e rescue => e
Rails.logger.error("Import/Export error: #{e.message}") # rubocop:disable Gitlab/RailsLogger Rails.logger.error("Import/Export error: #{e.message}") # rubocop:disable Gitlab/RailsLogger
raise Gitlab::ImportExport::Error.new('Incorrect JSON format') raise Gitlab::ImportExport::Error.new('Incorrect JSON format')
...@@ -30,26 +31,32 @@ module Gitlab ...@@ -30,26 +31,32 @@ module Gitlab
ActiveRecord::Base.uncached do ActiveRecord::Base.uncached do
ActiveRecord::Base.no_touching do ActiveRecord::Base.no_touching do
update_project_params
create_relations create_relations
end end
end end
# ensure that we have latest version
# of the restore
@restored_project = @project.reload
true
rescue => e rescue => e
@shared.error(e) @shared.error(e)
false false
end end
def restored_project private
return @project unless @tree_hash
@restored_project ||= restore_project def read_tree_hash
json = IO.read(@path)
ActiveSupport::JSON.decode(json)
end end
private
def members_mapper def members_mapper
@members_mapper ||= Gitlab::ImportExport::MembersMapper.new(exported_members: @project_members, @members_mapper ||= Gitlab::ImportExport::MembersMapper.new(exported_members: @project_members,
user: @user, user: @user,
project: restored_project) project: @project)
end end
# A Hash of the imported merge request ID -> imported ID. # A Hash of the imported merge request ID -> imported ID.
...@@ -83,13 +90,12 @@ module Gitlab ...@@ -83,13 +90,12 @@ module Gitlab
remove_group_models(relation_hash) if relation_hash.is_a?(Array) remove_group_models(relation_hash) if relation_hash.is_a?(Array)
@saved = false unless restored_project.append_or_update_attribute(relation_key, relation_hash) @saved = false unless @project.append_or_update_attribute(relation_key, relation_hash)
save_id_mappings(relation_key, relation_hash_batch, relation_hash) save_id_mappings(relation_key, relation_hash_batch, relation_hash)
# Restore the project again, extra query that skips holding the AR objects in memory @project.reset
@restored_project = Project.find(@project_id) end
end
# Older, serialized CI pipeline exports may only have a # Older, serialized CI pipeline exports may only have a
# merge_request_id and not the full hash of the merge request. To # merge_request_id and not the full hash of the merge request. To
...@@ -127,12 +133,10 @@ module Gitlab ...@@ -127,12 +133,10 @@ module Gitlab
reader.attributes_finder.find_relations_tree(:project) reader.attributes_finder.find_relations_tree(:project)
end end
def restore_project def update_project_params
Gitlab::Timeless.timeless(@project) do Gitlab::Timeless.timeless(@project) do
@project.update(project_params) @project.update(project_params)
end end
@project
end end
def project_params def project_params
...@@ -245,7 +249,7 @@ module Gitlab ...@@ -245,7 +249,7 @@ module Gitlab
members_mapper: members_mapper, members_mapper: members_mapper,
merge_requests_mapping: merge_requests_mapping, merge_requests_mapping: merge_requests_mapping,
user: @user, user: @user,
project: @restored_project, project: @project,
excluded_keys: excluded_keys_for_relation(relation_key)) excluded_keys: excluded_keys_for_relation(relation_key))
end.compact end.compact
......
...@@ -520,19 +520,20 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do ...@@ -520,19 +520,20 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
end end
end end
describe '#restored_project' do context 'Minimal JSON' do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:tree_hash) { { 'visibility_level' => visibility } } let(:tree_hash) { { 'visibility_level' => visibility } }
let(:restorer) { described_class.new(user: nil, shared: shared, project: project) } let(:restorer) { described_class.new(user: nil, shared: shared, project: project) }
before do before do
restorer.instance_variable_set(:@tree_hash, tree_hash) expect(restorer).to receive(:read_tree_hash) { tree_hash }
end end
context 'no group visibility' do context 'no group visibility' do
let(:visibility) { Gitlab::VisibilityLevel::PRIVATE } let(:visibility) { Gitlab::VisibilityLevel::PRIVATE }
it 'uses the project visibility' do it 'uses the project visibility' do
expect(restorer.restore).to eq(true)
expect(restorer.restored_project.visibility_level).to eq(visibility) expect(restorer.restored_project.visibility_level).to eq(visibility)
end end
end end
...@@ -544,6 +545,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do ...@@ -544,6 +545,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
it 'uses private visibility' do it 'uses private visibility' do
stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL])
expect(restorer.restore).to eq(true)
expect(restorer.restored_project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) expect(restorer.restored_project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
end end
end end
...@@ -561,6 +563,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do ...@@ -561,6 +563,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
let(:visibility) { Gitlab::VisibilityLevel::PUBLIC } let(:visibility) { Gitlab::VisibilityLevel::PUBLIC }
it 'uses the group visibility' do it 'uses the group visibility' do
expect(restorer.restore).to eq(true)
expect(restorer.restored_project.visibility_level).to eq(group_visibility) expect(restorer.restored_project.visibility_level).to eq(group_visibility)
end end
end end
...@@ -570,6 +573,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do ...@@ -570,6 +573,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
let(:visibility) { Gitlab::VisibilityLevel::PRIVATE } let(:visibility) { Gitlab::VisibilityLevel::PRIVATE }
it 'uses the project visibility' do it 'uses the project visibility' do
expect(restorer.restore).to eq(true)
expect(restorer.restored_project.visibility_level).to eq(visibility) expect(restorer.restored_project.visibility_level).to eq(visibility)
end end
end end
...@@ -579,6 +583,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do ...@@ -579,6 +583,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
let(:visibility) { Gitlab::VisibilityLevel::PUBLIC } let(:visibility) { Gitlab::VisibilityLevel::PUBLIC }
it 'uses the group visibility' do it 'uses the group visibility' do
expect(restorer.restore).to eq(true)
expect(restorer.restored_project.visibility_level).to eq(group_visibility) expect(restorer.restored_project.visibility_level).to eq(group_visibility)
end end
...@@ -586,6 +591,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do ...@@ -586,6 +591,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
it 'sets private visibility' do it 'sets private visibility' do
stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL])
expect(restorer.restore).to eq(true)
expect(restorer.restored_project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) expect(restorer.restored_project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
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