Commit 5a41d92b authored by Kamil Trzciński's avatar Kamil Trzciński

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

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

See merge request !8084
parents 1bb0191a 746e7e76
......@@ -92,6 +92,12 @@ module Ci
end
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|
build.run_after_commit do
BuildHooksWorker.perform_async(id)
......
......@@ -2,6 +2,7 @@ module Ci
class Runner < ActiveRecord::Base
extend Ci::Model
RUNNER_QUEUE_EXPIRY_TIME = 60.minutes
LAST_CONTACT_TIME = 1.hour.ago
AVAILABLE_SCOPES = %w[specific shared active paused online]
FORM_EDITABLE = %i[description tag_list active run_untagged locked]
......@@ -21,6 +22,8 @@ module Ci
scope :online, ->() { where('contacted_at > ?', LAST_CONTACT_TIME) }
scope :ordered, ->() { order(id: :desc) }
after_save :tick_runner_queue, if: :form_editable_changed?
scope :owned_or_shared, ->(project_id) do
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)
......@@ -122,8 +125,36 @@ module Ci
]
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
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
unless has_tags? || run_untagged?
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:
......@@ -226,7 +226,7 @@ module API
end
def render_api_error!(message, status)
error!({ 'message' => message }, status)
error!({ 'message' => message }, status, header)
end
def handle_api_exception(exception)
......
......@@ -16,6 +16,13 @@ module Ci
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]
return build_not_found!
end
new_update = current_runner.ensure_runner_queue_value
build = Ci::RegisterBuildService.new.execute(current_runner)
if build
......@@ -26,6 +33,8 @@ module Ci
else
Gitlab::Metrics.add_event(:build_not_found)
header 'X-GitLab-Last-Update', new_update
build_not_found!
end
end
......
......@@ -1401,4 +1401,14 @@ describe Ci::Build, :models do
it { is_expected.to eq(%w[predefined project pipeline yaml secret]) }
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
......@@ -263,6 +263,62 @@ describe Ci::Runner, models: true do
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
let(:runner) { create(:ci_runner) }
let(:project) { create(:project) }
......
......@@ -12,6 +12,7 @@ describe API::Helpers, api: true do
let(:params) { {} }
let(:env) { { 'REQUEST_METHOD' => 'GET' } }
let(:request) { Rack::Request.new(env) }
let(:header) { }
def set_env(user_or_token, identifier)
clear_env
......@@ -46,7 +47,7 @@ describe API::Helpers, api: true do
allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ value }
end
def error!(message, status)
def error!(message, status, header)
raise Exception.new("#{status} - #{message}")
end
......
......@@ -4,7 +4,8 @@ describe Ci::API::Builds do
include ApiHelpers
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
let(:pipeline) { create(:ci_pipeline_without_jobs, project: project, ref: 'master') }
......@@ -16,6 +17,8 @@ describe Ci::API::Builds do
describe "POST /builds/register" do
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!(:last_update) { }
let!(:new_update) { }
before do
stub_container_registry_config(enabled: false)
......@@ -24,7 +27,31 @@ describe Ci::API::Builds do
shared_examples 'no builds available' do
context 'when runner sends version in User-Agent' 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
context 'for beta version' do
......@@ -49,6 +76,7 @@ describe Ci::API::Builds do
register_builds info: { platform: :darwin }
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(runner.reload.platform).to eq("darwin")
expect(json_response["options"]).to eq({ "image" => "ruby:2.1", "services" => ["postgres"] })
......@@ -119,10 +147,10 @@ describe Ci::API::Builds do
end
context 'for shared runner' do
let(:shared_runner) { create(:ci_runner, token: "SharedRunner") }
let!(:runner) { create(:ci_runner, :shared, token: "SharedRunner") }
before do
register_builds shared_runner.token
register_builds(runner.token)
end
it_behaves_like 'no builds available'
......@@ -224,7 +252,9 @@ describe Ci::API::Builds do
end
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
......
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