Commit 588d5630 authored by Sean McGivern's avatar Sean McGivern

Remove GITLAB_TEMPFILE_IMMEDIATE_UNLINK env var

We have rolled this out in production and seen that it works: no
increase in error rate, and temporary files do not hang around any more.
parent 26be8f2c
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
if Gitlab::Runtime.puma? if Gitlab::Runtime.puma?
raise "Remove this monkey patch: #{__FILE__}" unless Puma::Const::VERSION == '5.1.1' raise "Remove this monkey patch: #{__FILE__}" unless Puma::Const::VERSION == '5.1.1'
if ENV['GITLAB_TEMPFILE_IMMEDIATE_UNLINK'] == '1'
# This is copied from https://github.com/puma/puma/blob/v5.1.1/lib/puma/client.rb, # This is copied from https://github.com/puma/puma/blob/v5.1.1/lib/puma/client.rb,
# with two additions: both times we create a temporary file, we immediately # with two additions: both times we create a temporary file, we immediately
# call `#unlink`. This means that if the process gets terminated without being # call `#unlink`. This means that if the process gets terminated without being
...@@ -99,5 +98,4 @@ if Gitlab::Runtime.puma? ...@@ -99,5 +98,4 @@ if Gitlab::Runtime.puma?
end end
end end
end end
end
end end
...@@ -14,9 +14,7 @@ module Gitlab ...@@ -14,9 +14,7 @@ module Gitlab
end end
def call(env) def call(env)
if ENV['GITLAB_TEMPFILE_IMMEDIATE_UNLINK'] == '1'
env[Rack::RACK_MULTIPART_TEMPFILE_FACTORY] = FACTORY env[Rack::RACK_MULTIPART_TEMPFILE_FACTORY] = FACTORY
end
@app.call(env) @app.call(env)
end end
......
...@@ -42,11 +42,6 @@ RSpec.describe Gitlab::Middleware::RackMultipartTempfileFactory do ...@@ -42,11 +42,6 @@ RSpec.describe Gitlab::Middleware::RackMultipartTempfileFactory do
context 'for a multipart request' do context 'for a multipart request' do
let(:env) { Rack::MockRequest.env_for('/', multipart_fixture) } let(:env) { Rack::MockRequest.env_for('/', multipart_fixture) }
context 'when the environment variable is enabled' do
before do
stub_env('GITLAB_TEMPFILE_IMMEDIATE_UNLINK', '1')
end
it 'immediately unlinks the temporary file' do it 'immediately unlinks the temporary file' do
tempfile = Tempfile.new('foo') tempfile = Tempfile.new('foo')
...@@ -64,25 +59,6 @@ RSpec.describe Gitlab::Middleware::RackMultipartTempfileFactory do ...@@ -64,25 +59,6 @@ RSpec.describe Gitlab::Middleware::RackMultipartTempfileFactory do
end end
end end
context 'when the environment variable is disabled' do
it 'does not immediately unlink the temporary file' do
tempfile = Tempfile.new('foo')
expect(tempfile.path).not_to be(nil)
expect(Rack::Multipart::Parser::TEMPFILE_FACTORY).to receive(:call).and_return(tempfile)
expect(tempfile).not_to receive(:unlink).and_call_original
subject.call(env)
expect(tempfile.path).not_to be(nil)
end
it 'processes the request as normal' do
expect(subject.call(env)).to eq([200, { 'Content-Type' => 'image/jpeg' }, [file_contents]])
end
end
end
context 'for a regular request' do context 'for a regular request' do
let(:env) { Rack::MockRequest.env_for('/', params: { 'foo' => 'bar' }) } let(:env) { Rack::MockRequest.env_for('/', params: { 'foo' => 'bar' }) }
......
...@@ -1634,7 +1634,6 @@ RSpec.describe API::Projects do ...@@ -1634,7 +1634,6 @@ RSpec.describe API::Projects do
end end
it "does not leave the temporary file in place after uploading, even when the tempfile reaper does not run" do it "does not leave the temporary file in place after uploading, even when the tempfile reaper does not run" do
stub_env('GITLAB_TEMPFILE_IMMEDIATE_UNLINK', '1')
tempfile = Tempfile.new('foo') tempfile = Tempfile.new('foo')
path = tempfile.path path = tempfile.path
......
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