From 4be77d0b057e3b26f48fbaee947bf8f91a755f41 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 8 May 2016 19:09:33 -0700 Subject: [PATCH] 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 --- CHANGELOG | 1 + app/models/project.rb | 2 +- lib/gitlab/git_access.rb | 7 ++++++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index e85d3c88b2..822fa4be56 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 dfd1e54ecf..5b556a2275 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 6cb4123987..d2a0e316cb 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