Commit aedac4e8 authored by James Lopez's avatar James Lopez

Merge branch '21698-redis-runner-last-build-ee' into 'master'

Reduce DB-load for build-queues by storing last_update in Redis

See merge request !1083
parents 5fb89926 c4c13f3c
...@@ -93,6 +93,12 @@ module Ci ...@@ -93,6 +93,12 @@ module Ci
end end
state_machine :status do state_machine :status do
after_transition any => [:pending] do |build|
build.run_after_commit do
BuildQueueWorker.perform_async(id)
end
end
after_transition pending: :running do |build| after_transition pending: :running do |build|
build.run_after_commit do build.run_after_commit do
BuildHooksWorker.perform_async(id) BuildHooksWorker.perform_async(id)
......
...@@ -2,6 +2,7 @@ module Ci ...@@ -2,6 +2,7 @@ module Ci
class Runner < ActiveRecord::Base class Runner < ActiveRecord::Base
extend Ci::Model extend Ci::Model
RUNNER_QUEUE_EXPIRY_TIME = 60.minutes
LAST_CONTACT_TIME = 1.hour.ago LAST_CONTACT_TIME = 1.hour.ago
AVAILABLE_SCOPES = %w[specific shared active paused online] AVAILABLE_SCOPES = %w[specific shared active paused online]
FORM_EDITABLE = %i[description tag_list active run_untagged locked] FORM_EDITABLE = %i[description tag_list active run_untagged locked]
...@@ -21,6 +22,8 @@ module Ci ...@@ -21,6 +22,8 @@ module Ci
scope :online, ->() { where('contacted_at > ?', LAST_CONTACT_TIME) } scope :online, ->() { where('contacted_at > ?', LAST_CONTACT_TIME) }
scope :ordered, ->() { order(id: :desc) } scope :ordered, ->() { order(id: :desc) }
after_save :tick_runner_queue, if: :form_editable_changed?
scope :owned_or_shared, ->(project_id) do scope :owned_or_shared, ->(project_id) do
joins('LEFT JOIN ci_runner_projects ON ci_runner_projects.runner_id = ci_runners.id') joins('LEFT JOIN ci_runner_projects ON ci_runner_projects.runner_id = ci_runners.id')
.where("ci_runner_projects.gl_project_id = :project_id OR ci_runners.is_shared = true", project_id: project_id) .where("ci_runner_projects.gl_project_id = :project_id OR ci_runners.is_shared = true", project_id: project_id)
...@@ -122,8 +125,36 @@ module Ci ...@@ -122,8 +125,36 @@ module Ci
] ]
end end
def tick_runner_queue
new_update = SecureRandom.hex
Gitlab::Redis.with { |redis| redis.set(runner_queue_key, new_update, ex: RUNNER_QUEUE_EXPIRY_TIME) }
new_update
end
def ensure_runner_queue_value
Gitlab::Redis.with do |redis|
value = SecureRandom.hex
redis.set(runner_queue_key, value, ex: RUNNER_QUEUE_EXPIRY_TIME, nx: true)
redis.get(runner_queue_key)
end
end
def is_runner_queue_value_latest?(value)
ensure_runner_queue_value == value if value.present?
end
private private
def runner_queue_key
"runner:build_queue:#{self.token}"
end
def form_editable_changed?
FORM_EDITABLE.any? do |editable|
public_send("#{editable}_changed?")
end
end
def tag_constraints def tag_constraints
unless has_tags? || run_untagged? unless has_tags? || run_untagged?
errors.add(:tags_list, errors.add(:tags_list,
......
module Ci
class UpdateBuildQueueService
def execute(build)
build.project.runners.each do |runner|
if runner.can_pick?(build)
runner.tick_runner_queue
end
end
end
end
end
class BuildQueueWorker
include Sidekiq::Worker
include BuildQueue
def perform(build_id)
Ci::Build.find_by(id: build_id).try do |build|
Ci::UpdateBuildQueueService.new.execute(build)
end
end
end
---
title: Reduce DB-load for build-queues by storing last_update in Redis
merge_request: 8084
author:
...@@ -233,7 +233,7 @@ module API ...@@ -233,7 +233,7 @@ module API
end end
def render_api_error!(message, status) def render_api_error!(message, status)
error!({ 'message' => message }, status) error!({ 'message' => message }, status, header)
end end
def handle_api_exception(exception) def handle_api_exception(exception)
......
...@@ -16,6 +16,13 @@ module Ci ...@@ -16,6 +16,13 @@ module Ci
not_found! unless current_runner.active? not_found! unless current_runner.active?
update_runner_info update_runner_info
if current_runner.is_runner_queue_value_latest?(params[:last_update])
header 'X-GitLab-Last-Update', params[:last_update]
return build_not_found!
end
new_update = current_runner.ensure_runner_queue_value
build = Ci::RegisterBuildService.new(current_runner).execute build = Ci::RegisterBuildService.new(current_runner).execute
if build if build
...@@ -26,6 +33,8 @@ module Ci ...@@ -26,6 +33,8 @@ module Ci
else else
Gitlab::Metrics.add_event(:build_not_found) Gitlab::Metrics.add_event(:build_not_found)
header 'X-GitLab-Last-Update', new_update
build_not_found! build_not_found!
end end
end end
......
...@@ -1401,4 +1401,14 @@ describe Ci::Build, :models do ...@@ -1401,4 +1401,14 @@ describe Ci::Build, :models do
it { is_expected.to eq(%w[predefined project pipeline yaml secret]) } it { is_expected.to eq(%w[predefined project pipeline yaml secret]) }
end end
end end
describe 'State transition: any => [:pending]' do
let(:build) { create(:ci_build, :created) }
it 'queues BuildQueueWorker' do
expect(BuildQueueWorker).to receive(:perform_async).with(build.id)
build.enqueue
end
end
end end
...@@ -263,6 +263,62 @@ describe Ci::Runner, models: true do ...@@ -263,6 +263,62 @@ describe Ci::Runner, models: true do
end end
end end
describe '#tick_runner_queue' do
let(:runner) { create(:ci_runner) }
it 'returns a new last_update value' do
expect(runner.tick_runner_queue).not_to be_empty
end
end
describe '#ensure_runner_queue_value' do
let(:runner) { create(:ci_runner) }
it 'sets a new last_update value when it is called the first time' do
last_update = runner.ensure_runner_queue_value
expect_value_in_redis.to eq(last_update)
end
it 'does not change if it is not expired and called again' do
last_update = runner.ensure_runner_queue_value
expect(runner.ensure_runner_queue_value).to eq(last_update)
expect_value_in_redis.to eq(last_update)
end
context 'updates runner queue after changing editable value' do
let!(:last_update) { runner.ensure_runner_queue_value }
before do
runner.update(description: 'new runner')
end
it 'sets a new last_update value' do
expect_value_in_redis.not_to eq(last_update)
end
end
context 'does not update runner value after save' do
let!(:last_update) { runner.ensure_runner_queue_value }
before do
runner.touch
end
it 'has an old last_update value' do
expect_value_in_redis.to eq(last_update)
end
end
def expect_value_in_redis
Gitlab::Redis.with do |redis|
runner_queue_key = runner.send(:runner_queue_key)
expect(redis.get(runner_queue_key))
end
end
end
describe '.assignable_for' do describe '.assignable_for' do
let(:runner) { create(:ci_runner) } let(:runner) { create(:ci_runner) }
let(:project) { create(:project) } let(:project) { create(:project) }
......
...@@ -12,6 +12,7 @@ describe API::Helpers, api: true do ...@@ -12,6 +12,7 @@ describe API::Helpers, api: true do
let(:params) { {} } let(:params) { {} }
let(:env) { { 'REQUEST_METHOD' => 'GET' } } let(:env) { { 'REQUEST_METHOD' => 'GET' } }
let(:request) { Rack::Request.new(env) } let(:request) { Rack::Request.new(env) }
let(:header) { }
def set_env(user_or_token, identifier) def set_env(user_or_token, identifier)
clear_env clear_env
...@@ -46,7 +47,7 @@ describe API::Helpers, api: true do ...@@ -46,7 +47,7 @@ describe API::Helpers, api: true do
allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ value } allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ value }
end end
def error!(message, status) def error!(message, status, header)
raise Exception.new("#{status} - #{message}") raise Exception.new("#{status} - #{message}")
end end
......
...@@ -4,7 +4,8 @@ describe Ci::API::Builds do ...@@ -4,7 +4,8 @@ describe Ci::API::Builds do
include ApiHelpers include ApiHelpers
let(:runner) { FactoryGirl.create(:ci_runner, tag_list: ["mysql", "ruby"]) } let(:runner) { FactoryGirl.create(:ci_runner, tag_list: ["mysql", "ruby"]) }
let(:project) { FactoryGirl.create(:empty_project) } let(:project) { FactoryGirl.create(:empty_project, shared_runners_enabled: false) }
let(:last_update) { nil }
describe "Builds API for runners" do describe "Builds API for runners" do
let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') } let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') }
...@@ -16,6 +17,8 @@ describe Ci::API::Builds do ...@@ -16,6 +17,8 @@ describe Ci::API::Builds do
describe "POST /builds/register" do describe "POST /builds/register" do
let!(:build) { create(:ci_build, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) } let!(:build) { create(:ci_build, pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0) }
let(:user_agent) { 'gitlab-ci-multi-runner 1.5.2 (1-5-stable; go1.6.3; linux/amd64)' } let(:user_agent) { 'gitlab-ci-multi-runner 1.5.2 (1-5-stable; go1.6.3; linux/amd64)' }
let!(:last_update) { }
let!(:new_update) { }
before do before do
stub_container_registry_config(enabled: false) stub_container_registry_config(enabled: false)
...@@ -24,7 +27,31 @@ describe Ci::API::Builds do ...@@ -24,7 +27,31 @@ describe Ci::API::Builds do
shared_examples 'no builds available' do shared_examples 'no builds available' do
context 'when runner sends version in User-Agent' do context 'when runner sends version in User-Agent' do
context 'for stable version' do context 'for stable version' do
it { expect(response).to have_http_status(204) } it 'gives 204 and set X-GitLab-Last-Update' do
expect(response).to have_http_status(204)
expect(response.header).to have_key('X-GitLab-Last-Update')
end
end
context 'when last_update is up-to-date' do
let(:last_update) { runner.ensure_runner_queue_value }
it 'gives 204 and set the same X-GitLab-Last-Update' do
expect(response).to have_http_status(204)
expect(response.header['X-GitLab-Last-Update'])
.to eq(last_update)
end
end
context 'when last_update is outdated' do
let(:last_update) { runner.ensure_runner_queue_value }
let(:new_update) { runner.tick_runner_queue }
it 'gives 204 and set a new X-GitLab-Last-Update' do
expect(response).to have_http_status(204)
expect(response.header['X-GitLab-Last-Update'])
.to eq(new_update)
end
end end
context 'for beta version' do context 'for beta version' do
...@@ -49,6 +76,7 @@ describe Ci::API::Builds do ...@@ -49,6 +76,7 @@ describe Ci::API::Builds do
register_builds info: { platform: :darwin } register_builds info: { platform: :darwin }
expect(response).to have_http_status(201) expect(response).to have_http_status(201)
expect(response.headers).not_to have_key('X-GitLab-Last-Update')
expect(json_response['sha']).to eq(build.sha) expect(json_response['sha']).to eq(build.sha)
expect(runner.reload.platform).to eq("darwin") expect(runner.reload.platform).to eq("darwin")
expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] }) expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] })
...@@ -119,10 +147,10 @@ describe Ci::API::Builds do ...@@ -119,10 +147,10 @@ describe Ci::API::Builds do
end end
context 'for shared runner' do context 'for shared runner' do
let(:shared_runner) { create(:ci_runner, token: "SharedRunner") } let!(:runner) { create(:ci_runner, :shared, token: "SharedRunner") }
before do before do
register_builds shared_runner.token register_builds(runner.token)
end end
it_behaves_like 'no builds available' it_behaves_like 'no builds available'
...@@ -224,7 +252,9 @@ describe Ci::API::Builds do ...@@ -224,7 +252,9 @@ describe Ci::API::Builds do
end end
def register_builds(token = runner.token, **params) def register_builds(token = runner.token, **params)
post ci_api("/builds/register"), params.merge(token: token), { 'User-Agent' => user_agent } new_params = params.merge(token: token, last_update: last_update)
post ci_api("/builds/register"), new_params, { 'User-Agent' => user_agent }
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment