Clean up migration code. Defining migration custom class in only post migration file which requires it for each_batch

db/post_migrate/20180430161409_migrate_legacy_artifacts_to_job_artifacts.rb

@@ -8,19 +8,32 @@ class MigrateLegacyArtifactsToJobArtifacts < ActiveRecord::Migration
 
   disable_ddl_transaction!
 
+  class Build < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'ci_builds'
+    self.inheritance_column = :_type_disabled
+
+    scope :with_legacy_artifacts, -> { where("artifacts_file <> ''") }
+
+    scope :without_new_artifacts, -> 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 = 1)')
+    end
+  end
+
   def up
     ##
-    # We add a temporary index to `artifacts_file`. If we don't have the index,
+    # We add a temporary index to the `ci_builds.artifacts_file` column.
-    # the first query (`SELECT .. WHERE .. ORDER BY id ASC LIMIT 1``) of `each_batch` will likely fail by statement timeout.
+    # Without the index, the first query (`SELECT .. WHERE .. ORDER BY id ASC LIMIT 1``) of `each_batch` will likely fail by statement timeout.
     # The following querires which will be executed in backgroun migrartions are fine without the index,
-    # because it's scanned by using `BETWEEN` clause (e.g. 'id BETWEEN 0 AND 2000') at the first place.
+    # because it's scanned by using `BETWEEN` clause (e.g. 'id BETWEEN 0 AND 2000') at the beginning and narrow down target rows.
     unless index_exists_by_name?(:ci_builds, TMP_INDEX)
       if Gitlab::Database.postgresql?
         add_concurrent_index :ci_builds, :artifacts_file, where: "artifacts_file <> ''", name: TMP_INDEX
       end
     end
 
-    ::Gitlab::BackgroundMigration::MigrateLegacyArtifacts::Build
+    MigrateLegacyArtifactsToJobArtifacts::Build
       .with_legacy_artifacts.without_new_artifacts.tap do |relation|
       queue_background_migration_jobs_by_range_at_intervals(relation,
                                                             MIGRATION,

lib/gitlab/background_migration/migrate_legacy_artifacts.rb

@@ -5,39 +5,10 @@
 module Gitlab
   module BackgroundMigration
     class MigrateLegacyArtifacts
-      class Build < ActiveRecord::Base
+      FILE_LOCAL_STORE = 1 # equal to ObjectStorage::Store::LOCAL
-        include EachBatch
+      ARCHIVE_FILE_TYPE = 1 # equal to Ci::JobArtifact.file_types['archive']
-
+      METADATA_FILE_TYPE = 2 # equal to Ci::JobArtifact.file_types['metadata']
-        self.table_name = 'ci_builds'
+      LEGACY_PATH_FILE_LOCATION = 1 # equal to Ci::JobArtifact.file_location['legacy_path']
-        self.inheritance_column = :_type_disabled
-
-        scope :with_legacy_artifacts, -> { where("artifacts_file <> ''") }
-
-        scope :without_new_artifacts, -> 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 = 1)')
-        end
-      end
-
-      class JobArtifact < ActiveRecord::Base
-        self.table_name = 'ci_job_artifacts'
-
-        LOCAL_STORE = 1 # Equavalant to ObjectStorage::Store::LOCAL
-
-        enum file_type: {
-          archive: 1,
-          metadata: 2,
-          trace: 3
-        }
-
-        ##
-        # File location of the file
-        # legacy_path: File.join(model.created_at.utc.strftime('%Y_%m'), model.project_id.to_s, model.id.to_s)
-        # hashed_path: File.join(disk_hash[0..1], disk_hash[2..3], disk_hash, creation_date, model.job_id.to_s, model.id.to_s)
-        enum file_location: {
-          hashed_path: nil,
-          legacy_path: 1
-        }
-      end
 
       def perform(start_id, stop_id)
         ActiveRecord::Base.transaction do
@@ -65,20 +36,20 @@ module Gitlab
           SELECT project_id,
                 id,
                 artifacts_expire_at,
-                #{MigrateLegacyArtifacts::JobArtifact.file_locations['legacy_path']},
+                #{LEGACY_PATH_FILE_LOCATION},
                 created_at,
                 created_at,
                 artifacts_file,
                 artifacts_size,
-                COALESCE(artifacts_file_store, #{JobArtifact::LOCAL_STORE}),
+                COALESCE(artifacts_file_store, #{FILE_LOCAL_STORE}),
-                #{MigrateLegacyArtifacts::JobArtifact.file_types['archive']}
+                #{ARCHIVE_FILE_TYPE}
             FROM ci_builds
           WHERE id BETWEEN #{start_id.to_i} AND #{stop_id.to_i}
                   AND artifacts_file <> ''
                   AND NOT EXISTS (
                     SELECT 1 FROM ci_job_artifacts
                     WHERE (ci_builds.id = ci_job_artifacts.job_id)
-                      AND ci_job_artifacts.file_type = #{MigrateLegacyArtifacts::JobArtifact.file_types['archive']})
+                      AND ci_job_artifacts.file_type = #{ARCHIVE_FILE_TYPE})
         EOF
       end
 
@@ -98,28 +69,31 @@ module Gitlab
           SELECT project_id,
                 id,
                 artifacts_expire_at,
-                #{MigrateLegacyArtifacts::JobArtifact.file_locations['legacy_path']},
+                #{LEGACY_PATH_FILE_LOCATION},
                 created_at,
                 created_at,
                 artifacts_metadata,
                 NULL,
-                COALESCE(artifacts_metadata_store, #{JobArtifact::LOCAL_STORE}),
+                COALESCE(artifacts_metadata_store, #{FILE_LOCAL_STORE}),
-                #{MigrateLegacyArtifacts::JobArtifact.file_types['metadata']}
+                #{METADATA_FILE_TYPE}
             FROM ci_builds
           WHERE id BETWEEN #{start_id.to_i} AND #{stop_id.to_i}
                   AND artifacts_file <> '' AND artifacts_metadata <> ''
                   AND NOT EXISTS (
                     SELECT 1 FROM ci_job_artifacts
                     WHERE (ci_builds.id = ci_job_artifacts.job_id)
-                      AND ci_job_artifacts.file_type = #{MigrateLegacyArtifacts::JobArtifact.file_types['metadata']})
+                      AND ci_job_artifacts.file_type = #{METADATA_FILE_TYPE})
         EOF
       end
 
       def delete_legacy_artifacts(start_id, stop_id)
         ActiveRecord::Base.connection.execute <<-EOF.strip_heredoc
-          UPDATE ci_builds SET
+          UPDATE ci_builds
-                  artifacts_file = NULL, artifacts_size = NULL, artifacts_file_store = NULL,
+             SET artifacts_file = NULL,
-                  artifacts_metadata = NULL, artifacts_metadata_store = NULL
+                 artifacts_file_store = NULL,
+                 artifacts_size = NULL,
+                 artifacts_metadata = NULL,
+                 artifacts_metadata_store = NULL
            WHERE id BETWEEN #{start_id.to_i} AND #{stop_id.to_i}
                   AND (artifacts_file <> '' OR artifacts_metadata <> '')
         EOF