Merge branch 'zj-circuit-breaker-removal' into 'master'

Remove Git circuit breaker

Closes #45405

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

10 files changed, 11 insertions, 144 deletions

diff --git a/app/controllers/admin/health_check_controller.rb b/app/controllers/admin/health_check_controller.rb
index 44864f9c7d0..25cc241e5b0 100644
--- a/app/controllers/admin/health_check_controller.rb
+++ b/app/controllers/admin/health_check_controller.rb
@@ -3,12 +3,5 @@
 class Admin::HealthCheckController < Admin::ApplicationController
   def show
     @errors = HealthCheck::Utils.process_checks(['standard'])
-    @failing_storage_statuses = Gitlab::Git::Storage::Health.for_failing_storages
-  end
-
-  def reset_storage_health
-    Gitlab::Git::Storage::FailureInfo.reset_all!
-    redirect_to admin_health_check_path,
-                notice: _('Git storage health information has been reset')
   end
 end
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index ec45e2813c5..bbeaeb7694e 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -66,7 +66,7 @@ class ApplicationController < ActionController::Base
     head :forbidden, retry_after: Gitlab::Auth::UniqueIpsLimiter.config.unique_ips_limit_time_window
   end
 
-  rescue_from Gitlab::Git::Storage::Inaccessible, GRPC::Unavailable, Gitlab::Git::CommandError do |exception|
+  rescue_from GRPC::Unavailable, Gitlab::Git::CommandError do |exception|
     log_exception(exception)
 
     headers['Retry-After'] = exception.retry_after if exception.respond_to?(:retry_after)
