Commit 0fd0b64b authored by Grzegorz Bizon's avatar Grzegorz Bizon

Use stages position column to track stage index

parent b337a086
...@@ -17,7 +17,7 @@ module Ci ...@@ -17,7 +17,7 @@ module Ci
validates :project, presence: true validates :project, presence: true
validates :pipeline, presence: true validates :pipeline, presence: true
validates :name, presence: true validates :name, presence: true
validates :priority, presence: true validates :position, presence: true
end end
after_initialize do after_initialize do
...@@ -25,9 +25,9 @@ module Ci ...@@ -25,9 +25,9 @@ module Ci
end end
before_validation unless: :importing? do before_validation unless: :importing? do
next if priority.present? next if position.present?
self.priority = statuses.select(:stage_idx) self.position = statuses.select(:stage_idx)
.where('stage_idx IS NOT NULL') .where('stage_idx IS NOT NULL')
.group(:stage_idx) .group(:stage_idx)
.order('COUNT(*) DESC') .order('COUNT(*) DESC')
......
...@@ -42,7 +42,7 @@ module Ci ...@@ -42,7 +42,7 @@ module Ci
def create_stage def create_stage
Ci::Stage.create!(name: @build.stage, Ci::Stage.create!(name: @build.stage,
priority: @build.stage_idx, position: @build.stage_idx,
pipeline: @build.pipeline, pipeline: @build.pipeline,
project: @build.project) project: @build.project)
end end
......
...@@ -7,10 +7,10 @@ class AddTmpStagePriorityIndexToCiBuilds < ActiveRecord::Migration ...@@ -7,10 +7,10 @@ class AddTmpStagePriorityIndexToCiBuilds < ActiveRecord::Migration
def up def up
add_concurrent_index(:ci_builds, [:stage_id, :stage_idx], add_concurrent_index(:ci_builds, [:stage_id, :stage_idx],
where: 'stage_idx IS NOT NULL', name: 'tmp_build_stage_priority_index') where: 'stage_idx IS NOT NULL', name: 'tmp_build_stage_position_index')
end end
def down def down
remove_concurrent_index_by_name(:ci_builds, 'tmp_build_stage_priority_index') remove_concurrent_index_by_name(:ci_builds, 'tmp_build_stage_position_index')
end end
end end
...@@ -4,6 +4,6 @@ class AddIndexToCiStage < ActiveRecord::Migration ...@@ -4,6 +4,6 @@ class AddIndexToCiStage < ActiveRecord::Migration
DOWNTIME = false DOWNTIME = false
def change def change
add_column :ci_stages, :priority, :integer add_column :ci_stages, :position, :integer
end end
end end
...@@ -322,7 +322,7 @@ ActiveRecord::Schema.define(version: 20180425131009) do ...@@ -322,7 +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_priority_index", where: "(stage_idx IS NOT NULL)", 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
...@@ -487,7 +487,7 @@ ActiveRecord::Schema.define(version: 20180425131009) do ...@@ -487,7 +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 "priority" 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
......
...@@ -26,19 +26,19 @@ module Gitlab ...@@ -26,19 +26,19 @@ module Gitlab
FROM freqs FROM freqs
) )
UPDATE ci_stages SET priority = indexes.index UPDATE ci_stages SET position = indexes.index
FROM indexes WHERE indexes.stage_id = ci_stages.id FROM indexes WHERE indexes.stage_id = ci_stages.id
AND ci_stages.priority IS NULL; AND ci_stages.position IS NULL;
SQL SQL
else else
<<~SQL <<~SQL
UPDATE ci_stages UPDATE ci_stages
SET priority = SET position =
(SELECT stage_idx FROM ci_builds (SELECT stage_idx FROM ci_builds
WHERE ci_builds.stage_id = ci_stages.id WHERE ci_builds.stage_id = ci_stages.id
GROUP BY ci_builds.stage_idx ORDER BY COUNT(*) DESC LIMIT 1) GROUP BY ci_builds.stage_idx ORDER BY COUNT(*) DESC LIMIT 1)
WHERE ci_stages.id BETWEEN #{start_id} AND #{stop_id} WHERE ci_stages.id BETWEEN #{start_id} AND #{stop_id}
AND ci_stages.priority IS NULL AND ci_stages.position IS NULL
SQL SQL
end end
end end
......
...@@ -19,7 +19,7 @@ module Gitlab ...@@ -19,7 +19,7 @@ module Gitlab
def attributes def attributes
{ name: @attributes.fetch(:name), { name: @attributes.fetch(:name),
priority: @attributes.fetch(:index), position: @attributes.fetch(:index),
pipeline: @pipeline, pipeline: @pipeline,
project: @pipeline.project } project: @pipeline.project }
end end
......
...@@ -21,7 +21,7 @@ FactoryBot.define do ...@@ -21,7 +21,7 @@ FactoryBot.define do
pipeline factory: :ci_empty_pipeline pipeline factory: :ci_empty_pipeline
name 'test' name 'test'
priority 1 position 1
status 'pending' status 'pending'
end end
end end
...@@ -26,10 +26,10 @@ describe Gitlab::BackgroundMigration::MigrateStageIndex, :migration, schema: 201 ...@@ -26,10 +26,10 @@ describe Gitlab::BackgroundMigration::MigrateStageIndex, :migration, schema: 201
end end
it 'correctly migrates stages indices' do it 'correctly migrates stages indices' do
expect(stages.all.pluck(:priority)).to all(be_nil) expect(stages.all.pluck(:position)).to all(be_nil)
described_class.new.perform(100, 101) described_class.new.perform(100, 101)
expect(stages.all.pluck(:priority)).to eq [2, 3] expect(stages.all.pluck(:position)).to eq [2, 3]
end end
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', priority: 0, project: project) pipeline.stages.build(name: 'test', position: 0, project: project)
step.perform! step.perform!
end end
......
...@@ -25,7 +25,7 @@ describe Gitlab::Ci::Pipeline::Seed::Stage do ...@@ -25,7 +25,7 @@ describe Gitlab::Ci::Pipeline::Seed::Stage 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) expect(subject.attributes)
.to include(:name, :priority, :pipeline, :project) .to include(:name, :position, :pipeline, :project)
end end
end end
......
...@@ -232,7 +232,7 @@ Ci::Stage: ...@@ -232,7 +232,7 @@ Ci::Stage:
- id - id
- name - name
- status - status
- priority - position
- lock_version - lock_version
- project_id - project_id
- pipeline_id - pipeline_id
......
...@@ -21,7 +21,7 @@ describe ScheduleStagesIndexMigration, :sidekiq, :migration do ...@@ -21,7 +21,7 @@ describe ScheduleStagesIndexMigration, :sidekiq, :migration do
it 'schedules delayed background migrations in batches' do it 'schedules delayed background migrations in batches' do
Sidekiq::Testing.fake! do Sidekiq::Testing.fake! do
Timecop.freeze do Timecop.freeze do
expect(stages.all).to all(have_attributes(priority: be_nil)) expect(stages.all).to all(have_attributes(position: be_nil))
migrate! migrate!
......
...@@ -89,9 +89,9 @@ describe Ci::Stage, :models do ...@@ -89,9 +89,9 @@ describe Ci::Stage, :models do
end end
describe '#index' do describe '#index' do
context 'when stage has been imported and does not have priority index set' do context 'when stage has been imported and does not have position index set' do
before do before do
stage.update_column(:priority, nil) stage.update_column(:position, nil)
end end
context 'when stage has statuses' do context 'when stage has statuses' do
...@@ -100,21 +100,21 @@ describe Ci::Stage, :models do ...@@ -100,21 +100,21 @@ describe Ci::Stage, :models do
end end
it 'recalculates index before updating status' do it 'recalculates index before updating status' do
expect(stage.reload.priority).to be_nil expect(stage.reload.position).to be_nil
stage.update_status stage.update_status
expect(stage.reload.priority).to eq 10 expect(stage.reload.position).to eq 10
end end
end end
context 'when stage does not have statuses' do context 'when stage does not have statuses' do
it 'fallbacks to zero' do it 'fallbacks to zero' do
expect(stage.reload.priority).to be_nil expect(stage.reload.position).to be_nil
stage.update_status stage.update_status
expect(stage.reload.priority).to eq 0 expect(stage.reload.position).to eq 0
end end
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