From 0fd0b64be63c18bb216f15d887e3ce0955dcf269 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 1 May 2018 14:30:44 +0200 Subject: [PATCH] Use stages position column to track stage index --- app/models/ci/stage.rb | 6 +++--- app/services/ci/ensure_stage_service.rb | 2 +- ...1040_add_tmp_stage_priority_index_to_ci_builds.rb | 4 ++-- db/migrate/20180417101940_add_index_to_ci_stage.rb | 2 +- db/schema.rb | 4 ++-- .../background_migration/migrate_stage_index.rb | 8 ++++---- lib/gitlab/ci/pipeline/seed/stage.rb | 2 +- spec/factories/ci/stages.rb | 2 +- .../background_migration/migrate_stage_index_spec.rb | 4 ++-- spec/lib/gitlab/ci/pipeline/chain/create_spec.rb | 2 +- spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb | 2 +- .../gitlab/import_export/safe_model_attributes.yml | 2 +- .../schedule_stages_index_migration_spec.rb | 2 +- spec/models/ci/stage_spec.rb | 12 ++++++------ 14 files changed, 27 insertions(+), 27 deletions(-) diff --git a/app/models/ci/stage.rb b/app/models/ci/stage.rb index 9a913213bb9..5a1eeb966aa 100644 --- a/app/models/ci/stage.rb +++ b/app/models/ci/stage.rb @@ -17,7 +17,7 @@ module Ci validates :project, presence: true validates :pipeline, presence: true validates :name, presence: true - validates :priority, presence: true + validates :position, presence: true end after_initialize do @@ -25,9 +25,9 @@ module Ci end 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') .group(:stage_idx) .order('COUNT(*) DESC') diff --git a/app/services/ci/ensure_stage_service.rb b/app/services/ci/ensure_stage_service.rb index e3e3e2d03c1..b8c7be2d350 100644 --- a/app/services/ci/ensure_stage_service.rb +++ b/app/services/ci/ensure_stage_service.rb @@ -42,7 +42,7 @@ module Ci def create_stage Ci::Stage.create!(name: @build.stage, - priority: @build.stage_idx, + position: @build.stage_idx, pipeline: @build.pipeline, project: @build.project) end diff --git a/db/migrate/20180417101040_add_tmp_stage_priority_index_to_ci_builds.rb b/db/migrate/20180417101040_add_tmp_stage_priority_index_to_ci_builds.rb index c07d1c69a81..ee82c70ecf8 100644 --- a/db/migrate/20180417101040_add_tmp_stage_priority_index_to_ci_builds.rb +++ b/db/migrate/20180417101040_add_tmp_stage_priority_index_to_ci_builds.rb @@ -7,10 +7,10 @@ class AddTmpStagePriorityIndexToCiBuilds < ActiveRecord::Migration def up 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 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 diff --git a/db/migrate/20180417101940_add_index_to_ci_stage.rb b/db/migrate/20180417101940_add_index_to_ci_stage.rb index ec21431b2a5..9dac78db774 100644 --- a/db/migrate/20180417101940_add_index_to_ci_stage.rb +++ b/db/migrate/20180417101940_add_index_to_ci_stage.rb @@ -4,6 +4,6 @@ class AddIndexToCiStage < ActiveRecord::Migration DOWNTIME = false def change - add_column :ci_stages, :priority, :integer + add_column :ci_stages, :position, :integer end end diff --git a/db/schema.rb b/db/schema.rb index 8c15da3875b..10cd1bff125 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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", ["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", ["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", ["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 @@ -487,7 +487,7 @@ ActiveRecord::Schema.define(version: 20180425131009) do t.string "name" t.integer "status" t.integer "lock_version" - t.integer "priority" + t.integer "position" end add_index "ci_stages", ["pipeline_id", "name"], name: "index_ci_stages_on_pipeline_id_and_name", unique: true, using: :btree diff --git a/lib/gitlab/background_migration/migrate_stage_index.rb b/lib/gitlab/background_migration/migrate_stage_index.rb index 140c75054df..f90f35a913d 100644 --- a/lib/gitlab/background_migration/migrate_stage_index.rb +++ b/lib/gitlab/background_migration/migrate_stage_index.rb @@ -26,19 +26,19 @@ module Gitlab 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 - AND ci_stages.priority IS NULL; + AND ci_stages.position IS NULL; SQL else <<~SQL UPDATE ci_stages - SET priority = + 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.priority IS NULL + AND ci_stages.position IS NULL SQL end end diff --git a/lib/gitlab/ci/pipeline/seed/stage.rb b/lib/gitlab/ci/pipeline/seed/stage.rb index 2ff80bdf3b9..2b58d9863a0 100644 --- a/lib/gitlab/ci/pipeline/seed/stage.rb +++ b/lib/gitlab/ci/pipeline/seed/stage.rb @@ -19,7 +19,7 @@ module Gitlab def attributes { name: @attributes.fetch(:name), - priority: @attributes.fetch(:index), + position: @attributes.fetch(:index), pipeline: @pipeline, project: @pipeline.project } end diff --git a/spec/factories/ci/stages.rb b/spec/factories/ci/stages.rb index f4f73a67e9a..ce61e6bf759 100644 --- a/spec/factories/ci/stages.rb +++ b/spec/factories/ci/stages.rb @@ -21,7 +21,7 @@ FactoryBot.define do pipeline factory: :ci_empty_pipeline name 'test' - priority 1 + position 1 status 'pending' end end diff --git a/spec/lib/gitlab/background_migration/migrate_stage_index_spec.rb b/spec/lib/gitlab/background_migration/migrate_stage_index_spec.rb index d20d3ebad31..f8107dd40b9 100644 --- a/spec/lib/gitlab/background_migration/migrate_stage_index_spec.rb +++ b/spec/lib/gitlab/background_migration/migrate_stage_index_spec.rb @@ -26,10 +26,10 @@ describe Gitlab::BackgroundMigration::MigrateStageIndex, :migration, schema: 201 end 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) - expect(stages.all.pluck(:priority)).to eq [2, 3] + expect(stages.all.pluck(:position)).to eq [2, 3] end end diff --git a/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb index 7513ba15811..0edc3f315bb 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/create_spec.rb @@ -17,7 +17,7 @@ describe Gitlab::Ci::Pipeline::Chain::Create do context 'when pipeline is ready to be saved' do before do - pipeline.stages.build(name: 'test', priority: 0, project: project) + pipeline.stages.build(name: 'test', position: 0, project: project) step.perform! end diff --git a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb index b4bd0976268..05ce3412fd8 100644 --- a/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/seed/stage_spec.rb @@ -25,7 +25,7 @@ describe Gitlab::Ci::Pipeline::Seed::Stage do it 'returns hash attributes of a stage' do expect(subject.attributes).to be_a Hash expect(subject.attributes) - .to include(:name, :priority, :pipeline, :project) + .to include(:name, :position, :pipeline, :project) end end diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index f9d3116de05..62da967cf96 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -232,7 +232,7 @@ Ci::Stage: - id - name - status -- priority +- position - lock_version - project_id - pipeline_id diff --git a/spec/migrations/schedule_stages_index_migration_spec.rb b/spec/migrations/schedule_stages_index_migration_spec.rb index b8d0f711a25..710264da375 100644 --- a/spec/migrations/schedule_stages_index_migration_spec.rb +++ b/spec/migrations/schedule_stages_index_migration_spec.rb @@ -21,7 +21,7 @@ describe ScheduleStagesIndexMigration, :sidekiq, :migration do it 'schedules delayed background migrations in batches' do Sidekiq::Testing.fake! 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! diff --git a/spec/models/ci/stage_spec.rb b/spec/models/ci/stage_spec.rb index f6cc61b3e9e..a00db1d2bfc 100644 --- a/spec/models/ci/stage_spec.rb +++ b/spec/models/ci/stage_spec.rb @@ -89,9 +89,9 @@ describe Ci::Stage, :models do end 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 - stage.update_column(:priority, nil) + stage.update_column(:position, nil) end context 'when stage has statuses' do @@ -100,21 +100,21 @@ describe Ci::Stage, :models do end it 'recalculates index before updating status' do - expect(stage.reload.priority).to be_nil + expect(stage.reload.position).to be_nil stage.update_status - expect(stage.reload.priority).to eq 10 + 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.priority).to be_nil + expect(stage.reload.position).to be_nil stage.update_status - expect(stage.reload.priority).to eq 0 + expect(stage.reload.position).to eq 0 end end end