Commit cfe4caaf authored by Bob Van Landuyt's avatar Bob Van Landuyt

Only check request_deadline flag once per request

Every time we check a feature flag using `Feature.enabled?` it could
return a different value based on the percentage that it's enabled
for.

So making sure we only call this value once per request, so we have a
consistent state for the entire request.
parent 452910ec
...@@ -18,7 +18,6 @@ module Gitlab ...@@ -18,7 +18,6 @@ module Gitlab
require_dependency Rails.root.join('lib/gitlab/redis/cache') require_dependency Rails.root.join('lib/gitlab/redis/cache')
require_dependency Rails.root.join('lib/gitlab/redis/queues') require_dependency Rails.root.join('lib/gitlab/redis/queues')
require_dependency Rails.root.join('lib/gitlab/redis/shared_state') require_dependency Rails.root.join('lib/gitlab/redis/shared_state')
require_dependency Rails.root.join('lib/gitlab/request_context')
require_dependency Rails.root.join('lib/gitlab/current_settings') require_dependency Rails.root.join('lib/gitlab/current_settings')
require_dependency Rails.root.join('lib/gitlab/middleware/read_only') require_dependency Rails.root.join('lib/gitlab/middleware/read_only')
require_dependency Rails.root.join('lib/gitlab/middleware/basic_health_check') require_dependency Rails.root.join('lib/gitlab/middleware/basic_health_check')
......
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
module Gitlab module Gitlab
class RequestContext class RequestContext
include Gitlab::Utils::StrongMemoize
include Singleton include Singleton
RequestDeadlineExceeded = Class.new(StandardError) RequestDeadlineExceeded = Class.new(StandardError)
...@@ -15,10 +16,12 @@ module Gitlab ...@@ -15,10 +16,12 @@ module Gitlab
end end
def request_deadline def request_deadline
return unless request_start_time strong_memoize(:request_deadline) do
return unless Feature.enabled?(:request_deadline) next unless request_start_time
next unless Feature.enabled?(:request_deadline)
@request_deadline ||= request_start_time + max_request_duration_seconds request_start_time + max_request_duration_seconds
end
end end
def ensure_deadline_not_exceeded! def ensure_deadline_not_exceeded!
......
...@@ -10,10 +10,12 @@ describe Gitlab::RequestContext, :request_store do ...@@ -10,10 +10,12 @@ describe Gitlab::RequestContext, :request_store do
describe '#request_deadline' do describe '#request_deadline' do
let(:request_start_time) { 1575982156.206008 } let(:request_start_time) { 1575982156.206008 }
it "sets the time to #{Settings.gitlab.max_request_duration_seconds} seconds in the future" do before do
allow(subject).to receive(:request_start_time).and_return(request_start_time) allow(subject).to receive(:request_start_time).and_return(request_start_time)
end
expect(subject.request_deadline).to eq(1575982156.206008 + Settings.gitlab.max_request_duration_seconds) it "sets the time to #{Settings.gitlab.max_request_duration_seconds} seconds in the future" do
expect(subject.request_deadline).to eq(request_start_time + Settings.gitlab.max_request_duration_seconds)
expect(subject.request_deadline).to be_a(Float) expect(subject.request_deadline).to be_a(Float)
end end
...@@ -22,6 +24,18 @@ describe Gitlab::RequestContext, :request_store do ...@@ -22,6 +24,18 @@ describe Gitlab::RequestContext, :request_store do
expect(subject.request_deadline).to be_nil expect(subject.request_deadline).to be_nil
end end
it 'only checks the feature once per request-instance' do
expect(Feature).to receive(:enabled?).with(:request_deadline).once
2.times { subject.request_deadline }
end
it 'returns nil when the feature is disabled' do
stub_feature_flags(request_deadline: false)
expect(subject.request_deadline).to be_nil
end
end end
describe '#ensure_request_deadline_not_exceeded!' do describe '#ensure_request_deadline_not_exceeded!' do
......
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