From 89b4304f12cd37d8715c274cdee080e95f2d3bad Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Tue, 29 May 2018 17:06:14 +0900
Subject: [PATCH 1/7] Add background migrations to arhive legacy traces

---
 .../20180529152628_archive_legacy_traces.rb   | 44 +++++++++++
 db/schema.rb                                  |  2 +-
 .../archive_legacy_traces.rb                  | 79 +++++++++++++++++++
 .../archive_legacy_traces_spec.rb             | 60 ++++++++++++++
 spec/migrations/archive_legacy_traces_spec.rb | 45 +++++++++++
 5 files changed, 229 insertions(+), 1 deletion(-)
 create mode 100644 db/post_migrate/20180529152628_archive_legacy_traces.rb
 create mode 100644 lib/gitlab/background_migration/archive_legacy_traces.rb
 create mode 100644 spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb
 create mode 100644 spec/migrations/archive_legacy_traces_spec.rb

diff --git a/db/post_migrate/20180529152628_archive_legacy_traces.rb b/db/post_migrate/20180529152628_archive_legacy_traces.rb
new file mode 100644
index 00000000000..78ec1ab1a94
--- /dev/null
+++ b/db/post_migrate/20180529152628_archive_legacy_traces.rb
@@ -0,0 +1,44 @@
+class ArchiveLegacyTraces < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  BATCH_SIZE = 10_000
+  BACKGROUND_MIGRATION_CLASS = 'ArchiveLegacyTraces'
+
+  disable_ddl_transaction!
+
+  class Build < ActiveRecord::Base
+    include EachBatch
+    self.table_name = 'ci_builds'
+    self.inheritance_column = :_type_disabled # Disable STI
+
+    scope :finished, -> { where(status: [:success, :failed, :canceled]) }
+
+    scope :without_new_traces, ->() do
+      where('NOT EXISTS (?)',
+        ::ArchiveLegacyTraces::JobArtifact.select(1).trace.where('ci_builds.id = ci_job_artifacts.job_id'))
+    end
+  end
+
+  class JobArtifact < ActiveRecord::Base
+    self.table_name = 'ci_job_artifacts'
+
+    enum file_type: {
+      archive: 1,
+      metadata: 2,
+      trace: 3
+    }
+  end
+
+  def up
+    queue_background_migration_jobs_by_range_at_intervals(
+      ::ArchiveLegacyTraces::Build.finished.without_new_traces,
+      BACKGROUND_MIGRATION_CLASS,
+      5.minutes,
+      batch_size: BATCH_SIZE)
+  end
+
+  def down
+    # noop
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 97247387bc7..a8f8e14a3fc 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: 20180529093006) do
+ActiveRecord::Schema.define(version: 20180529152628) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
diff --git a/lib/gitlab/background_migration/archive_legacy_traces.rb b/lib/gitlab/background_migration/archive_legacy_traces.rb
new file mode 100644
index 00000000000..9741a7c181e
--- /dev/null
+++ b/lib/gitlab/background_migration/archive_legacy_traces.rb
@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+# rubocop:disable Metrics/AbcSize
+# rubocop:disable Style/Documentation
+
+module Gitlab
+  module BackgroundMigration
+    class ArchiveLegacyTraces
+      class Build < ActiveRecord::Base
+        include ::HasStatus
+
+        self.table_name = 'ci_builds'
+        self.inheritance_column = :_type_disabled # Disable STI
+
+        belongs_to :project, foreign_key: :project_id, class_name: 'ArchiveLegacyTraces::Project'
+        has_one :job_artifacts_trace, -> () { where(file_type: ArchiveLegacyTraces::JobArtifact.file_types[:trace]) }, class_name: 'ArchiveLegacyTraces::JobArtifact', foreign_key: :job_id
+        has_many :trace_chunks, foreign_key: :build_id, class_name: 'ArchiveLegacyTraces::BuildTraceChunk'
+    
+        scope :finished, -> { where(status: [:success, :failed, :canceled]) }
+
+        scope :without_new_traces, ->() do
+          finished.where('NOT EXISTS (?)',
+            BackgroundMigration::ArchiveLegacyTraces::JobArtifact.select(1).trace.where('ci_builds.id = ci_job_artifacts.job_id'))
+        end
+
+        def trace
+          ::Gitlab::Ci::Trace.new(self)
+        end
+
+        def trace=(data)
+          raise NotImplementedError
+        end
+    
+        def old_trace
+          read_attribute(:trace)
+        end
+    
+        def erase_old_trace!
+          update_column(:trace, nil)
+        end
+      end
+
+      class JobArtifact < ActiveRecord::Base
+        self.table_name = 'ci_job_artifacts'
+
+        belongs_to :build
+        belongs_to :project
+
+        mount_uploader :file, JobArtifactUploader
+
+        enum file_type: {
+          archive: 1,
+          metadata: 2,
+          trace: 3
+        }
+      end
+
+      class BuildTraceChunk < ActiveRecord::Base
+        self.table_name = 'ci_build_trace_chunks'
+
+        belongs_to :build
+      end
+
+      class Project < ActiveRecord::Base
+        self.table_name = 'projects'
+
+        has_many :builds, foreign_key: :project_id, class_name: 'ArchiveLegacyTraces::Build'
+      end
+
+      def perform(start_id, stop_id)
+        BackgroundMigration::ArchiveLegacyTraces::Build
+          .finished
+          .without_new_traces
+          .where(id: (start_id..stop_id)).find_each do |build|
+            build.trace.archive!
+          end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb b/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb
new file mode 100644
index 00000000000..ecc6eea9284
--- /dev/null
+++ b/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb
@@ -0,0 +1,60 @@
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::ArchiveLegacyTraces, :migration, schema: 20180529152628 do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:builds) { table(:ci_builds) }
+  let(:job_artifacts) { table(:ci_job_artifacts) }
+
+  before do
+    namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1')
+    projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 123)
+    build = builds.create!(id: 1, project_id: 123, status: 'success')
+
+    @legacy_trace_dir = File.join(Settings.gitlab_ci.builds_path,
+      build.created_at.utc.strftime("%Y_%m"),
+      build.project_id.to_s)
+    
+    FileUtils.mkdir_p(@legacy_trace_dir)
+
+    @legacy_trace_path = File.join(@legacy_trace_dir, "#{build.id}.log")
+  end
+
+  context 'when trace file exsits at the right place' do
+    before do
+      File.open(@legacy_trace_path, 'wb') { |stream| stream.write('aiueo') }
+    end
+
+    it 'correctly archive legacy traces' do
+      expect(job_artifacts.count).to eq(0)
+      expect(File.exist?(@legacy_trace_path)).to be_truthy
+
+      described_class.new.perform(1, 1)
+
+      expect(job_artifacts.count).to eq(1)
+      expect(File.exist?(@legacy_trace_path)).to be_falsy
+      expect(File.read(new_trace_path)).to eq('aiueo')
+    end
+  end
+
+  context 'when trace file does not exsits at the right place' do
+    it 'correctly archive legacy traces' do
+      expect(job_artifacts.count).to eq(0)
+      expect(File.exist?(@legacy_trace_path)).to be_falsy
+
+      described_class.new.perform(1, 1)
+
+      expect(job_artifacts.count).to eq(0)
+    end
+  end
+
+  def new_trace_path
+    job_artifact = job_artifacts.first
+
+    disk_hash = Digest::SHA2.hexdigest(job_artifact.project_id.to_s)
+    creation_date = job_artifact.created_at.utc.strftime('%Y_%m_%d')
+
+    File.join(Gitlab.config.artifacts.path, disk_hash[0..1], disk_hash[2..3], disk_hash,
+      creation_date, job_artifact.job_id.to_s, job_artifact.id.to_s, 'job.log')
+  end
+end
diff --git a/spec/migrations/archive_legacy_traces_spec.rb b/spec/migrations/archive_legacy_traces_spec.rb
new file mode 100644
index 00000000000..fc61c4bec17
--- /dev/null
+++ b/spec/migrations/archive_legacy_traces_spec.rb
@@ -0,0 +1,45 @@
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20180529152628_archive_legacy_traces')
+
+describe ArchiveLegacyTraces, :migration do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:builds) { table(:ci_builds) }
+  let(:job_artifacts) { table(:ci_job_artifacts) }
+
+  before do
+    namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1')
+    projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 123)
+    build = builds.create!(id: 1)
+
+    @legacy_trace_path = File.join(
+      Settings.gitlab_ci.builds_path,
+      build.created_at.utc.strftime("%Y_%m"),
+      build.project_id.to_s,
+      "#{job.id}.log"
+    )
+
+    File.open(@legacy_trace_path, 'wb') { |stream| stream.write('aiueo') }
+  end
+
+  it 'correctly archive legacy traces' do
+    expect(job_artifacts.count).to eq(0)
+    expect(File.exist?(@legacy_trace_path)).to be_truthy
+
+    migrate!
+
+    expect(job_artifacts.count).to eq(1)
+    expect(File.exist?(@legacy_trace_path)).to be_falsy
+    expect(File.exist?(new_trace_path)).to be_truthy
+  end
+
+  def new_trace_path
+    job_artifact = job_artifacts.first
+
+    disk_hash = Digest::SHA2.hexdigest(job_artifact.project_id.to_s)
+    creation_date = job_artifact.created_at.utc.strftime('%Y_%m_%d')
+
+    File.join(disk_hash[0..1], disk_hash[2..3], disk_hash,
+      creation_date, job_artifact.job_id.to_s, job_artifact.id.to_s)
+  end
+end