diff --git a/app/controllers/health_controller.rb b/app/controllers/health_controller.rb
index ab4bc911e17..dc9a52f8da5 100644
--- a/app/controllers/health_controller.rb
+++ b/app/controllers/health_controller.rb
@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 class HealthController < ActionController::Base
-  protect_from_forgery with: :exception, except: :storage_check, prepend: true
+  protect_from_forgery with: :exception, prepend: true
   include RequiresWhitelistedMonitoringClient
 
   CHECKS = [
@@ -25,15 +25,6 @@ class HealthController < ActionController::Base
     render_check_results(results)
   end
 
-  def storage_check
-    results = Gitlab::Git::Storage::Checker.check_all
-
-    render json: {
-             check_interval: Gitlab::CurrentSettings.current_application_settings.circuitbreaker_check_interval,
-             results: results
-           }
-  end
-
   private
 
   def render_check_results(results)
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb
index 15cbfeea609..d6753e46165 100644
--- a/app/helpers/application_settings_helper.rb
+++ b/app/helpers/application_settings_helper.rb
@@ -108,37 +108,6 @@ module ApplicationSettingsHelper
     options_for_select(options, selected)
   end
 
-  def circuitbreaker_failure_count_help_text
-    health_link = link_to(s_('AdminHealthPageLink|health page'), admin_health_check_path)
-    api_link = link_to(s_('CircuitBreakerApiLink|circuitbreaker api'), help_page_path("api/repository_storage_health"))
-    message = _("The number of failures of after which GitLab will completely "\
-                "prevent access to the storage. The number of failures can be "\
-                "reset in the admin interface: %{link_to_health_page} or using "\
-                "the %{api_documentation_link}.")
-    message = message % { link_to_health_page: health_link, api_documentation_link: api_link }
-
-    message.html_safe
-  end
-
-  def circuitbreaker_access_retries_help_text
-    _('The number of attempts GitLab will make to access a storage.')
-  end
-
-  def circuitbreaker_failure_reset_time_help_text
-    _("The time in seconds GitLab will keep failure information. When no "\
-      "failures occur during this time, information about the mount is reset.")
-  end
-
-  def circuitbreaker_storage_timeout_help_text
-    _("The time in seconds GitLab will try to access storage. After this time a "\
-      "timeout error will be raised.")
-  end
-
-  def circuitbreaker_check_interval_help_text
-    _("The time in seconds between storage checks. When a previous check did "\
-      "complete yet, GitLab will skip a check.")
-  end
-
   def visible_attributes
     [
       :admin_notification_email,
@@ -150,11 +119,6 @@ module ApplicationSettingsHelper
       :authorized_keys_enabled,
       :auto_devops_enabled,
       :auto_devops_domain,
-      :circuitbreaker_access_retries,
-      :circuitbreaker_check_interval,
-      :circuitbreaker_failure_count_threshold,
-      :circuitbreaker_failure_reset_time,
-      :circuitbreaker_storage_timeout,
       :clientside_sentry_dsn,
       :clientside_sentry_enabled,
       :container_registry_token_expire_delay,
diff --git a/app/helpers/storage_health_helper.rb b/app/helpers/storage_health_helper.rb
deleted file mode 100644
index 182e8e6641b..00000000000
--- a/app/helpers/storage_health_helper.rb
+++ /dev/null
@@ -1,34 +0,0 @@
-# frozen_string_literal: true
-
-module StorageHealthHelper
-  def failing_storage_health_message(storage_health)
-    storage_name = content_tag(:strong, h(storage_health.storage_name))
-    host_names = h(storage_health.failing_on_hosts.to_sentence)
-    translation_params = { storage_name: storage_name,
-                           host_names: host_names,
-                           failed_attempts: storage_health.total_failures }
-
-    translation = n_('%{storage_name}: failed storage access attempt on host:',
-                     '%{storage_name}: %{failed_attempts} failed storage access attempts:',
-                     storage_health.total_failures) % translation_params
-
-    translation.html_safe
-  end
-
-  def message_for_circuit_breaker(circuit_breaker)
-    maximum_failures = circuit_breaker.failure_count_threshold
-    current_failures = circuit_breaker.failure_count
-
-    translation_params = { number_of_failures: current_failures,
-                           maximum_failures: maximum_failures }
-
-    if circuit_breaker.circuit_broken?
-      s_("%{number_of_failures} of %{maximum_failures} failures. GitLab will not "\
-         "retry automatically. Reset storage information when the problem is "\
-         "resolved.") % translation_params
-    else
-      _("%{number_of_failures} of %{maximum_failures} failures. GitLab will "\
-        "allow access on the next attempt.") % translation_params
-    end
-  end
-end
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 65a2f760f93..23131af1b7d 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -4,6 +4,7 @@ class ApplicationSetting < ActiveRecord::Base
   include CacheableAttributes
   include CacheMarkdownField
   include TokenAuthenticatable
+  include IgnorableColumn
 
   add_authentication_token_field :runners_registration_token
   add_authentication_token_field :health_check_access_token
@@ -27,6 +28,12 @@ class ApplicationSetting < ActiveRecord::Base
   serialize :domain_blacklist, Array # rubocop:disable Cop/ActiveRecordSerialize
   serialize :repository_storages # rubocop:disable Cop/ActiveRecordSerialize
 
+  ignore_column :circuitbreaker_failure_count_threshold
+  ignore_column :circuitbreaker_failure_reset_time
+  ignore_column :circuitbreaker_storage_timeout
+  ignore_column :circuitbreaker_access_retries
+  ignore_column :circuitbreaker_check_interval
+
   cache_markdown_field :sign_in_text
   cache_markdown_field :help_page_text
   cache_markdown_field :shared_runners_text, pipeline: :plain_markdown
@@ -150,17 +157,6 @@ class ApplicationSetting < ActiveRecord::Base
             presence: true,
             numericality: { greater_than_or_equal_to: 0 }
 
-  validates :circuitbreaker_failure_count_threshold,
-            :circuitbreaker_failure_reset_time,
-            :circuitbreaker_storage_timeout,
-            :circuitbreaker_check_interval,
-            presence: true,
-            numericality: { only_integer: true, greater_than_or_equal_to: 0 }
-
-  validates :circuitbreaker_access_retries,
-            presence: true,
-            numericality: { only_integer: true, greater_than_or_equal_to: 1 }
-
   validates :gitaly_timeout_default,
             presence: true,
             numericality: { only_integer: true, greater_than_or_equal_to: 0 }
diff --git a/app/views/admin/application_settings/_repository_storage.html.haml b/app/views/admin/application_settings/_repository_storage.html.haml
index 908b30cc3ce..c6c29ed1f21 100644
--- a/app/views/admin/application_settings/_repository_storage.html.haml
+++ b/app/views/admin/application_settings/_repository_storage.html.haml
@@ -20,32 +20,5 @@
           Manage repository storage paths. Learn more in the
           = succeed "." do
             = link_to "repository storages documentation", help_page_path("administration/repository_storage_paths")
-    .sub-section
-      %h4 Circuit breaker
-      .form-group
-        = f.label :circuitbreaker_check_interval, _('Check interval'), class: 'label-bold'
-        = f.number_field :circuitbreaker_check_interval, class: 'form-control'
-        .form-text.text-muted
-          = circuitbreaker_check_interval_help_text
-      .form-group
-        = f.label :circuitbreaker_access_retries, _('Number of access attempts'), class: 'label-bold'
-        = f.number_field :circuitbreaker_access_retries, class: 'form-control'
-        .form-text.text-muted
-          = circuitbreaker_access_retries_help_text
-      .form-group
-        = f.label :circuitbreaker_storage_timeout, _('Seconds to wait for a storage access attempt'), class: 'label-bold'
-        = f.number_field :circuitbreaker_storage_timeout, class: 'form-control'
-        .form-text.text-muted
-          = circuitbreaker_storage_timeout_help_text
-      .form-group
-        = f.label :circuitbreaker_failure_count_threshold, _('Maximum git storage failures'), class: 'label-bold'
-        = f.number_field :circuitbreaker_failure_count_threshold, class: 'form-control'
-        .form-text.text-muted
-          = circuitbreaker_failure_count_help_text
-      .form-group
-        = f.label :circuitbreaker_failure_reset_time, _('Seconds before reseting failure information'), class: 'label-bold'
-        = f.number_field :circuitbreaker_failure_reset_time, class: 'form-control'
-        .form-text.text-muted
-          = circuitbreaker_failure_reset_time_help_text
 
   = f.submit 'Save changes', class: "btn btn-success qa-save-changes-button"
diff --git a/app/views/admin/application_settings/repository.html.haml b/app/views/admin/application_settings/repository.html.haml
index be13138a764..b50a0dd5a18 100644
--- a/app/views/admin/application_settings/repository.html.haml
+++ b/app/views/admin/application_settings/repository.html.haml
@@ -20,7 +20,7 @@
     %button.btn.btn-default.js-settings-toggle{ type: 'button' }
       = expanded_by_default? ? _('Collapse') : _('Expand')
     %p
-      = _('Configure storage path and circuit breaker settings.')
+      = _('Configure storage path settings.')
   .settings-content
     = render 'repository_storage'
 
diff --git a/app/views/admin/health_check/_failing_storages.html.haml b/app/views/admin/health_check/_failing_storages.html.haml
deleted file mode 100644
index 6830201538d..00000000000
--- a/app/views/admin/health_check/_failing_storages.html.haml
+++ /dev/null
@@ -1,15 +0,0 @@
-- if failing_storages.any?
-  = _('There are problems accessing Git storage: ')
-  %ul
-    - failing_storages.each do |storage_health|
-      %li
-        = failing_storage_health_message(storage_health)
-        %ul
-          - storage_health.failing_circuit_breakers.each do |circuit_breaker|
-            %li
-              #{circuit_breaker.hostname}: #{message_for_circuit_breaker(circuit_breaker)}
-
-  = _("Access to failing storages has been temporarily disabled to allow the mount to recover. Reset storage information after the issue has been resolved to allow access again.")
-  .prepend-top-10
-    = button_to _("Reset git storage health information"), reset_storage_health_admin_health_check_path,
-      method: :post, class: 'btn btn-default'
diff --git a/app/views/admin/health_check/show.html.haml b/app/views/admin/health_check/show.html.haml
index d51ac854b04..0f5e97e288a 100644
--- a/app/views/admin/health_check/show.html.haml
+++ b/app/views/admin/health_check/show.html.haml
@@ -1,6 +1,6 @@
 - @no_container = true
 - page_title _('Health Check')
-- no_errors = @errors.blank? && @failing_storage_statuses.blank?
+- no_errors = @errors.blank?
 
 %div{ class: container_class }
   %h3.page-title= page_title
@@ -39,4 +39,3 @@
         #{ s_('HealthCheck|No Health Problems Detected') }
       - else
         = @errors
-        = render partial: 'failing_storages', object: @failing_storage_statuses