Follow-up from "Fix a broken background migration spec due to a missing column"
The following discussion from !40290 (merged) should be addressed:
-
@fabiopitino started a discussion: (+1 comment) @rymai I believe the problem is that we should not have a migration referencing a model that changes overtime. This would avoid having this type of ~"technical debt" in production code.
Would it work if we were having https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/background_migration/calculate_wiki_sizes.rb to be back in time with only what's needed for the migration?
module Gitlab module BackgroundMigration class CalculateWikiSizes class ProjectStatistics < ActiveRecord::Base self.table_name = 'project_statistics' def refresh_wiki_size! self.wiki_size = project.wiki.repository.size * 1.megabyte self.storage_size = repository_size + wiki_size + lfs_objects_size + build_artifacts_size + packages_size save! end end def perform(start_id, stop_id) ProjectStatistics.where(wiki_size: nil) .where(id: start_id..stop_id) .includes(project: [:route, :group, namespace: [:owner]]).find_each do |statistics| statistics.refresh_wiki_size! # we don't need to use refresh! and the before_save callback rescue => e Rails.logger.error "Failed to update wiki statistics. id: #{statistics.id} message: #{e.message}" # rubocop:disable Gitlab/RailsLogger end end end end end
I'm not saying to do it now as it's more important to fix master:broken but as an idea to make this background migration more reliable in a separate MR.