Commit 23efa56b authored by Stan Hu's avatar Stan Hu Committed by George Koltsov

Speed up project exports by moving the archive to the cache dir

As described in
https://github.com/carrierwaveuploader/carrierwave#large-files,
CarrierWave will first copy a file into the cache and then copy the
file to its final store.

Previously the `move_to_cache` value was always set to `false`, which
meant that CarrierWave would always copy the generated archive file
(e.g. `/path/tmp/work`) to the cache dir
(e.g. `/path/tmp/cache`). Since both paths are on the same filesystem,
this copy is unnecessary and slows down the generation of project
exports.

To ensure files are moved instead of copied, we can just inherit from
the GitlabUploader implementation of `move_to_cache`, which returns
`true` if it's a local file, `false` otherwise. We have to be careful
to only allow this optimization for project/group exports because
imports might be importing a static template.

For my test with the Linux kernel, this changed saved 47 seconds of
unnecessary I/O with a 3.4 GB archive.

Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/349425

Changelog: performance
parent 7976ea9c
......@@ -12,6 +12,10 @@ class ImportExportUploader < AttachmentUploader
end
def move_to_cache
# Exports create temporary files that we can safely move.
# Imports may be from project templates that we want to copy.
return super if mounted_as == :export_file
false
end
......
......@@ -10,6 +10,20 @@ RSpec.describe ImportExportUploader do
subject { described_class.new(model, :import_file) }
context 'local store' do
describe '#move_to_cache' do
it 'returns false' do
expect(subject.move_to_cache).to be false
end
context 'with project export' do
subject { described_class.new(model, :export_file) }
it 'returns true' do
expect(subject.move_to_cache).to be true
end
end
end
describe '#move_to_store' do
it 'returns true' do
expect(subject.move_to_store).to be true
......@@ -33,6 +47,20 @@ RSpec.describe ImportExportUploader do
let(:fixture) { File.join('spec', 'fixtures', 'group_export.tar.gz') }
end
describe '#move_to_cache' do
it 'returns false' do
expect(subject.move_to_cache).to be false
end
context 'with project export' do
subject { described_class.new(model, :export_file) }
it 'returns true' do
expect(subject.move_to_cache).to be false
end
end
end
describe '#move_to_store' do
it 'returns false' do
expect(subject.move_to_store).to be false
......
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