summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2017-11-20 15:27:52 -0800
committerStan Hu <stanhu@gmail.com>2017-11-20 15:27:52 -0800
commit3c52e2f06ef3234ab5ace532e21e194abab96b59 (patch)
tree7e637f058ab8155a3066ef7750031c28793d94ee
parent2b594daf9aa10f9a6469addb52f0df283f5347fb (diff)
downloadgitlab-ce-3c52e2f06ef3234ab5ace532e21e194abab96b59.tar.gz
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
-rw-r--r--lib/gitlab/middleware/read_only.rb6
-rw-r--r--spec/lib/gitlab/middleware/read_only_spec.rb5
2 files changed, 11 insertions, 0 deletions
diff --git a/lib/gitlab/middleware/read_only.rb b/lib/gitlab/middleware/read_only.rb
index 5e4932e4e57..dc77f737da8 100644
--- a/lib/gitlab/middleware/read_only.rb
+++ b/lib/gitlab/middleware/read_only.rb
@@ -74,10 +74,16 @@ module Gitlab
end
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'
end
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'
end
end
diff --git a/spec/lib/gitlab/middleware/read_only_spec.rb b/spec/lib/gitlab/middleware/read_only_spec.rb
index b14735943a5..044b93f0120 100644
--- a/spec/lib/gitlab/middleware/read_only_spec.rb
+++ b/spec/lib/gitlab/middleware/read_only_spec.rb
@@ -84,6 +84,7 @@ describe Gitlab::Middleware::ReadOnly do
end
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')
expect(response).to be_a_redirect
@@ -92,6 +93,8 @@ describe Gitlab::Middleware::ReadOnly do
context 'whitelisted requests' 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")
expect(response).not_to be_a_redirect
@@ -99,6 +102,7 @@ describe Gitlab::Middleware::ReadOnly do
end
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')
expect(response).not_to be_a_redirect
@@ -106,6 +110,7 @@ describe Gitlab::Middleware::ReadOnly do
end
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')
expect(response).not_to be_a_redirect