From b626fcc045dda7ac25b1b7d01229589d8fd00521 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Sun, 3 Jun 2018 12:08:54 +0900
Subject: [PATCH 2/7] Add changelog

---
 .../add-background-migrations-for-not-archived-traces.yml    | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 changelogs/unreleased/add-background-migrations-for-not-archived-traces.yml

diff --git a/changelogs/unreleased/add-background-migrations-for-not-archived-traces.yml b/changelogs/unreleased/add-background-migrations-for-not-archived-traces.yml
new file mode 100644
index 00000000000..b1b23c477df
--- /dev/null
+++ b/changelogs/unreleased/add-background-migrations-for-not-archived-traces.yml
@@ -0,0 +1,5 @@
+---
+title: Add background migrations for archiving legacy job traces
+merge_request: 19194
+author:
+type: performance

From 0d00d02e842a0c4b22d213e00143a08d97597000 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Sun, 3 Jun 2018 14:21:50 +0900
Subject: [PATCH 3/7] Directly refer application code from migration code

---
 app/models/ci/build.rb                        |  5 ++
 .../20180529152628_archive_legacy_traces.rb   | 19 +----
 .../archive_legacy_traces.rb                  | 77 +++----------------
 lib/tasks/gitlab/traces.rake                  |  4 +-
 .../archive_legacy_traces_spec.rb             |  2 +-
 5 files changed, 22 insertions(+), 85 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 75fd55a8f7b..6cbf9108244 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -55,6 +55,11 @@ module Ci
       where('(artifacts_file IS NOT NULL AND artifacts_file <> ?) OR EXISTS (?)',
         '', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').archive)
     end
+
+    scope :without_archived_trace, ->() do
+      where('NOT EXISTS (?)', Ci::JobArtifact.select(1).where('ci_builds.id = ci_job_artifacts.job_id').trace)
+    end
+
     scope :with_artifacts_stored_locally, -> { with_artifacts_archive.where(artifacts_file_store: [nil, LegacyArtifactUploader::Store::LOCAL]) }
     scope :with_artifacts_not_expired, ->() { with_artifacts_archive.where('artifacts_expire_at IS NULL OR artifacts_expire_at > ?', Time.now) }
     scope :with_expired_artifacts, ->() { with_artifacts_archive.where('artifacts_expire_at < ?', Time.now) }
diff --git a/db/post_migrate/20180529152628_archive_legacy_traces.rb b/db/post_migrate/20180529152628_archive_legacy_traces.rb
index 78ec1ab1a94..12f219f606c 100644
--- a/db/post_migrate/20180529152628_archive_legacy_traces.rb
+++ b/db/post_migrate/20180529152628_archive_legacy_traces.rb
@@ -2,7 +2,7 @@ class ArchiveLegacyTraces < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
   DOWNTIME = false
-  BATCH_SIZE = 10_000
+  BATCH_SIZE = 5000
   BACKGROUND_MIGRATION_CLASS = 'ArchiveLegacyTraces'
 
   disable_ddl_transaction!
@@ -14,25 +14,14 @@ class ArchiveLegacyTraces < ActiveRecord::Migration
 
     scope :finished, -> { where(status: [:success, :failed, :canceled]) }
 
-    scope :without_new_traces, ->() do
-      where('NOT EXISTS (?)',
-        ::ArchiveLegacyTraces::JobArtifact.select(1).trace.where('ci_builds.id = ci_job_artifacts.job_id'))
+    scope :without_archived_trace, -> do
+      where('NOT EXISTS (SELECT 1 FROM ci_job_artifacts WHERE ci_builds.id = ci_job_artifacts.job_id AND ci_job_artifacts.file_type = 3)')
     end
   end
 
-  class JobArtifact < ActiveRecord::Base
-    self.table_name = 'ci_job_artifacts'
-
-    enum file_type: {
-      archive: 1,
-      metadata: 2,
-      trace: 3
-    }
-  end
-
   def up
     queue_background_migration_jobs_by_range_at_intervals(
-      ::ArchiveLegacyTraces::Build.finished.without_new_traces,
+      ::ArchiveLegacyTraces::Build.finished.without_archived_trace,
       BACKGROUND_MIGRATION_CLASS,
       5.minutes,
       batch_size: BATCH_SIZE)
diff --git a/lib/gitlab/background_migration/archive_legacy_traces.rb b/lib/gitlab/background_migration/archive_legacy_traces.rb
index 9741a7c181e..0999ea95e56 100644
--- a/lib/gitlab/background_migration/archive_legacy_traces.rb
+++ b/lib/gitlab/background_migration/archive_legacy_traces.rb
@@ -5,73 +5,18 @@
 module Gitlab
   module BackgroundMigration
     class ArchiveLegacyTraces
-      class Build < ActiveRecord::Base
-        include ::HasStatus
-
-        self.table_name = 'ci_builds'
-        self.inheritance_column = :_type_disabled # Disable STI
-
-        belongs_to :project, foreign_key: :project_id, class_name: 'ArchiveLegacyTraces::Project'
-        has_one :job_artifacts_trace, -> () { where(file_type: ArchiveLegacyTraces::JobArtifact.file_types[:trace]) }, class_name: 'ArchiveLegacyTraces::JobArtifact', foreign_key: :job_id
-        has_many :trace_chunks, foreign_key: :build_id, class_name: 'ArchiveLegacyTraces::BuildTraceChunk'
-    
-        scope :finished, -> { where(status: [:success, :failed, :canceled]) }
-
-        scope :without_new_traces, ->() do
-          finished.where('NOT EXISTS (?)',
-            BackgroundMigration::ArchiveLegacyTraces::JobArtifact.select(1).trace.where('ci_builds.id = ci_job_artifacts.job_id'))
-        end
-
-        def trace
-          ::Gitlab::Ci::Trace.new(self)
-        end
-
-        def trace=(data)
-          raise NotImplementedError
-        end
-    
-        def old_trace
-          read_attribute(:trace)
-        end
-    
-        def erase_old_trace!
-          update_column(:trace, nil)
-        end
-      end
-
-      class JobArtifact < ActiveRecord::Base
-        self.table_name = 'ci_job_artifacts'
-
-        belongs_to :build
-        belongs_to :project
-
-        mount_uploader :file, JobArtifactUploader
-
-        enum file_type: {
-          archive: 1,
-          metadata: 2,
-          trace: 3
-        }
-      end
-
-      class BuildTraceChunk < ActiveRecord::Base
-        self.table_name = 'ci_build_trace_chunks'
-
-        belongs_to :build
-      end
-
-      class Project < ActiveRecord::Base
-        self.table_name = 'projects'
-
-        has_many :builds, foreign_key: :project_id, class_name: 'ArchiveLegacyTraces::Build'
-      end
-
       def perform(start_id, stop_id)
-        BackgroundMigration::ArchiveLegacyTraces::Build
-          .finished
-          .without_new_traces
-          .where(id: (start_id..stop_id)).find_each do |build|
-            build.trace.archive!
+        # This background migrations directly refer ::Ci::Build model which is defined in application code.
+        # In general, migration code should be isolated as much as possible in order to be idempotent.
+        # However, `archive!` logic is too complicated to be replicated. So we chose a way to refer ::Ci::Build directly
+        # and we don't change the `archive!` logic until 11.1
+        ::Ci::Build.finished.without_archived_trace
+          .where(id: start_id..stop_id).find_each do |build|
+            begin
+              build.trace.archive!
+            rescue => e
+              Rails.logger.error "Failed to archive live trace. id: #{build.id} message: #{e.message}"
+            end
           end
       end
     end
diff --git a/lib/tasks/gitlab/traces.rake b/lib/tasks/gitlab/traces.rake
index fd2a4f2d11a..ddcca69711f 100644
--- a/lib/tasks/gitlab/traces.rake
+++ b/lib/tasks/gitlab/traces.rake
@@ -8,9 +8,7 @@ namespace :gitlab do
       logger = Logger.new(STDOUT)
       logger.info('Archiving legacy traces')
 
-      Ci::Build.finished
-        .where('NOT EXISTS (?)',
-          Ci::JobArtifact.select(1).trace.where('ci_builds.id = ci_job_artifacts.job_id'))
+      Ci::Build.finished.without_archived_trace
         .order(id: :asc)
         .find_in_batches(batch_size: 1000) do |jobs|
         job_ids = jobs.map { |job| [job.id] }
diff --git a/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb b/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb
index ecc6eea9284..1a62b30ce81 100644
--- a/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb
+++ b/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb
@@ -14,7 +14,7 @@ describe Gitlab::BackgroundMigration::ArchiveLegacyTraces, :migration, schema: 2
     @legacy_trace_dir = File.join(Settings.gitlab_ci.builds_path,
       build.created_at.utc.strftime("%Y_%m"),
       build.project_id.to_s)
-    
+
     FileUtils.mkdir_p(@legacy_trace_dir)
 
     @legacy_trace_path = File.join(@legacy_trace_dir, "#{build.id}.log")

From bcd664f53a4009bc752fbc47e1c4d6f76c0b8cc2 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Sun, 3 Jun 2018 15:04:47 +0900
Subject: [PATCH 4/7] Fix specs. Rename migration file name which was
 conflicted with background migration's.

---
 ...2628_schedule_to_archive_legacy_traces.rb} |  4 +-
 .../archive_legacy_traces_spec.rb             | 35 ++++-----------
 spec/migrations/archive_legacy_traces_spec.rb | 45 -------------------
 .../schedule_to_archive_legacy_traces_spec.rb | 45 +++++++++++++++++++
 spec/support/trace/trace_helpers.rb           | 23 ++++++++++
 5 files changed, 78 insertions(+), 74 deletions(-)
 rename db/post_migrate/{20180529152628_archive_legacy_traces.rb => 20180529152628_schedule_to_archive_legacy_traces.rb} (84%)
 delete mode 100644 spec/migrations/archive_legacy_traces_spec.rb
 create mode 100644 spec/migrations/schedule_to_archive_legacy_traces_spec.rb
 create mode 100644 spec/support/trace/trace_helpers.rb

diff --git a/db/post_migrate/20180529152628_archive_legacy_traces.rb b/db/post_migrate/20180529152628_schedule_to_archive_legacy_traces.rb
similarity index 84%
rename from db/post_migrate/20180529152628_archive_legacy_traces.rb
rename to db/post_migrate/20180529152628_schedule_to_archive_legacy_traces.rb
index 12f219f606c..ea782e1596b 100644
--- a/db/post_migrate/20180529152628_archive_legacy_traces.rb
+++ b/db/post_migrate/20180529152628_schedule_to_archive_legacy_traces.rb
@@ -1,4 +1,4 @@
-class ArchiveLegacyTraces < ActiveRecord::Migration
+class ScheduleToArchiveLegacyTraces < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
   DOWNTIME = false
@@ -21,7 +21,7 @@ class ArchiveLegacyTraces < ActiveRecord::Migration
 
   def up
     queue_background_migration_jobs_by_range_at_intervals(
-      ::ArchiveLegacyTraces::Build.finished.without_archived_trace,
+      ::ScheduleToArchiveLegacyTraces::Build.finished.without_archived_trace,
       BACKGROUND_MIGRATION_CLASS,
       5.minutes,
       batch_size: BATCH_SIZE)
diff --git a/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb b/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb
index 1a62b30ce81..0765f4149f9 100644
--- a/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb
+++ b/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe Gitlab::BackgroundMigration::ArchiveLegacyTraces, :migration, schema: 20180529152628 do
+  include TraceHelpers
+
   let(:namespaces) { table(:namespaces) }
   let(:projects) { table(:projects) }
   let(:builds) { table(:ci_builds) }
@@ -9,52 +11,31 @@ describe Gitlab::BackgroundMigration::ArchiveLegacyTraces, :migration, schema: 2
   before do
     namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1')
     projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 123)
-    build = builds.create!(id: 1, project_id: 123, status: 'success')
-
-    @legacy_trace_dir = File.join(Settings.gitlab_ci.builds_path,
-      build.created_at.utc.strftime("%Y_%m"),
-      build.project_id.to_s)
-
-    FileUtils.mkdir_p(@legacy_trace_dir)
-
-    @legacy_trace_path = File.join(@legacy_trace_dir, "#{build.id}.log")
+    @build = builds.create!(id: 1, project_id: 123, status: 'success', type: 'Ci::Build')
   end
 
   context 'when trace file exsits at the right place' do
     before do
-      File.open(@legacy_trace_path, 'wb') { |stream| stream.write('aiueo') }
+      create_legacy_trace(@build, 'aiueo')
     end
 
     it 'correctly archive legacy traces' do
       expect(job_artifacts.count).to eq(0)
-      expect(File.exist?(@legacy_trace_path)).to be_truthy
+      expect(File.exist?(legacy_trace_path(@build))).to be_truthy
 
       described_class.new.perform(1, 1)
 
       expect(job_artifacts.count).to eq(1)
-      expect(File.exist?(@legacy_trace_path)).to be_falsy
-      expect(File.read(new_trace_path)).to eq('aiueo')
+      expect(File.exist?(legacy_trace_path(@build))).to be_falsy
+      expect(File.read(archived_trace_path(job_artifacts.first))).to eq('aiueo')
     end
   end
 
   context 'when trace file does not exsits at the right place' do
-    it 'correctly archive legacy traces' do
-      expect(job_artifacts.count).to eq(0)
-      expect(File.exist?(@legacy_trace_path)).to be_falsy
-
+    it 'does not raise errors and create job artifact row' do
       described_class.new.perform(1, 1)
 
       expect(job_artifacts.count).to eq(0)
     end
   end
-
-  def new_trace_path
-    job_artifact = job_artifacts.first
-
-    disk_hash = Digest::SHA2.hexdigest(job_artifact.project_id.to_s)
-    creation_date = job_artifact.created_at.utc.strftime('%Y_%m_%d')
-
-    File.join(Gitlab.config.artifacts.path, disk_hash[0..1], disk_hash[2..3], disk_hash,
-      creation_date, job_artifact.job_id.to_s, job_artifact.id.to_s, 'job.log')
-  end
 end
diff --git a/spec/migrations/archive_legacy_traces_spec.rb b/spec/migrations/archive_legacy_traces_spec.rb
deleted file mode 100644
index fc61c4bec17..00000000000
--- a/spec/migrations/archive_legacy_traces_spec.rb
+++ /dev/null
@@ -1,45 +0,0 @@
-require 'spec_helper'
-require Rails.root.join('db', 'post_migrate', '20180529152628_archive_legacy_traces')
-
-describe ArchiveLegacyTraces, :migration do
-  let(:namespaces) { table(:namespaces) }
-  let(:projects) { table(:projects) }
-  let(:builds) { table(:ci_builds) }
-  let(:job_artifacts) { table(:ci_job_artifacts) }
-
-  before do
-    namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1')
-    projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 123)
-    build = builds.create!(id: 1)
-
-    @legacy_trace_path = File.join(
-      Settings.gitlab_ci.builds_path,
-      build.created_at.utc.strftime("%Y_%m"),
-      build.project_id.to_s,
-      "#{job.id}.log"
-    )
-
-    File.open(@legacy_trace_path, 'wb') { |stream| stream.write('aiueo') }
-  end
-
-  it 'correctly archive legacy traces' do
-    expect(job_artifacts.count).to eq(0)
-    expect(File.exist?(@legacy_trace_path)).to be_truthy
-
-    migrate!
-
-    expect(job_artifacts.count).to eq(1)
-    expect(File.exist?(@legacy_trace_path)).to be_falsy
-    expect(File.exist?(new_trace_path)).to be_truthy
-  end
-
-  def new_trace_path
-    job_artifact = job_artifacts.first
-
-    disk_hash = Digest::SHA2.hexdigest(job_artifact.project_id.to_s)
-    creation_date = job_artifact.created_at.utc.strftime('%Y_%m_%d')
-
-    File.join(disk_hash[0..1], disk_hash[2..3], disk_hash,
-      creation_date, job_artifact.job_id.to_s, job_artifact.id.to_s)
-  end
-end
diff --git a/spec/migrations/schedule_to_archive_legacy_traces_spec.rb b/spec/migrations/schedule_to_archive_legacy_traces_spec.rb
new file mode 100644
index 00000000000..d3eac3c45ea
--- /dev/null
+++ b/spec/migrations/schedule_to_archive_legacy_traces_spec.rb
@@ -0,0 +1,45 @@
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20180529152628_schedule_to_archive_legacy_traces')
+
+describe ScheduleToArchiveLegacyTraces, :migration do
+  include TraceHelpers
+
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:builds) { table(:ci_builds) }
+  let(:job_artifacts) { table(:ci_job_artifacts) }
+
+  before do
+    namespaces.create!(id: 123, name: 'gitlab1', path: 'gitlab1')
+    projects.create!(id: 123, name: 'gitlab1', path: 'gitlab1', namespace_id: 123)
+    @build_success = builds.create!(id: 1, project_id: 123, status: 'success', type: 'Ci::Build')
+    @build_failed = builds.create!(id: 2, project_id: 123, status: 'failed', type: 'Ci::Build')
+    @builds_canceled = builds.create!(id: 3, project_id: 123, status: 'canceled', type: 'Ci::Build')
+    @build_running = builds.create!(id: 4, project_id: 123, status: 'running', type: 'Ci::Build')
+
+    create_legacy_trace(@build_success, 'This job is done')
+    create_legacy_trace(@build_failed, 'This job is done')
+    create_legacy_trace(@builds_canceled, 'This job is done')
+    create_legacy_trace(@build_running, 'This job is not done yet')
+  end
+
+  it 'correctly archive legacy traces' do
+    expect(job_artifacts.count).to eq(0)
+    expect(File.exist?(legacy_trace_path(@build_success))).to be_truthy
+    expect(File.exist?(legacy_trace_path(@build_failed))).to be_truthy
+    expect(File.exist?(legacy_trace_path(@builds_canceled))).to be_truthy
+    expect(File.exist?(legacy_trace_path(@build_running))).to be_truthy
+
+    migrate!
+
+    expect(job_artifacts.count).to eq(3)
+    expect(File.exist?(legacy_trace_path(@build_success))).to be_falsy
+    expect(File.exist?(legacy_trace_path(@build_failed))).to be_falsy
+    expect(File.exist?(legacy_trace_path(@builds_canceled))).to be_falsy
+    expect(File.exist?(legacy_trace_path(@build_running))).to be_truthy
+    expect(File.exist?(archived_trace_path(job_artifacts.where(job_id: @build_success.id).first))).to be_truthy
+    expect(File.exist?(archived_trace_path(job_artifacts.where(job_id: @build_failed.id).first))).to be_truthy
+    expect(File.exist?(archived_trace_path(job_artifacts.where(job_id: @builds_canceled.id).first))).to be_truthy
+    expect(job_artifacts.where(job_id: @build_running.id)).not_to be_exist
+  end
+end
diff --git a/spec/support/trace/trace_helpers.rb b/spec/support/trace/trace_helpers.rb
new file mode 100644
index 00000000000..f6d11b61038
--- /dev/null
+++ b/spec/support/trace/trace_helpers.rb
@@ -0,0 +1,23 @@
+module TraceHelpers
+  def create_legacy_trace(build, content)
+    File.open(legacy_trace_path(build), 'wb') { |stream| stream.write(content) }
+  end
+
+  def legacy_trace_path(build)
+    legacy_trace_dir = File.join(Settings.gitlab_ci.builds_path,
+      build.created_at.utc.strftime("%Y_%m"),
+      build.project_id.to_s)
+
+    FileUtils.mkdir_p(legacy_trace_dir)
+
+    File.join(legacy_trace_dir, "#{build.id}.log")
+  end
+
+  def archived_trace_path(job_artifact)
+    disk_hash = Digest::SHA2.hexdigest(job_artifact.project_id.to_s)
+    creation_date = job_artifact.created_at.utc.strftime('%Y_%m_%d')
+
+    File.join(Gitlab.config.artifacts.path, disk_hash[0..1], disk_hash[2..3], disk_hash,
+      creation_date, job_artifact.job_id.to_s, job_artifact.id.to_s, 'job.log')
+  end
+end

From 623accf9cb3babb81d789a013e6ad87ad0bf26e5 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Sun, 3 Jun 2018 15:09:49 +0900
Subject: [PATCH 5/7] Add type_build to ScheduleToArchiveLegacyTraces::Build

---
 .../20180529152628_schedule_to_archive_legacy_traces.rb       | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/db/post_migrate/20180529152628_schedule_to_archive_legacy_traces.rb b/db/post_migrate/20180529152628_schedule_to_archive_legacy_traces.rb
index ea782e1596b..965cd3a8714 100644
--- a/db/post_migrate/20180529152628_schedule_to_archive_legacy_traces.rb
+++ b/db/post_migrate/20180529152628_schedule_to_archive_legacy_traces.rb
@@ -12,6 +12,8 @@ class ScheduleToArchiveLegacyTraces < ActiveRecord::Migration
     self.table_name = 'ci_builds'
     self.inheritance_column = :_type_disabled # Disable STI
 
+    scope :type_build, -> { where(type: 'Ci::Build') }
+
     scope :finished, -> { where(status: [:success, :failed, :canceled]) }
 
     scope :without_archived_trace, -> do
@@ -21,7 +23,7 @@ class ScheduleToArchiveLegacyTraces < ActiveRecord::Migration
 
   def up
     queue_background_migration_jobs_by_range_at_intervals(
-      ::ScheduleToArchiveLegacyTraces::Build.finished.without_archived_trace,
+      ::ScheduleToArchiveLegacyTraces::Build.type_build.finished.without_archived_trace,
       BACKGROUND_MIGRATION_CLASS,
       5.minutes,
       batch_size: BATCH_SIZE)

From 2184c753fd81925c74a3a30abe9be187aa8c4133 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Sun, 3 Jun 2018 15:13:36 +0900
Subject: [PATCH 6/7] Revise comments in ArchiveLegacyTraces

---
 lib/gitlab/background_migration/archive_legacy_traces.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/gitlab/background_migration/archive_legacy_traces.rb b/lib/gitlab/background_migration/archive_legacy_traces.rb
index 0999ea95e56..5a4e5b2c471 100644
--- a/lib/gitlab/background_migration/archive_legacy_traces.rb
+++ b/lib/gitlab/background_migration/archive_legacy_traces.rb
@@ -6,10 +6,10 @@ module Gitlab
   module BackgroundMigration
     class ArchiveLegacyTraces
       def perform(start_id, stop_id)
-        # This background migrations directly refer ::Ci::Build model which is defined in application code.
+        # This background migration directly refers to ::Ci::Build model which is defined in application code.
         # In general, migration code should be isolated as much as possible in order to be idempotent.
-        # However, `archive!` logic is too complicated to be replicated. So we chose a way to refer ::Ci::Build directly
-        # and we don't change the `archive!` logic until 11.1
+        # However, `archive!` method is too complicated to be replicated by coping its subsequent code.
+        # So we chose a way to use ::Ci::Build directly and we don't change the `archive!` method until 11.1
         ::Ci::Build.finished.without_archived_trace
           .where(id: start_id..stop_id).find_each do |build|
             begin

