From aaccbaaeec4c0953c5e43dc8539f79c6c43db24b Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer <jacob@gitlab.com> Date: Wed, 30 Nov 2022 10:13:30 +0000 Subject: [PATCH] Remove ObjectStorage::BackgroundMove concern This concern has been a no-op since https://gitlab.com/gitlab-org/gitlab/-/merge_requests/97170. Part of https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1690. Changelog: other --- app/models/appearance.rb | 1 - app/models/bulk_imports/export_upload.rb | 1 - app/models/ci/build.rb | 1 - app/models/ci/job_artifact.rb | 1 - app/models/concerns/avatarable.rb | 1 - app/models/import_export_upload.rb | 1 - app/models/lfs_object.rb | 1 - app/models/merge_request_diff.rb | 1 - .../import_export/relation_export_upload.rb | 1 - app/uploaders/file_mover.rb | 1 - app/uploaders/file_uploader.rb | 4 -- app/uploaders/object_storage.rb | 42 ------------------- ee/app/models/ee/note.rb | 1 - .../models/user_permission_export_upload.rb | 1 - spec/models/lfs_object_spec.rb | 2 +- spec/support/rspec_order_todo.yml | 1 - spec/uploaders/file_mover_spec.rb | 6 --- 17 files changed, 1 insertion(+), 66 deletions(-) diff --git a/app/models/appearance.rb b/app/models/appearance.rb index bd948c2c32a87..03fbf0d176c38 100644 --- a/app/models/appearance.rb +++ b/app/models/appearance.rb @@ -3,7 +3,6 @@ class Appearance < ApplicationRecord include CacheableAttributes include CacheMarkdownField - include ObjectStorage::BackgroundMove include WithUploads attribute :title, default: '' diff --git a/app/models/bulk_imports/export_upload.rb b/app/models/bulk_imports/export_upload.rb index a9cba5119afe2..4304032b28c75 100644 --- a/app/models/bulk_imports/export_upload.rb +++ b/app/models/bulk_imports/export_upload.rb @@ -3,7 +3,6 @@ module BulkImports class ExportUpload < ApplicationRecord include WithUploads - include ObjectStorage::BackgroundMove self.table_name = 'bulk_import_export_uploads' diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 12c9f45e4c462..743a5b26d46b5 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -7,7 +7,6 @@ class Build < Ci::Processable include Ci::Contextable include TokenAuthenticatable include AfterCommitQueue - include ObjectStorage::BackgroundMove include Presentable include Importable include Ci::HasRef diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index 922806a21c354..af233609a3b85 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -5,7 +5,6 @@ class JobArtifact < Ci::ApplicationRecord include Ci::Partitionable include IgnorableColumns include AfterCommitQueue - include ObjectStorage::BackgroundMove include UpdateProjectStatistics include UsageStatistics include Sortable diff --git a/app/models/concerns/avatarable.rb b/app/models/concerns/avatarable.rb index b32502c3ee295..f419fa8518e7c 100644 --- a/app/models/concerns/avatarable.rb +++ b/app/models/concerns/avatarable.rb @@ -16,7 +16,6 @@ module Avatarable included do prepend ShadowMethods - include ObjectStorage::BackgroundMove include Gitlab::Utils::StrongMemoize include ApplicationHelper diff --git a/app/models/import_export_upload.rb b/app/models/import_export_upload.rb index bc363cce8dd82..bdb5365363730 100644 --- a/app/models/import_export_upload.rb +++ b/app/models/import_export_upload.rb @@ -2,7 +2,6 @@ class ImportExportUpload < ApplicationRecord include WithUploads - include ObjectStorage::BackgroundMove belongs_to :project belongs_to :group diff --git a/app/models/lfs_object.rb b/app/models/lfs_object.rb index 8aa48561e600c..e1f28c0e1179a 100644 --- a/app/models/lfs_object.rb +++ b/app/models/lfs_object.rb @@ -4,7 +4,6 @@ class LfsObject < ApplicationRecord include AfterCommitQueue include Checksummable include EachBatch - include ObjectStorage::BackgroundMove include FileStoreMounter has_many :lfs_objects_projects diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 08c56131d78cc..40b2a8d16bead 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -6,7 +6,6 @@ class MergeRequestDiff < ApplicationRecord include ManualInverseAssociation include EachBatch include Gitlab::Utils::StrongMemoize - include ObjectStorage::BackgroundMove include BulkInsertableAssociations # Don't display more than 100 commits at once diff --git a/app/models/projects/import_export/relation_export_upload.rb b/app/models/projects/import_export/relation_export_upload.rb index 965dc39d19fe8..12cfb3415d81a 100644 --- a/app/models/projects/import_export/relation_export_upload.rb +++ b/app/models/projects/import_export/relation_export_upload.rb @@ -4,7 +4,6 @@ module Projects module ImportExport class RelationExportUpload < ApplicationRecord include WithUploads - include ObjectStorage::BackgroundMove self.table_name = 'project_relation_export_uploads' diff --git a/app/uploaders/file_mover.rb b/app/uploaders/file_mover.rb index 95bc2680ed68f..92ab2d88b4157 100644 --- a/app/uploaders/file_mover.rb +++ b/app/uploaders/file_mover.rb @@ -24,7 +24,6 @@ def execute if update_markdown update_upload_model - uploader.schedule_background_upload end end diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb index 7250ce5c0b0b7..f947f70985c49 100644 --- a/app/uploaders/file_uploader.rb +++ b/app/uploaders/file_uploader.rb @@ -27,10 +27,6 @@ class FileUploader < GitlabUploader after :remove, :prune_store_dir - # FileUploader do not run in a model transaction, so we can simply - # enqueue a job after the :store hook. - after :store, :schedule_background_upload - def self.root File.join(options.storage_path, 'uploads') end diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index ac764b219b112..c07ca3b81071a 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -67,10 +67,6 @@ def upload=(upload) super end - def schedule_background_upload(*args) - # TODO remove this method https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1690 - end - def exclusive_lease_key # For FileUploaders, model may have many uploaders. In that case # we want to use exclusive key per upload, not per model to allow @@ -93,40 +89,6 @@ def current_upload_satisfies?(paths, model) end end - # Add support for automatic background uploading after the file is stored. - # - module BackgroundMove - extend ActiveSupport::Concern - - def background_upload(mount_points = []) - return unless mount_points.any? - - run_after_commit do - mount_points.each { |mount| send(mount).schedule_background_upload } # rubocop:disable GitlabSecurity/PublicSend - end - end - - def changed_mounts - self.class.uploaders.select do |mount, uploader_class| - mounted_as = uploader_class.serialization_column(self.class, mount) - uploader = send(:"#{mounted_as}") # rubocop:disable GitlabSecurity/PublicSend - - next unless uploader - next unless uploader.exists? - next unless send(:"saved_change_to_#{mounted_as}?") # rubocop:disable GitlabSecurity/PublicSend - - mount - end.keys - end - - included do - include AfterCommitQueue - after_save do - background_upload(changed_mounts) - end - end - end - module Concern extend ActiveSupport::Concern @@ -305,10 +267,6 @@ def migrate!(new_store) end end - def schedule_background_upload(*args) - # TODO remove this method https://gitlab.com/gitlab-com/gl-infra/scalability/-/issues/1690 - end - def fog_directory self.class.remote_store_path end diff --git a/ee/app/models/ee/note.rb b/ee/app/models/ee/note.rb index e495dd633db0e..d69bd11eb283b 100644 --- a/ee/app/models/ee/note.rb +++ b/ee/app/models/ee/note.rb @@ -6,7 +6,6 @@ module Note extend ::Gitlab::Utils::Override prepended do - include ::ObjectStorage::BackgroundMove include Elastic::ApplicationVersionedSearch include UsageStatistics diff --git a/ee/app/models/user_permission_export_upload.rb b/ee/app/models/user_permission_export_upload.rb index 6abf35f8c7ae6..d03c363355130 100644 --- a/ee/app/models/user_permission_export_upload.rb +++ b/ee/app/models/user_permission_export_upload.rb @@ -2,7 +2,6 @@ class UserPermissionExportUpload < ApplicationRecord include WithUploads - include ObjectStorage::BackgroundMove belongs_to :user, -> { where(admin: true) } diff --git a/spec/models/lfs_object_spec.rb b/spec/models/lfs_object_spec.rb index aedbea3cd2604..e38ffd97eb938 100644 --- a/spec/models/lfs_object_spec.rb +++ b/spec/models/lfs_object_spec.rb @@ -92,7 +92,7 @@ end end - describe '#schedule_background_upload' do + describe 'storage types' do before do stub_lfs_setting(enabled: true) end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index d2367f93d41b1..92a26a6504e7c 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -10420,7 +10420,6 @@ - './spec/uploaders/records_uploads_spec.rb' - './spec/uploaders/terraform/state_uploader_spec.rb' - './spec/uploaders/uploader_helper_spec.rb' -- './spec/uploaders/workers/object_storage/background_move_worker_spec.rb' - './spec/uploaders/workers/object_storage/migrate_uploads_worker_spec.rb' - './spec/validators/addressable_url_validator_spec.rb' - './spec/validators/any_field_validator_spec.rb' diff --git a/spec/uploaders/file_mover_spec.rb b/spec/uploaders/file_mover_spec.rb index 3b8c6f6f881cf..13a5a7e054943 100644 --- a/spec/uploaders/file_mover_spec.rb +++ b/spec/uploaders/file_mover_spec.rb @@ -57,12 +57,6 @@ .to change { tmp_upload.reload.attributes.values_at('model_id', 'model_type') } .from([user.id, 'User']).to([snippet.id, 'Snippet']) end - - it 'schedules a background migration' do - expect_any_instance_of(PersonalFileUploader).to receive(:schedule_background_upload).once - - subject - end end context 'when update_markdown fails' do -- GitLab