Commit 3c52e2f0 authored by Stan Hu's avatar Stan Hu

Optimize read-only middleware so that it does not consume as much CPU

In !15082, we changed the behavior of the middleware to call
`Rails.application.routes.recognize_path` whenever a new route arrived.
However, this can be a CPU-intensive task because Rails needs to allocate
memory and compile 850+ different regular expressions, which are complicated
in GitLab.

As a short-term fix, we can do a lightweight string match before
we do the heavier comparison.

Closes #40185, gitlab-com/infrastructure#3240
parent 2b594daf
...@@ -74,10 +74,16 @@ module Gitlab ...@@ -74,10 +74,16 @@ module Gitlab
end end
def grack_route def grack_route
# Calling route_hash may be expensive. Only do it if we think there's a possible match
return false unless request.path.end_with?('.git/git-upload-pack')
route_hash[:controller] == 'projects/git_http' && route_hash[:action] == 'git_upload_pack' route_hash[:controller] == 'projects/git_http' && route_hash[:action] == 'git_upload_pack'
end end
def lfs_route def lfs_route
# Calling route_hash may be expensive. Only do it if we think there's a possible match
return false unless request.path.end_with?('/info/lfs/objects/batch')
route_hash[:controller] == 'projects/lfs_api' && route_hash[:action] == 'batch' route_hash[:controller] == 'projects/lfs_api' && route_hash[:action] == 'batch'
end end
end end
......
...@@ -84,6 +84,7 @@ describe Gitlab::Middleware::ReadOnly do ...@@ -84,6 +84,7 @@ describe Gitlab::Middleware::ReadOnly do
end end
it 'expects POST of new file that looks like an LFS batch url to be disallowed' do it 'expects POST of new file that looks like an LFS batch url to be disallowed' do
expect(Rails.application.routes).to receive(:recognize_path).and_call_original
response = request.post('/root/gitlab-ce/new/master/app/info/lfs/objects/batch') response = request.post('/root/gitlab-ce/new/master/app/info/lfs/objects/batch')
expect(response).to be_a_redirect expect(response).to be_a_redirect
...@@ -92,6 +93,8 @@ describe Gitlab::Middleware::ReadOnly do ...@@ -92,6 +93,8 @@ describe Gitlab::Middleware::ReadOnly do
context 'whitelisted requests' do context 'whitelisted requests' do
it 'expects a POST internal request to be allowed' do it 'expects a POST internal request to be allowed' do
expect(Rails.application.routes).not_to receive(:recognize_path)
response = request.post("/api/#{API::API.version}/internal") response = request.post("/api/#{API::API.version}/internal")
expect(response).not_to be_a_redirect expect(response).not_to be_a_redirect
...@@ -99,6 +102,7 @@ describe Gitlab::Middleware::ReadOnly do ...@@ -99,6 +102,7 @@ describe Gitlab::Middleware::ReadOnly do
end end
it 'expects a POST LFS request to batch URL to be allowed' do it 'expects a POST LFS request to batch URL to be allowed' do
expect(Rails.application.routes).to receive(:recognize_path).and_call_original
response = request.post('/root/rouge.git/info/lfs/objects/batch') response = request.post('/root/rouge.git/info/lfs/objects/batch')
expect(response).not_to be_a_redirect expect(response).not_to be_a_redirect
...@@ -106,6 +110,7 @@ describe Gitlab::Middleware::ReadOnly do ...@@ -106,6 +110,7 @@ describe Gitlab::Middleware::ReadOnly do
end end
it 'expects a POST request to git-upload-pack URL to be allowed' do it 'expects a POST request to git-upload-pack URL to be allowed' do
expect(Rails.application.routes).to receive(:recognize_path).and_call_original
response = request.post('/root/rouge.git/git-upload-pack') response = request.post('/root/rouge.git/git-upload-pack')
expect(response).not_to be_a_redirect expect(response).not_to be_a_redirect
......
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