Commit 37dd0737 authored by Stan Hu's avatar Stan Hu

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
parent 39340e42
...@@ -12,4 +12,20 @@ class LfsObjectUploader < GitlabUploader ...@@ -12,4 +12,20 @@ class LfsObjectUploader < GitlabUploader
def filename def filename
model.oid[4..-1] model.oid[4..-1]
end 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 end
---
title: Fix LFS timeouts when trying to save large files
merge_request:
author:
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
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