Commit 8b0b4ece authored by Kamil Trzciński's avatar Kamil Trzciński

Merge branch 'backstage/gb/migrate-pipeline-stages-index' into 'master'

Migrate pipeline stages indices

Closes #43009

See merge request gitlab-org/gitlab-ce!18420
parents b8a84830 0fd0b64b
...@@ -13,14 +13,27 @@ module Ci ...@@ -13,14 +13,27 @@ module Ci
has_many :statuses, class_name: 'CommitStatus', foreign_key: :stage_id has_many :statuses, class_name: 'CommitStatus', foreign_key: :stage_id
has_many :builds, foreign_key: :stage_id has_many :builds, foreign_key: :stage_id
validates :project, presence: true, unless: :importing? with_options unless: :importing? do
validates :pipeline, presence: true, unless: :importing? validates :project, presence: true
validates :name, presence: true, unless: :importing? validates :pipeline, presence: true
validates :name, presence: true
validates :position, presence: true
end
after_initialize do |stage| after_initialize do
self.status = DEFAULT_STATUS if self.status.nil? self.status = DEFAULT_STATUS if self.status.nil?
end end
before_validation unless: :importing? do
next if position.present?
self.position = statuses.select(:stage_idx)
.where('stage_idx IS NOT NULL')
.group(:stage_idx)
.order('COUNT(*) DESC')
.first&.stage_idx.to_i
end
state_machine :status, initial: :created do state_machine :status, initial: :created do
event :enqueue do event :enqueue do
transition created: :pending transition created: :pending
......
...@@ -42,6 +42,7 @@ module Ci ...@@ -42,6 +42,7 @@ module Ci
def create_stage def create_stage
Ci::Stage.create!(name: @build.stage, Ci::Stage.create!(name: @build.stage,
position: @build.stage_idx,
pipeline: @build.pipeline, pipeline: @build.pipeline,
project: @build.project) project: @build.project)
end end
......
class AddTmpStagePriorityIndexToCiBuilds < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_concurrent_index(:ci_builds, [:stage_id, :stage_idx],
where: 'stage_idx IS NOT NULL', name: 'tmp_build_stage_position_index')
end
def down
remove_concurrent_index_by_name(:ci_builds, 'tmp_build_stage_position_index')
end
end
class AddIndexToCiStage < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def change
add_column :ci_stages, :position, :integer
end
end
class ScheduleStagesIndexMigration < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
MIGRATION = 'MigrateStageIndex'.freeze
BATCH_SIZE = 10000
disable_ddl_transaction!
class Stage < ActiveRecord::Base
include EachBatch
self.table_name = 'ci_stages'
end
def up
disable_statement_timeout
Stage.all.tap do |relation|
queue_background_migration_jobs_by_range_at_intervals(relation,
MIGRATION,
5.minutes,
batch_size: BATCH_SIZE)
end
end
def down
# noop
end
end
...@@ -322,6 +322,7 @@ ActiveRecord::Schema.define(version: 20180425131009) do ...@@ -322,6 +322,7 @@ ActiveRecord::Schema.define(version: 20180425131009) do
add_index "ci_builds", ["project_id", "id"], name: "index_ci_builds_on_project_id_and_id", using: :btree add_index "ci_builds", ["project_id", "id"], name: "index_ci_builds_on_project_id_and_id", using: :btree
add_index "ci_builds", ["protected"], name: "index_ci_builds_on_protected", using: :btree add_index "ci_builds", ["protected"], name: "index_ci_builds_on_protected", using: :btree
add_index "ci_builds", ["runner_id"], name: "index_ci_builds_on_runner_id", using: :btree add_index "ci_builds", ["runner_id"], name: "index_ci_builds_on_runner_id", using: :btree
add_index "ci_builds", ["stage_id", "stage_idx"], name: "tmp_build_stage_position_index", where: "(stage_idx IS NOT NULL)", using: :btree
add_index "ci_builds", ["stage_id"], name: "index_ci_builds_on_stage_id", using: :btree add_index "ci_builds", ["stage_id"], name: "index_ci_builds_on_stage_id", using: :btree
add_index "ci_builds", ["status", "type", "runner_id"], name: "index_ci_builds_on_status_and_type_and_runner_id", using: :btree add_index "ci_builds", ["status", "type", "runner_id"], name: "index_ci_builds_on_status_and_type_and_runner_id", using: :btree
add_index "ci_builds", ["status"], name: "index_ci_builds_on_status", using: :btree add_index "ci_builds", ["status"], name: "index_ci_builds_on_status", using: :btree
...@@ -486,6 +487,7 @@ ActiveRecord::Schema.define(version: 20180425131009) do ...@@ -486,6 +487,7 @@ ActiveRecord::Schema.define(version: 20180425131009) do
t.string "name" t.string "name"
t.integer "status" t.integer "status"
t.integer "lock_version" t.integer "lock_version"
t.integer "position"
end end
add_index "ci_stages", ["pipeline_id", "name"], name: "index_ci_stages_on_pipeline_id_and_name", unique: true, using: :btree add_index "ci_stages", ["pipeline_id", "name"], name: "index_ci_stages_on_pipeline_id_and_name", unique: true, using: :btree
......
# frozen_string_literal: true
# rubocop:disable Style/Documentation
module Gitlab
module BackgroundMigration
class MigrateStageIndex
def perform(start_id, stop_id)
migrate_stage_index_sql(start_id.to_i, stop_id.to_i).tap do |sql|
ActiveRecord::Base.connection.execute(sql)
end
end
private
def migrate_stage_index_sql(start_id, stop_id)
if Gitlab::Database.postgresql?
<<~SQL
WITH freqs AS (
SELECT stage_id, stage_idx, COUNT(*) AS freq FROM ci_builds
WHERE stage_id BETWEEN #{start_id} AND #{stop_id}
AND stage_idx IS NOT NULL
GROUP BY stage_id, stage_idx
), indexes AS (
SELECT DISTINCT stage_id, last_value(stage_idx)
OVER (PARTITION BY stage_id ORDER BY freq ASC) AS index
FROM freqs
)
UPDATE ci_stages SET position = indexes.index
FROM indexes WHERE indexes.stage_id = ci_stages.id
AND ci_stages.position IS NULL;
SQL
else
<<~SQL
UPDATE ci_stages
SET position =
(SELECT stage_idx FROM ci_builds
WHERE ci_builds.stage_id = ci_stages.id
GROUP BY ci_builds.stage_idx ORDER BY COUNT(*) DESC LIMIT 1)
WHERE ci_stages.id BETWEEN #{start_id} AND #{stop_id}
AND ci_stages.position IS NULL
SQL
end
end
end
end
end
...@@ -19,6 +19,7 @@ module Gitlab ...@@ -19,6 +19,7 @@ module Gitlab
def attributes def attributes
{ name: @attributes.fetch(:name), { name: @attributes.fetch(:name),
position: @attributes.fetch(:index),
pipeline: @pipeline, pipeline: @pipeline,
project: @pipeline.project } project: @pipeline.project }
end end
......
...@@ -21,6 +21,7 @@ FactoryBot.define do ...@@ -21,6 +21,7 @@ FactoryBot.define do
pipeline factory: :ci_empty_pipeline pipeline factory: :ci_empty_pipeline
name 'test' name 'test'
position 1
status 'pending' status 'pending'
end end
end end
...@@ -2,6 +2,7 @@ FactoryBot.define do ...@@ -2,6 +2,7 @@ FactoryBot.define do
factory :commit_status, class: CommitStatus do factory :commit_status, class: CommitStatus do
name 'default' name 'default'
stage 'test' stage 'test'
stage_idx 0
status 'success' status 'success'
description 'commit status' description 'commit status'
pipeline factory: :ci_pipeline_with_one_job pipeline factory: :ci_pipeline_with_one_job
......
require 'spec_helper'
describe Gitlab::BackgroundMigration::MigrateStageIndex, :migration, schema: 20180420080616 do
let(:namespaces) { table(:namespaces) }
let(:projects) { table(:projects) }
let(:pipelines) { table(:ci_pipelines) }
let(:stages) { table(:ci_stages) }
let(:jobs) { table(:ci_builds) }
before do
namespaces.create(id: 10, name: 'gitlab-org', path: 'gitlab-org')
projects.create!(id: 11, namespace_id: 10, name: 'gitlab', path: 'gitlab')
pipelines.create!(id: 12, project_id: 11, ref: 'master', sha: 'adf43c3a')
stages.create(id: 100, project_id: 11, pipeline_id: 12, name: 'build')
stages.create(id: 101, project_id: 11, pipeline_id: 12, name: 'test')
jobs.create!(id: 121, commit_id: 12, project_id: 11,
stage_idx: 2, stage_id: 100)
jobs.create!(id: 122, commit_id: 12, project_id: 11,
stage_idx: 2, stage_id: 100)
jobs.create!(id: 123, commit_id: 12, project_id: 11,
stage_idx: 10, stage_id: 100)
jobs.create!(id: 124, commit_id: 12, project_id: 11,
stage_idx: 3, stage_id: 101)
end
it 'correctly migrates stages indices' do
expect(stages.all.pluck(:position)).to all(be_nil)
described_class.new.perform(100, 101)
expect(stages.all.pluck(:position)).to eq [2, 3]
end
end
...@@ -17,7 +17,7 @@ describe Gitlab::Ci::Pipeline::Chain::Create do ...@@ -17,7 +17,7 @@ describe Gitlab::Ci::Pipeline::Chain::Create do
context 'when pipeline is ready to be saved' do context 'when pipeline is ready to be saved' do
before do before do
pipeline.stages.build(name: 'test', project: project) pipeline.stages.build(name: 'test', position: 0, project: project)
step.perform! step.perform!
end end
......
...@@ -24,7 +24,8 @@ describe Gitlab::Ci::Pipeline::Seed::Stage do ...@@ -24,7 +24,8 @@ describe Gitlab::Ci::Pipeline::Seed::Stage do
describe '#attributes' do describe '#attributes' do
it 'returns hash attributes of a stage' do it 'returns hash attributes of a stage' do
expect(subject.attributes).to be_a Hash expect(subject.attributes).to be_a Hash
expect(subject.attributes).to include(:name, :project) expect(subject.attributes)
.to include(:name, :position, :pipeline, :project)
end end
end end
......
...@@ -232,6 +232,7 @@ Ci::Stage: ...@@ -232,6 +232,7 @@ Ci::Stage:
- id - id
- name - name
- status - status
- position
- lock_version - lock_version
- project_id - project_id
- pipeline_id - pipeline_id
......
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20180420080616_schedule_stages_index_migration')
describe ScheduleStagesIndexMigration, :sidekiq, :migration do
let(:namespaces) { table(:namespaces) }
let(:projects) { table(:projects) }
let(:pipelines) { table(:ci_pipelines) }
let(:stages) { table(:ci_stages) }
before do
stub_const("#{described_class}::BATCH_SIZE", 1)
namespaces.create(id: 12, name: 'gitlab-org', path: 'gitlab-org')
projects.create!(id: 123, namespace_id: 12, name: 'gitlab', path: 'gitlab')
pipelines.create!(id: 1, project_id: 123, ref: 'master', sha: 'adf43c3a')
stages.create!(id: 121, project_id: 123, pipeline_id: 1, name: 'build')
stages.create!(id: 122, project_id: 123, pipeline_id: 1, name: 'test')
stages.create!(id: 123, project_id: 123, pipeline_id: 1, name: 'deploy')
end
it 'schedules delayed background migrations in batches' do
Sidekiq::Testing.fake! do
Timecop.freeze do
expect(stages.all).to all(have_attributes(position: be_nil))
migrate!
expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, 121, 121)
expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, 122, 122)
expect(described_class::MIGRATION).to be_scheduled_delayed_migration(15.minutes, 123, 123)
expect(BackgroundMigrationWorker.jobs.size).to eq 3
end
end
end
end
...@@ -51,7 +51,7 @@ describe Ci::Stage, :models do ...@@ -51,7 +51,7 @@ describe Ci::Stage, :models do
end end
end end
describe 'update_status' do describe '#update_status' do
context 'when stage objects needs to be updated' do context 'when stage objects needs to be updated' do
before do before do
create(:ci_build, :success, stage_id: stage.id) create(:ci_build, :success, stage_id: stage.id)
...@@ -87,4 +87,36 @@ describe Ci::Stage, :models do ...@@ -87,4 +87,36 @@ describe Ci::Stage, :models do
end end
end end
end end
describe '#index' do
context 'when stage has been imported and does not have position index set' do
before do
stage.update_column(:position, nil)
end
context 'when stage has statuses' do
before do
create(:ci_build, :running, stage_id: stage.id, stage_idx: 10)
end
it 'recalculates index before updating status' do
expect(stage.reload.position).to be_nil
stage.update_status
expect(stage.reload.position).to eq 10
end
end
context 'when stage does not have statuses' do
it 'fallbacks to zero' do
expect(stage.reload.position).to be_nil
stage.update_status
expect(stage.reload.position).to eq 0
end
end
end
end
end end
...@@ -6,7 +6,9 @@ describe Ci::RetryBuildService do ...@@ -6,7 +6,9 @@ describe Ci::RetryBuildService do
set(:pipeline) { create(:ci_pipeline, project: project) } set(:pipeline) { create(:ci_pipeline, project: project) }
let(:stage) do let(:stage) do
Ci::Stage.create!(project: project, pipeline: pipeline, name: 'test') create(:ci_stage_entity, project: project,
pipeline: pipeline,
name: 'test')
end end
let(:build) { create(:ci_build, pipeline: pipeline, stage_id: stage.id) } let(:build) { create(:ci_build, pipeline: pipeline, stage_id: stage.id) }
......
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