From 8f1f73d4e3ca82e3d449e478606f133d19ead7b1 Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Mon, 4 Jun 2018 14:28:21 +0900
Subject: [PATCH 7/7] Fix typo in spec. Add a test for the case of when trace
 is stored in database

---
 .../archive_legacy_traces_spec.rb             | 26 ++++++++++++++++---
 spec/support/trace/trace_helpers.rb           |  4 +++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb b/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb
index 0765f4149f9..877c061d11b 100644
--- a/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb
+++ b/spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb
@@ -16,7 +16,7 @@ describe Gitlab::BackgroundMigration::ArchiveLegacyTraces, :migration, schema: 2
 
   context 'when trace file exsits at the right place' do
     before do
-      create_legacy_trace(@build, 'aiueo')
+      create_legacy_trace(@build, 'trace in file')
     end
 
     it 'correctly archive legacy traces' do
@@ -27,15 +27,33 @@ describe Gitlab::BackgroundMigration::ArchiveLegacyTraces, :migration, schema: 2
 
       expect(job_artifacts.count).to eq(1)
       expect(File.exist?(legacy_trace_path(@build))).to be_falsy
-      expect(File.read(archived_trace_path(job_artifacts.first))).to eq('aiueo')
+      expect(File.read(archived_trace_path(job_artifacts.first))).to eq('trace in file')
     end
   end
 
   context 'when trace file does not exsits at the right place' do
-    it 'does not raise errors and create job artifact row' do
-      described_class.new.perform(1, 1)
+    it 'does not raise errors nor create job artifact' do
+      expect { described_class.new.perform(1, 1) }.not_to raise_error
 
       expect(job_artifacts.count).to eq(0)
     end
   end
+
+  context 'when trace data exsits in database' do
+    before do
+      create_legacy_trace_in_db(@build, 'trace in db')
+    end
+
+    it 'correctly archive legacy traces' do
+      expect(job_artifacts.count).to eq(0)
+      expect(@build.read_attribute(:trace)).not_to be_empty
+
+      described_class.new.perform(1, 1)
+
+      @build.reload
+      expect(job_artifacts.count).to eq(1)
+      expect(@build.read_attribute(:trace)).to be_nil
+      expect(File.read(archived_trace_path(job_artifacts.first))).to eq('trace in db')
+    end
+  end
 end
diff --git a/spec/support/trace/trace_helpers.rb b/spec/support/trace/trace_helpers.rb
index f6d11b61038..c7802bbcb94 100644
--- a/spec/support/trace/trace_helpers.rb
+++ b/spec/support/trace/trace_helpers.rb
@@ -3,6 +3,10 @@ module TraceHelpers
     File.open(legacy_trace_path(build), 'wb') { |stream| stream.write(content) }
   end
 
+  def create_legacy_trace_in_db(build, content)
+    build.update_column(:trace, content)
+  end
+
   def legacy_trace_path(build)
     legacy_trace_dir = File.join(Settings.gitlab_ci.builds_path,
       build.created_at.utc.strftime("%Y_%m"),