From 6714fbad8fc25d3cc6b295d26e3c220987c60fb0 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Fri, 19 Jan 2018 14:25:30 +0100
Subject: [PATCH 01/20] Remove redundant pipeline stages from the database

---
 ...121225_remove_redundant_pipeline_stages.rb | 26 +++++++++++++
 db/schema.rb                                  |  2 +-
 .../remove_redundant_pipeline_stages_spec.rb  | 39 +++++++++++++++++++
 3 files changed, 66 insertions(+), 1 deletion(-)
 create mode 100644 db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
 create mode 100644 spec/migrations/remove_redundant_pipeline_stages_spec.rb

diff --git a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
new file mode 100644
index 00000000000..3868e0adae1
--- /dev/null
+++ b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
@@ -0,0 +1,26 @@
+class RemoveRedundantPipelineStages < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+   redundant_stages_ids = <<~SQL
+      SELECT id FROM ci_stages a WHERE (
+        SELECT COUNT(*) FROM ci_stages b
+          WHERE a.pipeline_id = b.pipeline_id AND a.name = b.name
+      ) > 1
+    SQL
+
+    execute <<~SQL
+      UPDATE ci_builds SET stage_id = NULL WHERE ci_builds.stage_id IN (#{redundant_stages_ids})
+    SQL
+
+    execute <<~SQL
+      DELETE FROM ci_stages WHERE ci_stages.id IN (#{redundant_stages_ids})
+    SQL
+  end
+
+  def down
+    # noop
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index a0901833c3d..9becd794553 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180113220114) do
+ActiveRecord::Schema.define(version: 20180119121225) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
diff --git a/spec/migrations/remove_redundant_pipeline_stages_spec.rb b/spec/migrations/remove_redundant_pipeline_stages_spec.rb
new file mode 100644
index 00000000000..3bd2a9a2146
--- /dev/null
+++ b/spec/migrations/remove_redundant_pipeline_stages_spec.rb
@@ -0,0 +1,39 @@
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20180119121225_remove_redundant_pipeline_stages.rb')
+
+describe RemoveRedundantPipelineStages, :migration do
+  let(:projects) { table(:projects) }
+  let(:pipelines) { table(:ci_pipelines) }
+  let(:stages) { table(:ci_stages) }
+  let(:builds) { table(:ci_builds) }
+
+  before do
+    projects.create!(id: 123, name: 'gitlab', path: 'gitlab-ce')
+    pipelines.create!(id: 234, project_id: 123, ref: 'master', sha: 'adf43c3a')
+
+    stages.create!(id: 6, project_id: 123, pipeline_id: 234, name: 'build')
+    stages.create!(id: 10, project_id: 123, pipeline_id: 234, name: 'build')
+    stages.create!(id: 21, project_id: 123, pipeline_id: 234, name: 'build')
+    stages.create!(id: 41, project_id: 123, pipeline_id: 234, name: 'test')
+    stages.create!(id: 62, project_id: 123, pipeline_id: 234, name: 'test')
+    stages.create!(id: 102, project_id: 123, pipeline_id: 234, name: 'deploy')
+
+    builds.create!(id: 1, commit_id: 234, project_id: 123, stage_id: 10)
+    builds.create!(id: 2, commit_id: 234, project_id: 123, stage_id: 21)
+    builds.create!(id: 3, commit_id: 234, project_id: 123, stage_id: 21)
+    builds.create!(id: 4, commit_id: 234, project_id: 123, stage_id: 41)
+    builds.create!(id: 5, commit_id: 234, project_id: 123, stage_id: 62)
+    builds.create!(id: 6, commit_id: 234, project_id: 123, stage_id: 102)
+  end
+
+  it 'removes ambiguous stages and preserves builds' do
+    expect(stages.all.count).to eq 6
+    expect(builds.all.count).to eq 6
+
+    migrate!
+
+    expect(stages.all.count).to eq 1
+    expect(builds.all.count).to eq 6
+    expect(builds.all.pluck(:stage_id).compact).to eq [102]
+  end
+end

From e73333242ac42a1020319f5d491daf0647af4c54 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 24 Jan 2018 12:56:11 +0100
Subject: [PATCH 02/20] Optimize SQL query that removes duplicated stages

---
 .../20180119121225_remove_redundant_pipeline_stages.rb    | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
index 3868e0adae1..c1705cd8757 100644
--- a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
+++ b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
@@ -5,10 +5,10 @@ class RemoveRedundantPipelineStages < ActiveRecord::Migration
 
   def up
    redundant_stages_ids = <<~SQL
-      SELECT id FROM ci_stages a WHERE (
-        SELECT COUNT(*) FROM ci_stages b
-          WHERE a.pipeline_id = b.pipeline_id AND a.name = b.name
-      ) > 1
+      SELECT id FROM ci_stages WHERE (pipeline_id, name) IN (
+        SELECT pipeline_id, name FROM ci_stages
+          GROUP BY pipeline_id, name HAVING COUNT(*) > 1
+      )
     SQL
 
     execute <<~SQL

From aa290573aece6408ad8bb98c7a04257202be5ff8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 24 Jan 2018 14:26:03 +0100
Subject: [PATCH 03/20] Add a new service for creating detached CI/CD jobs

---
 app/models/commit_status.rb                   |  4 +++-
 app/services/ci/create_job_service.rb         | 12 +++++++++++
 app/services/projects/update_pages_service.rb | 18 +++++++++--------
 lib/api/commit_statuses.rb                    | 10 +++++++++-
 spec/services/ci/create_job_service_spec.rb   | 20 +++++++++++++++++++
 5 files changed, 54 insertions(+), 10 deletions(-)
 create mode 100644 app/services/ci/create_job_service.rb
 create mode 100644 spec/services/ci/create_job_service_spec.rb

diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index c0263c0b4e2..f010b0ce9db 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -50,7 +50,9 @@ class CommitStatus < ActiveRecord::Base
   ##
   # We still create some CommitStatuses outside of CreatePipelineService.
   #
-  # These are pages deployments and external statuses.
+  # These are pages deployments and external statuses. We now handle these
+  # using CreateJobService, but we still need to ensure that a job has a
+  # stage assigned. TODO, In future releases we will add model validation.
   #
   before_create unless: :importing? do
     Ci::EnsureStageService.new(project, user).execute(self) do |stage|
diff --git a/app/services/ci/create_job_service.rb b/app/services/ci/create_job_service.rb
new file mode 100644
index 00000000000..683c3557cd0
--- /dev/null
+++ b/app/services/ci/create_job_service.rb
@@ -0,0 +1,12 @@
+module Ci
+  class CreateJobService < BaseService
+    def execute(subject = nil)
+      (subject || yield).tap do |subject|
+        Ci::EnsureStageService.new(project, current_user)
+          .execute(subject)
+
+        subject.save!
+      end
+    end
+  end
+end
diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb
index a773222bf17..63d2b6c76e6 100644
--- a/app/services/projects/update_pages_service.rb
+++ b/app/services/projects/update_pages_service.rb
@@ -58,14 +58,16 @@ module Projects
     end
 
     def create_status
-      GenericCommitStatus.new(
-        project: project,
-        pipeline: build.pipeline,
-        user: build.user,
-        ref: build.ref,
-        stage: 'deploy',
-        name: 'pages:deploy'
-      )
+      Ci::CreateJobService.new(project, build.user).execute do
+        GenericCommitStatus.new(
+          project: project,
+          pipeline: build.pipeline,
+          user: build.user,
+          ref: build.ref,
+          stage: 'deploy',
+          name: 'pages:deploy'
+        )
+      end
     end
 
     def extract_archive!(temp_path)
diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb
index 829eef18795..76cb9a8c808 100644
--- a/lib/api/commit_statuses.rb
+++ b/lib/api/commit_statuses.rb
@@ -69,6 +69,7 @@ module API
         name = params[:name] || params[:context] || 'default'
 
         pipeline = @project.pipeline_for(ref, commit.sha)
+
         unless pipeline
           pipeline = @project.pipelines.create!(
             source: :external,
@@ -90,7 +91,14 @@ module API
         optional_attributes =
           attributes_for_keys(%w[target_url description coverage])
 
-        status.update(optional_attributes) if optional_attributes.any?
+        status.assign_attributes(optional_attributes) if optional_attributes.any?
+
+        if status.new_record?
+          Ci::CreateJobService.new(@project, current_user).execute(status)
+        else
+          status.save!
+        end
+
         render_validation_error!(status) if status.invalid?
 
         begin
diff --git a/spec/services/ci/create_job_service_spec.rb b/spec/services/ci/create_job_service_spec.rb
new file mode 100644
index 00000000000..a7a8032ba28
--- /dev/null
+++ b/spec/services/ci/create_job_service_spec.rb
@@ -0,0 +1,20 @@
+require 'spec_helper'
+
+describe Ci::CreateJobService, '#execute' do
+  set(:project) { create(:project, :repository) }
+  let(:user) { create(:admin) }
+  let(:status) { build(:ci_build) }
+  let(:service) { described_class.new(project, user) }
+
+  it 'persists job object instantiated in the block' do
+    expect(service.execute { status }).to be_persisted
+  end
+
+  it 'persists a job instance passed as an argument' do
+    expect(service.execute(status)).to be_persisted
+  end
+
+  it 'ensures that a job has a stage assigned' do
+    expect(service.execute(status).stage_id).to be_present
+  end
+end

From e4aac7f365105cbc7547239066db075a44d69788 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 24 Jan 2018 14:40:11 +0100
Subject: [PATCH 04/20] Do not raise an error when trying to persist a job

---
 app/models/commit_status.rb                 | 4 ++--
 app/services/ci/create_job_service.rb       | 2 +-
 lib/api/commit_statuses.rb                  | 2 +-
 spec/services/ci/create_job_service_spec.rb | 6 ++++++
 4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index f010b0ce9db..47af20975ec 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -51,8 +51,8 @@ class CommitStatus < ActiveRecord::Base
   # We still create some CommitStatuses outside of CreatePipelineService.
   #
   # These are pages deployments and external statuses. We now handle these
-  # using CreateJobService, but we still need to ensure that a job has a
-  # stage assigned. TODO, In future releases we will add model validation.
+  # jobs using CreateJobService, but we still need to ensure that a job has
+  # a stage assigned. TODO, In future releases we will add model validation.
   #
   before_create unless: :importing? do
     Ci::EnsureStageService.new(project, user).execute(self) do |stage|
diff --git a/app/services/ci/create_job_service.rb b/app/services/ci/create_job_service.rb
index 683c3557cd0..7cb77edd690 100644
--- a/app/services/ci/create_job_service.rb
+++ b/app/services/ci/create_job_service.rb
@@ -5,7 +5,7 @@ module Ci
         Ci::EnsureStageService.new(project, current_user)
           .execute(subject)
 
-        subject.save!
+        subject.save
       end
     end
   end
diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb
index 76cb9a8c808..3ec17c4d9f5 100644
--- a/lib/api/commit_statuses.rb
+++ b/lib/api/commit_statuses.rb
@@ -96,7 +96,7 @@ module API
         if status.new_record?
           Ci::CreateJobService.new(@project, current_user).execute(status)
         else
-          status.save!
+          status.save if status.changed?
         end
 
         render_validation_error!(status) if status.invalid?
diff --git a/spec/services/ci/create_job_service_spec.rb b/spec/services/ci/create_job_service_spec.rb
index a7a8032ba28..d9dd1a40325 100644
--- a/spec/services/ci/create_job_service_spec.rb
+++ b/spec/services/ci/create_job_service_spec.rb
@@ -17,4 +17,10 @@ describe Ci::CreateJobService, '#execute' do
   it 'ensures that a job has a stage assigned' do
     expect(service.execute(status).stage_id).to be_present
   end
+
+  it 'does not raise error if status is invalid' do
+    status.name = nil
+
+    expect(service.execute(status)).not_to be_persisted
+  end
 end

From ce71b1ce072b024802e7e4ff07486e253c24d964 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 24 Jan 2018 15:21:20 +0100
Subject: [PATCH 05/20] Fix removing redundant pipeline stages on MySQL

---
 ...0119121225_remove_redundant_pipeline_stages.rb | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
index c1705cd8757..597f6c3ee48 100644
--- a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
+++ b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
@@ -12,12 +12,19 @@ class RemoveRedundantPipelineStages < ActiveRecord::Migration
     SQL
 
     execute <<~SQL
-      UPDATE ci_builds SET stage_id = NULL WHERE ci_builds.stage_id IN (#{redundant_stages_ids})
+      UPDATE ci_builds SET stage_id = NULL WHERE stage_id IN (#{redundant_stages_ids})
     SQL
 
-    execute <<~SQL
-      DELETE FROM ci_stages WHERE ci_stages.id IN (#{redundant_stages_ids})
-    SQL
+    if Gitlab::Database.postgresql?
+      execute <<~SQL
+        DELETE FROM ci_stages WHERE id IN (#{redundant_stages_ids})
+      SQL
+    else # We can't modify a table we are selecting from on MySQL
+      execute <<~SQL
+        DELETE a FROM ci_stages AS a, ci_stages AS b
+          WHERE a.pipeline_id = b.pipeline_id AND a.name = b.name
+      SQL
+    end
   end
 
   def down

From b97b4d258552cadc55f408a28569c4a0d34b38a3 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 25 Jan 2018 12:55:11 +0100
Subject: [PATCH 06/20] Add an unique index on pipeline stage name

---
 ...1139_add_unique_pipeline_stage_name_index.rb | 17 +++++++++++++++++
 db/schema.rb                                    |  5 +++--
 2 files changed, 20 insertions(+), 2 deletions(-)
 create mode 100644 db/post_migrate/20180125111139_add_unique_pipeline_stage_name_index.rb

diff --git a/db/post_migrate/20180125111139_add_unique_pipeline_stage_name_index.rb b/db/post_migrate/20180125111139_add_unique_pipeline_stage_name_index.rb
new file mode 100644
index 00000000000..4fa148b2446
--- /dev/null
+++ b/db/post_migrate/20180125111139_add_unique_pipeline_stage_name_index.rb
@@ -0,0 +1,17 @@
+class AddUniquePipelineStageNameIndex < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    remove_concurrent_index :ci_stages, [:pipeline_id, :name]
+    add_concurrent_index :ci_stages, [:pipeline_id, :name], unique: true
+  end
+
+  def down
+    remove_concurrent_index :ci_stages, [:pipeline_id, :name], unique: true
+    add_concurrent_index :ci_stages, [:pipeline_id, :name]
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 9becd794553..d0802bed8f7 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180119121225) do
+ActiveRecord::Schema.define(version: 20180125111139) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -450,7 +450,8 @@ ActiveRecord::Schema.define(version: 20180119121225) do
     t.integer "lock_version"
   end
 
-  add_index "ci_stages", ["pipeline_id", "name"], name: "index_ci_stages_on_pipeline_id_and_name", using: :btree
+  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", unique: true, using: :btree
   add_index "ci_stages", ["pipeline_id"], name: "index_ci_stages_on_pipeline_id", using: :btree
   add_index "ci_stages", ["project_id"], name: "index_ci_stages_on_project_id", using: :btree
 

From 50e9824115bbefeb59319b4f20622b162c22063f Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 25 Jan 2018 13:28:18 +0100
Subject: [PATCH 07/20] Fix migration removing duplicate stages on MySQL again

---
 .../20180119121225_remove_redundant_pipeline_stages.rb           | 1 +
 1 file changed, 1 insertion(+)

diff --git a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
index 597f6c3ee48..ce38026e7e2 100644
--- a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
+++ b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
@@ -23,6 +23,7 @@ class RemoveRedundantPipelineStages < ActiveRecord::Migration
       execute <<~SQL
         DELETE a FROM ci_stages AS a, ci_stages AS b
           WHERE a.pipeline_id = b.pipeline_id AND a.name = b.name
+            AND a.id <> b.id
       SQL
     end
   end

From 9b73f14d07233af78512a93f9b8eadd696cd6fd1 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 25 Jan 2018 14:18:41 +0100
Subject: [PATCH 08/20] Fix indentation in migration removing duplicated stages

---
 .../20180119121225_remove_redundant_pipeline_stages.rb          | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
index ce38026e7e2..89ed422ea1c 100644
--- a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
+++ b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
@@ -4,7 +4,7 @@ class RemoveRedundantPipelineStages < ActiveRecord::Migration
   DOWNTIME = false
 
   def up
-   redundant_stages_ids = <<~SQL
+    redundant_stages_ids = <<~SQL
       SELECT id FROM ci_stages WHERE (pipeline_id, name) IN (
         SELECT pipeline_id, name FROM ci_stages
           GROUP BY pipeline_id, name HAVING COUNT(*) > 1

From 4e4ee368c9c02fcd52a1173df4e8ca882c644dba Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 25 Jan 2018 14:31:27 +0100
Subject: [PATCH 09/20] Remove unique index not added in a migration from
 schema

---
 db/schema.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/db/schema.rb b/db/schema.rb
index d0802bed8f7..97cbbd111c8 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -451,7 +451,6 @@ ActiveRecord::Schema.define(version: 20180125111139) do
   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", unique: true, using: :btree
   add_index "ci_stages", ["pipeline_id"], name: "index_ci_stages_on_pipeline_id", using: :btree
   add_index "ci_stages", ["project_id"], name: "index_ci_stages_on_project_id", using: :btree
 

From 48b0bc330add0fbb3398890e20a7444ba69d5f9a Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Thu, 25 Jan 2018 15:15:51 +0100
Subject: [PATCH 10/20] Fix specs for retry build after making stages unique

---
 app/services/ci/retry_build_service.rb       |  2 +-
 spec/services/ci/retry_build_service_spec.rb | 36 +++++++++++---------
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb
index c552193e66b..6128b2a8fbb 100644
--- a/app/services/ci/retry_build_service.rb
+++ b/app/services/ci/retry_build_service.rb
@@ -1,7 +1,7 @@
 module Ci
   class RetryBuildService < ::BaseService
     CLONE_ACCESSORS = %i[pipeline project ref tag options commands name
-                         allow_failure stage_id stage stage_idx trigger_request
+                         allow_failure stage stage_id stage_idx trigger_request
                          yaml_variables when environment coverage_regex
                          description tag_list protected].freeze
 
diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb
index a06397a0782..cea5c8a6c05 100644
--- a/spec/services/ci/retry_build_service_spec.rb
+++ b/spec/services/ci/retry_build_service_spec.rb
@@ -5,7 +5,11 @@ describe Ci::RetryBuildService do
   set(:project) { create(:project) }
   set(:pipeline) { create(:ci_pipeline, project: project) }
 
-  let(:build) { create(:ci_build, pipeline: pipeline) }
+  let(:stage) do
+    Ci::Stage.create!(project: project, pipeline: pipeline, name: 'test')
+  end
+
+  let(:build) { create(:ci_build, pipeline: pipeline, stage_id: stage.id) }
 
   let(:service) do
     described_class.new(project, user)
@@ -26,29 +30,27 @@ describe Ci::RetryBuildService do
        user_id auto_canceled_by_id retried failure_reason].freeze
 
   shared_examples 'build duplication' do
-    let(:stage) do
-      # TODO, we still do not have factory for new stages, we will need to
-      # switch existing factory to persist stages, instead of using LegacyStage
-      #
-      Ci::Stage.create!(project: project, pipeline: pipeline, name: 'test')
-    end
+    let(:another_pipeline) { create(:ci_empty_pipeline, project: project) }
 
     let(:build) do
       create(:ci_build, :failed, :artifacts, :expired, :erased,
              :queued, :coverage, :tags, :allowed_to_fail, :on_tag,
              :triggered, :trace, :teardown_environment,
-             description: 'my-job', stage: 'test',  pipeline: pipeline,
-             auto_canceled_by: create(:ci_empty_pipeline, project: project)) do |build|
-               ##
-               # TODO, workaround for FactoryBot limitation when having both
-               # stage (text) and stage_id (integer) columns in the table.
-               build.stage_id = stage.id
-             end
+             description: 'my-job', stage: 'test', stage_id: stage.id,
+             pipeline: pipeline, auto_canceled_by: another_pipeline)
+    end
+
+    before do
+      # Make sure that build has both `stage_id` and `stage` because FactoryBot
+      # can reset one of the fields when assigning another. We plan to deprecate
+      # and remove legacy `stage` column in the future.
+      build.update_attributes(stage: 'test', stage_id: stage.id)
     end
 
     describe 'clone accessors' do
       CLONE_ACCESSORS.each do |attribute|
         it "clones #{attribute} build attribute" do
+          expect(build.send(attribute)).not_to be_nil
           expect(new_build.send(attribute)).not_to be_nil
           expect(new_build.send(attribute)).to eq build.send(attribute)
         end
@@ -121,10 +123,12 @@ describe Ci::RetryBuildService do
 
       context 'when there are subsequent builds that are skipped' do
         let!(:subsequent_build) do
-          create(:ci_build, :skipped, stage_idx: 1, pipeline: pipeline)
+          create(:ci_build, :skipped, stage_idx: 2,
+                                      pipeline: pipeline,
+                                      stage: 'deploy')
         end
 
-        it 'resumes pipeline processing in subsequent stages' do
+        it 'resumes pipeline processing in a subsequent stage' do
           service.execute(build)
 
           expect(subsequent_build.reload).to be_created

From 5f57c7a5a54d844fb3f8566dd81da3cdde2f3c5b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 5 Feb 2018 15:32:52 +0100
Subject: [PATCH 11/20] Revert create job service because of load balancing

Currently we still need to run EnsureStageService within a transaction,
because when it runs within in a transaction we are going to stick to
the primary database when using database load balancing. Extracting this
out of the transaction makes it possible to hit into problems with
replication lag in pipeline commit status API, which can cause a lot of
trouble.
---
 app/models/commit_status.rb                   |  4 +--
 app/services/ci/create_job_service.rb         | 12 ---------
 app/services/projects/update_pages_service.rb | 18 ++++++-------
 ...121225_remove_redundant_pipeline_stages.rb | 19 +++++++++++---
 ...39_add_unique_pipeline_stage_name_index.rb | 17 ------------
 lib/api/commit_statuses.rb                    | 10 +------
 spec/services/ci/create_job_service_spec.rb   | 26 -------------------
 7 files changed, 25 insertions(+), 81 deletions(-)
 delete mode 100644 app/services/ci/create_job_service.rb
 delete mode 100644 db/post_migrate/20180125111139_add_unique_pipeline_stage_name_index.rb
 delete mode 100644 spec/services/ci/create_job_service_spec.rb

diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb
index 47af20975ec..c0263c0b4e2 100644
--- a/app/models/commit_status.rb
+++ b/app/models/commit_status.rb
@@ -50,9 +50,7 @@ class CommitStatus < ActiveRecord::Base
   ##
   # We still create some CommitStatuses outside of CreatePipelineService.
   #
-  # These are pages deployments and external statuses. We now handle these
-  # jobs using CreateJobService, but we still need to ensure that a job has
-  # a stage assigned. TODO, In future releases we will add model validation.
+  # These are pages deployments and external statuses.
   #
   before_create unless: :importing? do
     Ci::EnsureStageService.new(project, user).execute(self) do |stage|
diff --git a/app/services/ci/create_job_service.rb b/app/services/ci/create_job_service.rb
deleted file mode 100644
index 7cb77edd690..00000000000
--- a/app/services/ci/create_job_service.rb
+++ /dev/null
@@ -1,12 +0,0 @@
-module Ci
-  class CreateJobService < BaseService
-    def execute(subject = nil)
-      (subject || yield).tap do |subject|
-        Ci::EnsureStageService.new(project, current_user)
-          .execute(subject)
-
-        subject.save
-      end
-    end
-  end
-end
diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb
index 63d2b6c76e6..a773222bf17 100644
--- a/app/services/projects/update_pages_service.rb
+++ b/app/services/projects/update_pages_service.rb
@@ -58,16 +58,14 @@ module Projects
     end
 
     def create_status
-      Ci::CreateJobService.new(project, build.user).execute do
-        GenericCommitStatus.new(
-          project: project,
-          pipeline: build.pipeline,
-          user: build.user,
-          ref: build.ref,
-          stage: 'deploy',
-          name: 'pages:deploy'
-        )
-      end
+      GenericCommitStatus.new(
+        project: project,
+        pipeline: build.pipeline,
+        user: build.user,
+        ref: build.ref,
+        stage: 'deploy',
+        name: 'pages:deploy'
+      )
     end
 
     def extract_archive!(temp_path)
diff --git a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
index 89ed422ea1c..be5a80261c0 100644
--- a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
+++ b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
@@ -4,6 +4,21 @@ class RemoveRedundantPipelineStages < ActiveRecord::Migration
   DOWNTIME = false
 
   def up
+    remove_concurrent_index :ci_stages, [:pipeline_id, :name]
+
+    remove_redundant_pipeline_stages!
+
+    add_concurrent_index :ci_stages, [:pipeline_id, :name], unique: true
+  end
+
+  def down
+    remove_concurrent_index :ci_stages, [:pipeline_id, :name], unique: true
+    add_concurrent_index :ci_stages, [:pipeline_id, :name]
+  end
+
+  private
+
+  def remove_redundant_pipeline_stages!
     redundant_stages_ids = <<~SQL
       SELECT id FROM ci_stages WHERE (pipeline_id, name) IN (
         SELECT pipeline_id, name FROM ci_stages
@@ -27,8 +42,4 @@ class RemoveRedundantPipelineStages < ActiveRecord::Migration
       SQL
     end
   end
-
-  def down
-    # noop
-  end
 end
diff --git a/db/post_migrate/20180125111139_add_unique_pipeline_stage_name_index.rb b/db/post_migrate/20180125111139_add_unique_pipeline_stage_name_index.rb
deleted file mode 100644
index 4fa148b2446..00000000000
--- a/db/post_migrate/20180125111139_add_unique_pipeline_stage_name_index.rb
+++ /dev/null
@@ -1,17 +0,0 @@
-class AddUniquePipelineStageNameIndex < ActiveRecord::Migration
-  include Gitlab::Database::MigrationHelpers
-
-  DOWNTIME = false
-
-  disable_ddl_transaction!
-
-  def up
-    remove_concurrent_index :ci_stages, [:pipeline_id, :name]
-    add_concurrent_index :ci_stages, [:pipeline_id, :name], unique: true
-  end
-
-  def down
-    remove_concurrent_index :ci_stages, [:pipeline_id, :name], unique: true
-    add_concurrent_index :ci_stages, [:pipeline_id, :name]
-  end
-end
diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb
index 3ec17c4d9f5..829eef18795 100644
--- a/lib/api/commit_statuses.rb
+++ b/lib/api/commit_statuses.rb
@@ -69,7 +69,6 @@ module API
         name = params[:name] || params[:context] || 'default'
 
         pipeline = @project.pipeline_for(ref, commit.sha)
-
         unless pipeline
           pipeline = @project.pipelines.create!(
             source: :external,
@@ -91,14 +90,7 @@ module API
         optional_attributes =
           attributes_for_keys(%w[target_url description coverage])
 
-        status.assign_attributes(optional_attributes) if optional_attributes.any?
-
-        if status.new_record?
-          Ci::CreateJobService.new(@project, current_user).execute(status)
-        else
-          status.save if status.changed?
-        end
-
+        status.update(optional_attributes) if optional_attributes.any?
         render_validation_error!(status) if status.invalid?
 
         begin
diff --git a/spec/services/ci/create_job_service_spec.rb b/spec/services/ci/create_job_service_spec.rb
deleted file mode 100644
index d9dd1a40325..00000000000
--- a/spec/services/ci/create_job_service_spec.rb
+++ /dev/null
@@ -1,26 +0,0 @@
-require 'spec_helper'
-
-describe Ci::CreateJobService, '#execute' do
-  set(:project) { create(:project, :repository) }
-  let(:user) { create(:admin) }
-  let(:status) { build(:ci_build) }
-  let(:service) { described_class.new(project, user) }
-
-  it 'persists job object instantiated in the block' do
-    expect(service.execute { status }).to be_persisted
-  end
-
-  it 'persists a job instance passed as an argument' do
-    expect(service.execute(status)).to be_persisted
-  end
-
-  it 'ensures that a job has a stage assigned' do
-    expect(service.execute(status).stage_id).to be_present
-  end
-
-  it 'does not raise error if status is invalid' do
-    status.name = nil
-
-    expect(service.execute(status)).not_to be_persisted
-  end
-end

From b5c69ce3abed5f8269dee256fca02d3e74b30347 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 5 Feb 2018 16:22:19 +0100
Subject: [PATCH 12/20] Retry migration removing stages in case of duplicates

---
 ...121225_remove_redundant_pipeline_stages.rb | 21 +++++++++++++++----
 .../remove_redundant_pipeline_stages_spec.rb  |  9 ++++++++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
index be5a80261c0..9644cfabb60 100644
--- a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
+++ b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
@@ -3,12 +3,15 @@ class RemoveRedundantPipelineStages < ActiveRecord::Migration
 
   DOWNTIME = false
 
-  def up
-    remove_concurrent_index :ci_stages, [:pipeline_id, :name]
+  disable_ddl_transaction!
 
+  def up(attempts: 100)
+    remove_outdated_index!
     remove_redundant_pipeline_stages!
-
-    add_concurrent_index :ci_stages, [:pipeline_id, :name], unique: true
+    add_unique_index!
+  rescue ActiveRecord::RecordNotUnique
+    retry if (attempts -= 1) > 0
+    raise
   end
 
   def down
@@ -18,6 +21,16 @@ class RemoveRedundantPipelineStages < ActiveRecord::Migration
 
   private
 
+  def remove_outdated_index!
+    return unless index_exists?(:ci_stages, [:pipeline_id, :name])
+
+    remove_concurrent_index :ci_stages, [:pipeline_id, :name]
+  end
+
+  def add_unique_index!
+    add_concurrent_index :ci_stages, [:pipeline_id, :name], unique: true
+  end
+
   def remove_redundant_pipeline_stages!
     redundant_stages_ids = <<~SQL
       SELECT id FROM ci_stages WHERE (pipeline_id, name) IN (
diff --git a/spec/migrations/remove_redundant_pipeline_stages_spec.rb b/spec/migrations/remove_redundant_pipeline_stages_spec.rb
index 3bd2a9a2146..b2b6446f2c8 100644
--- a/spec/migrations/remove_redundant_pipeline_stages_spec.rb
+++ b/spec/migrations/remove_redundant_pipeline_stages_spec.rb
@@ -36,4 +36,13 @@ describe RemoveRedundantPipelineStages, :migration do
     expect(builds.all.count).to eq 6
     expect(builds.all.pluck(:stage_id).compact).to eq [102]
   end
+
+  it 'retries when duplicated stages are being created during migration' do
+    allow(subject).to receive(:remove_outdated_index!)
+    expect(subject).to receive(:remove_redundant_pipeline_stages!).exactly(3).times
+    allow(subject).to receive(:add_unique_index!)
+      .and_raise(ActiveRecord::RecordNotUnique.new('Duplicated stages present!'))
+
+    expect { subject.up(attempts: 3) }.to raise_error ActiveRecord::RecordNotUnique
+  end
 end

From 538ad92a1cdbf145106c6e49a6669da02fe94543 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 5 Feb 2018 16:47:34 +0100
Subject: [PATCH 13/20] Fix transactions-related race condition in stages code

---
 app/services/ci/ensure_stage_service.rb       |  7 ++-
 spec/services/ci/ensure_stage_service_spec.rb | 51 +++++++++++++++++++
 2 files changed, 57 insertions(+), 1 deletion(-)
 create mode 100644 spec/services/ci/ensure_stage_service_spec.rb

diff --git a/app/services/ci/ensure_stage_service.rb b/app/services/ci/ensure_stage_service.rb
index dc2f49e8db1..9a48f7096cf 100644
--- a/app/services/ci/ensure_stage_service.rb
+++ b/app/services/ci/ensure_stage_service.rb
@@ -7,6 +7,8 @@ module Ci
   # stage.
   #
   class EnsureStageService < BaseService
+    PipelineStageError = Class.new(StandardError)
+
     def execute(build)
       @build = build
 
@@ -22,8 +24,11 @@ module Ci
 
     private
 
-    def ensure_stage
+    def ensure_stage(attempts: 2)
       find_stage || create_stage
+    rescue ActiveRecord::RecordNotUnique
+      retry if (attempts -= 1) > 0
+      raise PipelineStageError, 'Fix me!'
     end
 
     def find_stage
diff --git a/spec/services/ci/ensure_stage_service_spec.rb b/spec/services/ci/ensure_stage_service_spec.rb
new file mode 100644
index 00000000000..47ba1e74c15
--- /dev/null
+++ b/spec/services/ci/ensure_stage_service_spec.rb
@@ -0,0 +1,51 @@
+require 'spec_helper'
+
+describe Ci::EnsureStageService, '#execute' do
+  set(:project) { create(:project) }
+  set(:user) { create(:user) }
+
+  let(:stage) { create(:ci_stage_entity) }
+  let(:job) { build(:ci_build) }
+
+  let(:service) { described_class.new(project, user) }
+
+  context 'when build has a stage assigned' do
+   it 'does not create a new stage' do
+     job.assign_attributes(stage_id: stage.id)
+
+     expect { service.execute(job) }.not_to change { Ci::Stage.count }
+   end
+  end
+
+  context 'when build does not have a stage assigned' do
+   it 'creates a new stage' do
+     job.assign_attributes(stage_id: nil, stage: 'test')
+
+     expect { service.execute(job) }.to change { Ci::Stage.count }.by(1)
+   end
+  end
+
+  context 'when build is invalid' do
+   it 'does not create a new stage' do
+     job.assign_attributes(stage_id: nil, ref: nil)
+
+     expect { service.execute(job) }.not_to change { Ci::Stage.count }
+   end
+  end
+
+  context 'when new stage can not be created because of an exception' do
+    before do
+      allow(Ci::Stage).to receive(:create!)
+        .and_raise(ActiveRecord::RecordNotUnique.new('Duplicates!'))
+    end
+
+    it 'retries up to two times' do
+      job.assign_attributes(stage_id: nil)
+
+      expect(service).to receive(:find_stage).exactly(2).times
+
+      expect { service.execute(job) }
+        .to raise_error(Ci::EnsureStageService::PipelineStageError)
+    end
+  end
+end

From b50bfb04ab7408c7f4194b589b2db2ca12679c5d Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 6 Feb 2018 09:29:40 +0100
Subject: [PATCH 14/20] Fix indentation offenses in ensure stage service

---
 spec/services/ci/ensure_stage_service_spec.rb | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/spec/services/ci/ensure_stage_service_spec.rb b/spec/services/ci/ensure_stage_service_spec.rb
index 47ba1e74c15..fa3baf3236a 100644
--- a/spec/services/ci/ensure_stage_service_spec.rb
+++ b/spec/services/ci/ensure_stage_service_spec.rb
@@ -10,27 +10,27 @@ describe Ci::EnsureStageService, '#execute' do
   let(:service) { described_class.new(project, user) }
 
   context 'when build has a stage assigned' do
-   it 'does not create a new stage' do
-     job.assign_attributes(stage_id: stage.id)
+    it 'does not create a new stage' do
+      job.assign_attributes(stage_id: stage.id)
 
-     expect { service.execute(job) }.not_to change { Ci::Stage.count }
-   end
+      expect { service.execute(job) }.not_to change { Ci::Stage.count }
+    end
   end
 
   context 'when build does not have a stage assigned' do
-   it 'creates a new stage' do
-     job.assign_attributes(stage_id: nil, stage: 'test')
+    it 'creates a new stage' do
+      job.assign_attributes(stage_id: nil, stage: 'test')
 
-     expect { service.execute(job) }.to change { Ci::Stage.count }.by(1)
-   end
+      expect { service.execute(job) }.to change { Ci::Stage.count }.by(1)
+    end
   end
 
   context 'when build is invalid' do
-   it 'does not create a new stage' do
-     job.assign_attributes(stage_id: nil, ref: nil)
+    it 'does not create a new stage' do
+      job.assign_attributes(stage_id: nil, ref: nil)
 
-     expect { service.execute(job) }.not_to change { Ci::Stage.count }
-   end
+      expect { service.execute(job) }.not_to change { Ci::Stage.count }
+    end
   end
 
   context 'when new stage can not be created because of an exception' do

From e011e291dadb00b260e74c25a64134f78d4d2177 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 6 Feb 2018 09:31:39 +0100
Subject: [PATCH 15/20] Fix database schema version to match latest migration

---
 db/schema.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/db/schema.rb b/db/schema.rb
index 97cbbd111c8..08af208e652 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20180125111139) do
+ActiveRecord::Schema.define(version: 20180119121225) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"

From e178135d57a9b06788878721316d00efa77ac4a4 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 6 Feb 2018 09:58:50 +0100
Subject: [PATCH 16/20] Add more specs for unique stages index migration

---
 ...121225_remove_redundant_pipeline_stages.rb |  5 +++-
 .../remove_redundant_pipeline_stages_spec.rb  | 23 ++++++++++++++-----
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
index 9644cfabb60..6783b717543 100644
--- a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
+++ b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
@@ -11,7 +11,10 @@ class RemoveRedundantPipelineStages < ActiveRecord::Migration
     add_unique_index!
   rescue ActiveRecord::RecordNotUnique
     retry if (attempts -= 1) > 0
-    raise
+    raise StandardError, <<~EOS
+      Failed to add an unique index to ci_stages, despite retrying the
+      migration 100 times. See gitlab-org/gitlab-ce!16580.
+    EOS
   end
 
   def down
diff --git a/spec/migrations/remove_redundant_pipeline_stages_spec.rb b/spec/migrations/remove_redundant_pipeline_stages_spec.rb
index b2b6446f2c8..8325f986594 100644
--- a/spec/migrations/remove_redundant_pipeline_stages_spec.rb
+++ b/spec/migrations/remove_redundant_pipeline_stages_spec.rb
@@ -37,12 +37,23 @@ describe RemoveRedundantPipelineStages, :migration do
     expect(builds.all.pluck(:stage_id).compact).to eq [102]
   end
 
-  it 'retries when duplicated stages are being created during migration' do
-    allow(subject).to receive(:remove_outdated_index!)
-    expect(subject).to receive(:remove_redundant_pipeline_stages!).exactly(3).times
-    allow(subject).to receive(:add_unique_index!)
-      .and_raise(ActiveRecord::RecordNotUnique.new('Duplicated stages present!'))
+  it 'retries when incorrectly added index exception is caught' do
+    allow_any_instance_of(described_class)
+      .to receive(:remove_redundant_pipeline_stages!)
 
-    expect { subject.up(attempts: 3) }.to raise_error ActiveRecord::RecordNotUnique
+    expect_any_instance_of(described_class)
+      .to receive(:remove_outdated_index!)
+      .exactly(100).times.and_call_original
+
+    expect { migrate! }
+      .to raise_error StandardError, /Failed to add an unique index/
+  end
+
+  it 'does not retry when unknown exception is being raised' do
+    allow(subject).to receive(:remove_outdated_index!)
+    expect(subject).to receive(:remove_redundant_pipeline_stages!).once
+    allow(subject).to receive(:add_unique_index!).and_raise(StandardError)
+
+    expect { subject.up(attempts: 3) }.to raise_error StandardError
   end
 end

From 708059bd9a8f6509b95f7d63c69ce4106d23d6a0 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 6 Feb 2018 10:02:33 +0100
Subject: [PATCH 17/20] Make exception in ensure stage service more descriptive

---
 app/services/ci/ensure_stage_service.rb       | 7 +++++--
 spec/services/ci/ensure_stage_service_spec.rb | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/app/services/ci/ensure_stage_service.rb b/app/services/ci/ensure_stage_service.rb
index 9a48f7096cf..775b3668c1d 100644
--- a/app/services/ci/ensure_stage_service.rb
+++ b/app/services/ci/ensure_stage_service.rb
@@ -7,7 +7,7 @@ module Ci
   # stage.
   #
   class EnsureStageService < BaseService
-    PipelineStageError = Class.new(StandardError)
+    EnsureStageError = Class.new(StandardError)
 
     def execute(build)
       @build = build
@@ -28,7 +28,10 @@ module Ci
       find_stage || create_stage
     rescue ActiveRecord::RecordNotUnique
       retry if (attempts -= 1) > 0
-      raise PipelineStageError, 'Fix me!'
+      raise EnsureStageError, <<~EOS
+        Possible bug in the database load balancing detected!
+        Please fix me!
+      EOS
     end
 
     def find_stage
diff --git a/spec/services/ci/ensure_stage_service_spec.rb b/spec/services/ci/ensure_stage_service_spec.rb
index fa3baf3236a..d17e30763d7 100644
--- a/spec/services/ci/ensure_stage_service_spec.rb
+++ b/spec/services/ci/ensure_stage_service_spec.rb
@@ -45,7 +45,7 @@ describe Ci::EnsureStageService, '#execute' do
       expect(service).to receive(:find_stage).exactly(2).times
 
       expect { service.execute(job) }
-        .to raise_error(Ci::EnsureStageService::PipelineStageError)
+        .to raise_error(Ci::EnsureStageService::EnsureStageError)
     end
   end
 end

From c30f4421749efc08051436496f2aec3269132412 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 6 Feb 2018 14:16:12 +0100
Subject: [PATCH 18/20] Improve exceptions messages in code creating stages

---
 app/services/ci/ensure_stage_service.rb                     | 6 ++++--
 .../20180119121225_remove_redundant_pipeline_stages.rb      | 5 ++++-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/app/services/ci/ensure_stage_service.rb b/app/services/ci/ensure_stage_service.rb
index 775b3668c1d..87f19b333de 100644
--- a/app/services/ci/ensure_stage_service.rb
+++ b/app/services/ci/ensure_stage_service.rb
@@ -28,9 +28,11 @@ module Ci
       find_stage || create_stage
     rescue ActiveRecord::RecordNotUnique
       retry if (attempts -= 1) > 0
+
       raise EnsureStageError, <<~EOS
-        Possible bug in the database load balancing detected!
-        Please fix me!
+        We failed to find or create a unique pipeline stage after 2 retries.
+        This should never happen and is most likely the result of a bug in
+        the database load balancing code.
       EOS
     end
 
diff --git a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
index 6783b717543..d896bb52347 100644
--- a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
+++ b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
@@ -11,9 +11,12 @@ class RemoveRedundantPipelineStages < ActiveRecord::Migration
     add_unique_index!
   rescue ActiveRecord::RecordNotUnique
     retry if (attempts -= 1) > 0
+
     raise StandardError, <<~EOS
       Failed to add an unique index to ci_stages, despite retrying the
-      migration 100 times. See gitlab-org/gitlab-ce!16580.
+      migration 100 times.
+
+      See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/16580.
     EOS
   end
 

From adb6dd067037a3df1951b5dacae37a06541ec286 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 7 Feb 2018 09:18:11 +0100
Subject: [PATCH 19/20] Remove old index after executing a query in stages
 migration

This makes it possible to heavily optimize this migration, because we
need an outdated index to remove redundant stages faster.
---
 .../20180119121225_remove_redundant_pipeline_stages.rb          | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
index d896bb52347..1962c16d84a 100644
--- a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
+++ b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
@@ -6,8 +6,8 @@ class RemoveRedundantPipelineStages < ActiveRecord::Migration
   disable_ddl_transaction!
 
   def up(attempts: 100)
-    remove_outdated_index!
     remove_redundant_pipeline_stages!
+    remove_outdated_index!
     add_unique_index!
   rescue ActiveRecord::RecordNotUnique
     retry if (attempts -= 1) > 0

From 394c1c65883883d5ac8bcbeecd9fe05b1d3fd87b Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 7 Feb 2018 13:09:53 +0100
Subject: [PATCH 20/20] Disable statement timeout when removing redundant
 stages

---
 .../20180119121225_remove_redundant_pipeline_stages.rb          | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
index 1962c16d84a..61ea85eb2a7 100644
--- a/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
+++ b/db/post_migrate/20180119121225_remove_redundant_pipeline_stages.rb
@@ -38,6 +38,8 @@ class RemoveRedundantPipelineStages < ActiveRecord::Migration
   end
 
   def remove_redundant_pipeline_stages!
+    disable_statement_timeout
+
     redundant_stages_ids = <<~SQL
       SELECT id FROM ci_stages WHERE (pipeline_id, name) IN (
         SELECT pipeline_id, name FROM ci_stages