From 830198b75c1fc75791187455f7976f0a87493ef7 Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Tue, 21 Mar 2017 13:58:31 +0100 Subject: Improve resiliency of build picking Every time we pick a new build, or we get into conflict we force runner to retry the operation to make sure that all builds are being picked. It makes us to loose one request, but also makes sure that we are consistent. It is fair trade, as number of generated builds is relatively small 0.1% compared to the traffic. --- app/models/ci/runner_project.rb | 4 ++++ lib/api/helpers/runner.rb | 8 ++++++++ lib/api/runner.rb | 27 ++++++++++++++++++++------- lib/ci/api/builds.rb | 29 +++++++++++++++++++---------- lib/ci/api/helpers.rb | 8 ++++++++ 5 files changed, 59 insertions(+), 17 deletions(-) diff --git a/app/models/ci/runner_project.rb b/app/models/ci/runner_project.rb index 5f01a0daae9..0267040ba0f 100644 --- a/app/models/ci/runner_project.rb +++ b/app/models/ci/runner_project.rb @@ -6,5 +6,9 @@ module Ci belongs_to :project validates :runner_id, uniqueness: { scope: :project_id } + + after_save :tick_runner_queue + + delegate :tick_runner_queue, to: :runner, allow_nil: true end end diff --git a/lib/api/helpers/runner.rb b/lib/api/helpers/runner.rb index 74848a6e144..265be59d36b 100644 --- a/lib/api/helpers/runner.rb +++ b/lib/api/helpers/runner.rb @@ -15,6 +15,14 @@ module API attributes_for_keys(%w(name version revision platform architecture), params['info']) end + def get_runner_version + params.fetch('info', {}).fetch('version', "unknown") + end + + def header_last_update(value) + header 'X-GitLab-Last-Update', value + end + def authenticate_runner! forbidden! unless current_runner end diff --git a/lib/api/runner.rb b/lib/api/runner.rb index 4c9db2c8716..4f27c14d8e8 100644 --- a/lib/api/runner.rb +++ b/lib/api/runner.rb @@ -74,31 +74,44 @@ module API end post '/request' do authenticate_runner! - no_content! unless current_runner.active? update_runner_info if current_runner.is_runner_queue_value_latest?(params[:last_update]) - header 'X-GitLab-Last-Update', params[:last_update] - Gitlab::Metrics.add_event(:build_not_found_cached) + Gitlab::Metrics.add_event(:build_not_found_cached, + version: get_runner_version) + header_last_update(params[:last_update]) return no_content! end new_update = current_runner.ensure_runner_queue_value + + unless current_runner.active? + Gitlab::Metrics.add_event(:runner_inactive, + version: get_runner_version) + header_last_update(new_update) + return no_content! + end + result = ::Ci::RegisterJobService.new(current_runner).execute if result.valid? if result.build Gitlab::Metrics.add_event(:build_found, - project: result.build.project.path_with_namespace) + project: result.build.project.path_with_namespace, + version: get_runner_version) + header_last_update(current_runner.tick_runner_queue_value) present result.build, with: Entities::JobRequest::Response else - Gitlab::Metrics.add_event(:build_not_found) - header 'X-GitLab-Last-Update', new_update + Gitlab::Metrics.add_event(:build_not_found, + version: get_runner_version) + header_last_update(new_update) no_content! end else # We received build that is invalid due to concurrency conflict - Gitlab::Metrics.add_event(:build_invalid) + Gitlab::Metrics.add_event(:build_invalid, + version: get_runner_version) + header_last_update(current_runner.tick_runner_queue_value) conflict! end end diff --git a/lib/ci/api/builds.rb b/lib/ci/api/builds.rb index 746e76a1b1f..f39b509873c 100644 --- a/lib/ci/api/builds.rb +++ b/lib/ci/api/builds.rb @@ -13,35 +13,44 @@ module Ci post "register" do authenticate_runner! required_attributes! [:token] - not_found! unless current_runner.active? update_runner_info if current_runner.is_runner_queue_value_latest?(params[:last_update]) - header 'X-GitLab-Last-Update', params[:last_update] - Gitlab::Metrics.add_event(:build_not_found_cached) + Gitlab::Metrics.add_event(:build_not_found_cached, + version: get_runner_version) + header_last_update(params[:last_update]) return build_not_found! end new_update = current_runner.ensure_runner_queue_value + unless current_runner.active? + Gitlab::Metrics.add_event(:runner_inactive, + version: get_runner_version) + header_last_update(new_update) + return build_not_found! + end + result = Ci::RegisterJobService.new(current_runner).execute if result.valid? if result.build Gitlab::Metrics.add_event(:build_found, - project: result.build.project.path_with_namespace) - + project: result.build.project.path_with_namespace, + version: get_runner_version) + header_last_update(current_runner.tick_runner_queue_value) present result.build, with: Entities::BuildDetails else - Gitlab::Metrics.add_event(:build_not_found) - - header 'X-GitLab-Last-Update', new_update - + Gitlab::Metrics.add_event(:build_not_found, + version: get_runner_version) + header_last_update(new_update) build_not_found! end else # We received build that is invalid due to concurrency conflict - Gitlab::Metrics.add_event(:build_invalid) + Gitlab::Metrics.add_event(:build_invalid, + version: get_runner_version) + header_last_update(current_runner.tick_runner_queue_value) conflict! end end diff --git a/lib/ci/api/helpers.rb b/lib/ci/api/helpers.rb index 996990b464f..868a9cdec70 100644 --- a/lib/ci/api/helpers.rb +++ b/lib/ci/api/helpers.rb @@ -79,6 +79,14 @@ module Ci def max_artifacts_size current_application_settings.max_artifacts_size.megabytes.to_i end + + def get_runner_version + params.fetch('info', {}).fetch('version', "unknown") + end + + def header_last_update(value) + header 'X-GitLab-Last-Update', value + end end end end -- cgit v1.2.1