diff options
Diffstat (limited to 'app')
| -rw-r--r-- | app/assets/javascripts/lib/graphql.js | 3 | ||||
| -rw-r--r-- | app/assets/stylesheets/pages/experimental_separate_sign_up.scss | 1 | ||||
| -rw-r--r-- | app/models/ci/build.rb | 36 | ||||
| -rw-r--r-- | app/models/commit_status_enums.rb | 4 | ||||
| -rw-r--r-- | app/presenters/commit_status_presenter.rb | 6 | ||||
| -rw-r--r-- | app/services/ci/register_job_service.rb | 81 | ||||
| -rw-r--r-- | app/views/registrations/welcome.html.haml | 2 |
7 files changed, 99 insertions, 34 deletions
diff --git a/app/assets/javascripts/lib/graphql.js b/app/assets/javascripts/lib/graphql.js index c05db4a5c71..ca797cde913 100644 --- a/app/assets/javascripts/lib/graphql.js +++ b/app/assets/javascripts/lib/graphql.js @@ -26,7 +26,8 @@ export default (resolvers = {}, config = {}) => { createUploadLink(httpOptions), new BatchHttpLink(httpOptions), ), - cache: new InMemoryCache(config.cacheConfig), + cache: new InMemoryCache({ ...config.cacheConfig, freezeResults: true }), resolvers, + assumeImmutableResults: true, }); }; diff --git a/app/assets/stylesheets/pages/experimental_separate_sign_up.scss b/app/assets/stylesheets/pages/experimental_separate_sign_up.scss index 8b1ec1ced35..53dfdd10788 100644 --- a/app/assets/stylesheets/pages/experimental_separate_sign_up.scss +++ b/app/assets/stylesheets/pages/experimental_separate_sign_up.scss @@ -23,6 +23,7 @@ .signup-heading h2 { font-weight: $gl-font-weight-bold; + padding: 0 10px; @include media-breakpoint-down(md) { font-size: $gl-font-size-large; diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 4089fcf7097..62815589c3c 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -35,6 +35,10 @@ module Ci refspecs: -> (build) { build.merge_request_ref? } }.freeze + DEFAULT_RETRIES = { + scheduler_failure: 2 + }.freeze + has_one :deployment, as: :deployable, class_name: 'Deployment' has_many :trace_sections, class_name: 'Ci::BuildTraceSection' has_many :trace_chunks, class_name: 'Ci::BuildTraceChunk', foreign_key: :build_id @@ -372,18 +376,25 @@ module Ci pipeline.builds.retried.where(name: self.name).count end - def retries_max - normalized_retry.fetch(:max, 0) + def retry_failure? + max_allowed_retries = nil + max_allowed_retries ||= options_retry_max if retry_on_reason_or_always? + max_allowed_retries ||= DEFAULT_RETRIES.fetch(failure_reason.to_sym, 0) + + max_allowed_retries > 0 && retries_count < max_allowed_retries end - def retry_when - normalized_retry.fetch(:when, ['always']) + def options_retry_max + options_retry[:max] end - def retry_failure? - return false if retries_max.zero? || retries_count >= retries_max + def options_retry_when + options_retry.fetch(:when, ['always']) + end - retry_when.include?('always') || retry_when.include?(failure_reason.to_s) + def retry_on_reason_or_always? + options_retry_when.include?(failure_reason.to_s) || + options_retry_when.include?('always') end def latest? @@ -831,6 +842,13 @@ module Ci :creating end + # Consider this object to have a structural integrity problems + def doom! + update_columns( + status: :failed, + failure_reason: :data_integrity_failure) + end + private def successful_deployment_status @@ -875,8 +893,8 @@ module Ci # format, but builds created before GitLab 11.5 and saved in database still # have the old integer only format. This method returns the retry option # normalized as a hash in 11.5+ format. - def normalized_retry - strong_memoize(:normalized_retry) do + def options_retry + strong_memoize(:options_retry) do value = options&.dig(:retry) value = value.is_a?(Integer) ? { max: value } : value.to_h value.with_indifferent_access diff --git a/app/models/commit_status_enums.rb b/app/models/commit_status_enums.rb index a540e291990..2ca6d15e642 100644 --- a/app/models/commit_status_enums.rb +++ b/app/models/commit_status_enums.rb @@ -15,7 +15,9 @@ module CommitStatusEnums stale_schedule: 7, job_execution_timeout: 8, archived_failure: 9, - unmet_prerequisites: 10 + unmet_prerequisites: 10, + scheduler_failure: 11, + data_integrity_failure: 12 } end end diff --git a/app/presenters/commit_status_presenter.rb b/app/presenters/commit_status_presenter.rb index f1182ec26f4..66ae840a619 100644 --- a/app/presenters/commit_status_presenter.rb +++ b/app/presenters/commit_status_presenter.rb @@ -11,7 +11,9 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated stale_schedule: 'Delayed job could not be executed by some reason, please try again', job_execution_timeout: 'The script exceeded the maximum execution time set for the job', archived_failure: 'The job is archived and cannot be run', - unmet_prerequisites: 'The job failed to complete prerequisite tasks' + unmet_prerequisites: 'The job failed to complete prerequisite tasks', + scheduler_failure: 'The scheduler failed to assign job to the runner, please try again or contact system administrator', + data_integrity_failure: 'There has been a structural integrity problem detected, please contact system administrator' }.freeze private_constant :CALLOUT_FAILURE_MESSAGES @@ -33,6 +35,6 @@ class CommitStatusPresenter < Gitlab::View::Presenter::Delegated end def unrecoverable? - script_failure? || missing_dependency_failure? || archived_failure? + script_failure? || missing_dependency_failure? || archived_failure? || scheduler_failure? || data_integrity_failure? end end diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index d8f32ff88ce..ff140444c1c 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -42,26 +42,16 @@ module Ci end builds.each do |build| - next unless runner.can_pick?(build) - - begin - # In case when 2 runners try to assign the same build, second runner will be declined - # with StateMachines::InvalidTransition or StaleObjectError when doing run! or save method. - if assign_runner!(build, params) - register_success(build) - - return Result.new(build, true) - end - rescue StateMachines::InvalidTransition, ActiveRecord::StaleObjectError - # We are looping to find another build that is not conflicting - # It also indicates that this build can be picked and passed to runner. - # If we don't do it, basically a bunch of runners would be competing for a build - # and thus we will generate a lot of 409. This will increase - # the number of generated requests, also will reduce significantly - # how many builds can be picked by runner in a unit of time. - # In case we hit the concurrency-access lock, - # we still have to return 409 in the end, - # to make sure that this is properly handled by runner. + result = process_build(build, params) + next unless result + + if result.valid? + register_success(result.build) + + return result + else + # The usage of valid: is described in + # handling of ActiveRecord::StaleObjectError valid = false end end @@ -73,6 +63,35 @@ module Ci private + def process_build(build, params) + return unless runner.can_pick?(build) + + # In case when 2 runners try to assign the same build, second runner will be declined + # with StateMachines::InvalidTransition or StaleObjectError when doing run! or save method. + if assign_runner!(build, params) + Result.new(build, true) + end + rescue StateMachines::InvalidTransition, ActiveRecord::StaleObjectError + # We are looping to find another build that is not conflicting + # It also indicates that this build can be picked and passed to runner. + # If we don't do it, basically a bunch of runners would be competing for a build + # and thus we will generate a lot of 409. This will increase + # the number of generated requests, also will reduce significantly + # how many builds can be picked by runner in a unit of time. + # In case we hit the concurrency-access lock, + # we still have to return 409 in the end, + # to make sure that this is properly handled by runner. + Result.new(nil, false) + rescue => ex + raise ex unless Feature.enabled?(:ci_doom_build, default_enabled: true) + + scheduler_failure!(build) + track_exception_for_build(ex, build) + + # skip, and move to next one + nil + end + def assign_runner!(build, params) build.runner_id = runner.id build.runner_session_attributes = params[:session] if params[:session].present? @@ -96,6 +115,28 @@ module Ci true end + def scheduler_failure!(build) + Gitlab::OptimisticLocking.retry_lock(build, 3) do |subject| + subject.drop!(:scheduler_failure) + end + rescue => ex + build.doom! + + # This requires extra exception, otherwise we would loose information + # why we cannot perform `scheduler_failure` + track_exception_for_build(ex, build) + end + + def track_exception_for_build(ex, build) + Gitlab::Sentry.track_acceptable_exception(ex, extra: { + build_id: build.id, + build_name: build.name, + build_stage: build.stage, + pipeline_id: build.pipeline_id, + project_id: build.project_id + }) + end + # rubocop: disable CodeReuse/ActiveRecord def builds_for_shared_runner new_builds. diff --git a/app/views/registrations/welcome.html.haml b/app/views/registrations/welcome.html.haml index 02ab974ecc0..76c4a935584 100644 --- a/app/views/registrations/welcome.html.haml +++ b/app/views/registrations/welcome.html.haml @@ -1,4 +1,4 @@ -- content_for(:page_title, _('Welcome to GitLab<br>%{username}!' % { username: html_escape(current_user.username) }).html_safe) +- content_for(:page_title, _('Welcome to GitLab %{username}!') % { username: current_user.username }) - max_name_length = 128 .text-center.mb-3 = _('In order to tailor your experience with GitLab<br>we would like to know a bit more about you.').html_safe |