summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2016-05-08 19:09:33 -0700
committerStan Hu <stanhu@gmail.com>2016-05-09 01:17:14 -0700
commit4be77d0b057e3b26f48fbaee947bf8f91a755f41 (patch)
tree3377a3b9a9a05cd45827259267d7cf87eb352a27
parent4bc4f06512620271a8d454b966e7f5c288a68829 (diff)
downloadgitlab-ce-4be77d0b057e3b26f48fbaee947bf8f91a755f41.tar.gz
Improve multiple branch push performance by memoizing permission checking
If you attempt to push thousands of branches at once, the 60-second timeout will occur because GitAccess checking does a lot of work to check if the user has permission to push to a branch. This changes does two things: 1. Instead of making 1 DB query per branch push, use a memoized list of protected branches to check 2. Memoize what permissions the user has to perform on this project On a test of 10,000 branch pushes, this prevents gitlab-shell from hitting the 60-second timeout. Closes #17225
-rw-r--r--CHANGELOG1
-rw-r--r--app/models/project.rb2
-rw-r--r--lib/gitlab/git_access.rb7
3 files changed, 8 insertions, 2 deletions
diff --git a/CHANGELOG b/CHANGELOG
index e85d3c88b28..822fa4be565 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -4,6 +4,7 @@ v 8.8.0 (unreleased)
- Assign labels and milestone to target project when moving issue. !3934 (Long Nguyen)
- Project#open_branches has been cleaned up and no longer loads entire records into memory.
- Escape HTML in commit titles in system note messages
+ - Improve multiple branch push performance by memoizing permission checking
- Log to application.log when an admin starts and stops impersonating a user
- Updated gitlab_git to 10.1.0
- GitAccess#protected_tag? no longer loads all tags just to check if a single one exists
diff --git a/app/models/project.rb b/app/models/project.rb
index dfd1e54ecf7..5b556a22755 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -767,7 +767,7 @@ class Project < ActiveRecord::Base
# Check if current branch name is marked as protected in the system
def protected_branch?(branch_name)
- protected_branches.where(name: branch_name).any?
+ protected_branch_names.include?(branch_name)
end
def developers_can_push_to_protected_branch?(branch_name)
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index 6cb41239871..d2a0e316cbe 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -122,6 +122,11 @@ module Gitlab
build_status_object(true)
end
+ def can_user_do_action?(action)
+ @permission_cache ||= {}
+ @permission_cache[action] ||= user.can?(action, project)
+ end
+
def change_access_check(change)
oldrev, newrev, ref = change.split(' ')
@@ -135,7 +140,7 @@ module Gitlab
:push_code
end
- unless user.can?(action, project)
+ unless can_user_do_action?(action)
status =
case action
when :force_push_code_to_protected_branches