From 37dd073782520f70d021e8ca9960e0a1c9696129 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Thu, 1 Jun 2017 13:15:46 -0700 Subject: Fix LFS timeouts when trying to save large files The following was happening: 1. Workhorse stores an LFS file in /var/opt/gitlab/gitlab-rails/shared/lfs-objects 2. CarrierWave then renames the file to a temporary directory (e.g. /opt/gitlab/embedded/service/gitlab-rails/tmp) 3. CarrierWave then renames the file to its final location (e.g. /var/opt/gitlab/gitlab-rails/shared/lfs-objects) When the LFS upload path was on a different filesystem than the Rails installation, step 2 could take a longer than 10 seconds, at which point Workhorse would time out with "badgateway: failed after 10s: context canceled". This change makes the work path in the same root as the LFS storage path, preventing moves across filesystems. Closes #33218 --- app/uploaders/lfs_object_uploader.rb | 16 +++++++++++ .../sh-fix-lfs-from-moving-across-filesystems.yml | 4 +++ spec/uploaders/lfs_object_uploader_spec.rb | 31 ++++++++++++++++++++++ 3 files changed, 51 insertions(+) create mode 100644 changelogs/unreleased/sh-fix-lfs-from-moving-across-filesystems.yml create mode 100644 spec/uploaders/lfs_object_uploader_spec.rb diff --git a/app/uploaders/lfs_object_uploader.rb b/app/uploaders/lfs_object_uploader.rb index 95a891111e1..02589959c2f 100644 --- a/app/uploaders/lfs_object_uploader.rb +++ b/app/uploaders/lfs_object_uploader.rb @@ -12,4 +12,20 @@ class LfsObjectUploader < GitlabUploader def filename model.oid[4..-1] end + + def work_dir + File.join(Gitlab.config.lfs.storage_path, 'tmp', 'work') + end + + private + + # To prevent LFS files from moving across filesystems, override the default + # implementation: + # http://github.com/carrierwaveuploader/carrierwave/blob/v1.0.0/lib/carrierwave/uploader/cache.rb#L181-L183 + def workfile_path(for_file = original_filename) + # To be safe, keep this directory outside of the the cache directory + # because calling CarrierWave.clean_cache_files! will remove any files in + # the cache directory. + File.join(work_dir, @cache_id, version_name.to_s, for_file) + end end diff --git a/changelogs/unreleased/sh-fix-lfs-from-moving-across-filesystems.yml b/changelogs/unreleased/sh-fix-lfs-from-moving-across-filesystems.yml new file mode 100644 index 00000000000..161bce45601 --- /dev/null +++ b/changelogs/unreleased/sh-fix-lfs-from-moving-across-filesystems.yml @@ -0,0 +1,4 @@ +--- +title: Fix LFS timeouts when trying to save large files +merge_request: +author: diff --git a/spec/uploaders/lfs_object_uploader_spec.rb b/spec/uploaders/lfs_object_uploader_spec.rb new file mode 100644 index 00000000000..c3b72e7d677 --- /dev/null +++ b/spec/uploaders/lfs_object_uploader_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe LfsObjectUploader do + let(:uploader) { described_class.new(build_stubbed(:empty_project)) } + + describe '#cache!' do + it 'caches the file in the cache directory' do + # One to get the work dir, the other to remove it + expect(uploader).to receive(:workfile_path).exactly(2).times.and_call_original + expect(FileUtils).to receive(:mv).with(anything, /^#{uploader.work_dir}/).and_call_original + expect(FileUtils).to receive(:mv).with(/^#{uploader.work_dir}/, /^#{uploader.cache_dir}/).and_call_original + + fixture = Rails.root.join('spec', 'fixtures', 'rails_sample.jpg') + uploader.cache!(fixture_file_upload(fixture)) + + expect(uploader.file.path).to start_with(uploader.cache_dir) + end + end + + describe '#move_to_cache' do + it 'is true' do + expect(uploader.move_to_cache).to eq(true) + end + end + + describe '#move_to_store' do + it 'is true' do + expect(uploader.move_to_store).to eq(true) + end + end +end -- cgit v1.2.3