Merge branch '37824-many-branches-lock-server' into 'master'

Project with many branches can lock server running "git branch --contains XXX"

Closes #37824

See merge request gitlab-org/gitlab-ce!14812

6 files changed, 81 insertions, 24 deletions

diff --git a/app/assets/stylesheets/framework/wells.scss b/app/assets/stylesheets/framework/wells.scss
index 5f9756bf58a..68824ff8418 100644
--- a/app/assets/stylesheets/framework/wells.scss
+++ b/app/assets/stylesheets/framework/wells.scss
@@ -52,6 +52,37 @@
   .label.label-gray {
     background-color: $well-expand-item;
   }
+
+  .branches {
+    display: inline;
+  }
+
+  .branch-link {
+    margin-bottom: 2px;
+  }
+
+  .limit-box {
+    cursor: pointer;
+    display: inline-flex;
+    align-items: center;
+    background-color: $red-100;
+    border-radius: $border-radius-default;
+    text-align: center;
+
+    &:hover {
+      background-color: $red-200;
+    }
+
+    .limit-icon {
+      margin: 0 8px;
+    }
+
+    .limit-message {
+      line-height: 16px;
+      margin-right: 8px;
+      font-size: 12px;
+    }
+  }
 }
 
 .light-well {
diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index a62f05db7db..494d412b532 100644
--- a/app/controllers/projects/commit_controller.rb
+++ b/app/controllers/projects/commit_controller.rb
@@ -16,6 +16,8 @@ class Projects::CommitController < Projects::ApplicationController
   before_action :define_note_vars, only: [:show, :diff_for_path]
   before_action :authorize_edit_tree!, only: [:revert, :cherry_pick]
 
+  BRANCH_SEARCH_LIMIT = 1000
+
   def show
     apply_diff_view_cookie!
 
@@ -56,8 +58,14 @@ class Projects::CommitController < Projects::ApplicationController
   end
 
   def branches
-    @branches = @project.repository.branch_names_contains(commit.id)
-    @tags = @project.repository.tag_names_contains(commit.id)
+    # branch_names_contains/tag_names_contains can take a long time when there are thousands of
+    # branches/tags - each `git branch --contains xxx` request can consume a cpu core.
+    # so only do the query when there are a manageable number of branches/tags
+    @branches_limit_exceeded = @project.repository.branch_count > BRANCH_SEARCH_LIMIT
+    @branches = @branches_limit_exceeded ? [] : @project.repository.branch_names_contains(commit.id)
+
+    @tags_limit_exceeded = @project.repository.tag_count > BRANCH_SEARCH_LIMIT
+    @tags = @tags_limit_exceeded ? [] : @project.repository.tag_names_contains(commit.id)
     render layout: false
   end
 
diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb
index ef22cafc2e2..f9a666fa1e6 100644
--- a/app/helpers/commits_helper.rb
+++ b/app/helpers/commits_helper.rb
@@ -60,23 +60,33 @@ module CommitsHelper
     branches.include?(project.default_branch) ? branches.delete(project.default_branch) : branches.pop
   end
 
+  # Returns a link formatted as a commit branch link
+  def commit_branch_link(url, text)
+    link_to(url, class: 'label label-gray ref-name branch-link') do
+      icon('code-fork') + " #{text}"
+    end
+  end
+
   # Returns the sorted alphabetically links to branches, separated by a comma
   def commit_branches_links(project, branches)
     branches.sort.map do |branch|
-      link_to(project_ref_path(project, branch), class: "label label-gray ref-name") do
-        icon('code-fork') + " #{branch}"
-      end
-    end.join(" ").html_safe
+      commit_branch_link(project_ref_path(project, branch), branch)
+    end.join(' ').html_safe
+  end
+
+  # Returns a link formatted as a commit tag link
+  def commit_tag_link(url, text)
+    link_to(url, class: 'label label-gray ref-name') do
+      icon('tag') + " #{text}"
+    end
   end
 
   # Returns the sorted links to tags, separated by a comma
   def commit_tags_links(project, tags)
     sorted = VersionSorter.rsort(tags)
     sorted.map do |tag|
-      link_to(project_ref_path(project, tag), class: "label label-gray ref-name") do
-        icon('tag') + " #{tag}"
-      end
-    end.join(" ").html_safe
+      commit_tag_link(project_ref_path(project, tag), tag)
+    end.join(' ').html_safe
   end
 
   def link_to_browse_code(project, commit)
diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml
index 3f4415f9e5e..8b9c1bbb602 100644
--- a/app/views/projects/commit/_commit_box.html.haml
+++ b/app/views/projects/commit/_commit_box.html.haml
@@ -61,7 +61,7 @@
     %span.cgray= n_('parent', 'parents', @commit.parents.count)
     - @commit.parents.each do |parent|
       = link_to parent.short_id, project_commit_path(@project, parent), class: "commit-sha"
-    %span.commit-info.branches
+    .commit-info.branches
       %i.fa.fa-spinner.fa-spin
 
   - if @commit.last_pipeline
diff --git a/app/views/projects/commit/_limit_exceeded_message.html.haml b/app/views/projects/commit/_limit_exceeded_message.html.haml
new file mode 100644
index 00000000000..84a52d49487
--- /dev/null
+++ b/app/views/projects/commit/_limit_exceeded_message.html.haml
@@ -0,0 +1,8 @@
+.has-tooltip{ class: "limit-box limit-box-#{objects} prepend-left-5", data: { title: "Project has too many #{label_for_message} to search"} }
+  .limit-icon
+    - if objects == :branch
+      = icon('code-fork')
+    - else
+      = icon('tag')
+  .limit-message
+    %span #{label_for_message.capitalize} unavailable
diff --git a/app/views/projects/commit/branches.html.haml b/app/views/projects/commit/branches.html.haml
index 911c9ddce06..8611129b356 100644
--- a/app/views/projects/commit/branches.html.haml
+++ b/app/views/projects/commit/branches.html.haml
@@ -1,15 +1,15 @@
-- if @branches.any? || @tags.any?
+- if @branches_limit_exceeded
+  = render 'limit_exceeded_message', objects: :branch, label_for_message: "branches"
+- elsif @branches.any?
   - branch = commit_default_branch(@project, @branches)
-  = link_to(project_ref_path(@project, branch), class: "label label-gray ref-name") do
-    = icon('code-fork')
-    = branch
+  = commit_branch_link(project_ref_path(@project, branch), branch)
 
-  -# `commit_default_branch` deletes the default branch from `@branches`,
-  -# so only render this if we have more branches left
-  - if @branches.any? || @tags.any?
-    %span
-      = link_to "…", "#", class: "js-details-expand label label-gray"
-
-      %span.js-details-content.hide
-        = commit_branches_links(@project, @branches) if @branches.any?
-        = commit_tags_links(@project, @tags) if @tags.any?
+- if @branches.any? || @tags.any? || @tags_limit_exceeded
+  %span
+    = link_to "…", "#", class: "js-details-expand label label-gray"
+    %span.js-details-content.hide
+      = commit_branches_links(@project, @branches)
+      - if @tags_limit_exceeded
+        = render 'limit_exceeded_message', objects: :tag, label_for_message: "tags"
+      - else
+        = commit_tags_links(@project, @tags)