diff options
author | Micaël Bergeron <mbergeron@gitlab.com> | 2018-02-28 10:44:34 -0500 |
---|---|---|
committer | Micaël Bergeron <mbergeron@gitlab.com> | 2018-03-01 10:40:40 -0500 |
commit | d59210de58dc8a377130cfdd3fc3197a4ca7bb1d (patch) | |
tree | e95089290704b2c64a09ef5e70d3a616770f4db7 /app/uploaders | |
parent | a3e46d9b68ebe2b4c78425fa3a77ebcb3133eef0 (diff) | |
download | gitlab-ce-d59210de58dc8a377130cfdd3fc3197a4ca7bb1d.tar.gz |
Merge branch 'fix/sm/atomic-migration' into 'master'
Fix migrate! method (Minimal fix with ExclusiveLock to prevent race conditions)
Closes #4928 and #4980
See merge request gitlab-org/gitlab-ee!4624
Diffstat (limited to 'app/uploaders')
-rw-r--r-- | app/uploaders/object_storage.rb | 78 |
1 files changed, 51 insertions, 27 deletions
diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index 43d881e5aba..1880cd100dc 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -88,7 +88,13 @@ module ObjectStorage def changed_mounts self.class.uploaders.select do |mount, uploader_class| mounted_as = uploader_class.serialization_column(self.class, mount) - mount if send(:"#{mounted_as}_changed?") # rubocop:disable GitlabSecurity/PublicSend + uploader = send(:"#{mounted_as}") # rubocop:disable GitlabSecurity/PublicSend + + next unless uploader + next unless uploader.exists? + next unless send(:"#{mounted_as}_changed?") # rubocop:disable GitlabSecurity/PublicSend + + mount end.keys end @@ -164,7 +170,7 @@ module ObjectStorage return unless persist_object_store? updated = model.update_column(store_serialization_column, object_store) - raise ActiveRecordError unless updated + raise 'Failed to update object store' unless updated end def use_file @@ -190,32 +196,12 @@ module ObjectStorage # new_store: Enum (Store::LOCAL, Store::REMOTE) # def migrate!(new_store) - return unless object_store != new_store - return unless file - - new_file = nil - file_to_delete = file - from_object_store = object_store - self.object_store = new_store # changes the storage and file - - cache_stored_file! if file_storage? + uuid = Gitlab::ExclusiveLease.new(exclusive_lease_key, timeout: 1.hour.to_i).try_obtain + raise 'Already running' unless uuid - with_callbacks(:migrate, file_to_delete) do - with_callbacks(:store, file_to_delete) do # for #store_versions! - new_file = storage.store!(file) - persist_object_store! - self.file = new_file - end - end - - file - rescue => e - # in case of failure delete new file - new_file.delete unless new_file.nil? - # revert back to the old file - self.object_store = from_object_store - self.file = file_to_delete - raise e + unsafe_migrate!(new_store) + ensure + Gitlab::ExclusiveLease.cancel(exclusive_lease_key, uuid) end def schedule_background_upload(*args) @@ -298,5 +284,43 @@ module ObjectStorage raise UnknownStoreError end end + + def exclusive_lease_key + "object_storage_migrate:#{model.class}:#{model.id}" + end + + # + # Move the file to another store + # + # new_store: Enum (Store::LOCAL, Store::REMOTE) + # + def unsafe_migrate!(new_store) + return unless object_store != new_store + return unless file + + new_file = nil + file_to_delete = file + from_object_store = object_store + self.object_store = new_store # changes the storage and file + + cache_stored_file! if file_storage? + + with_callbacks(:migrate, file_to_delete) do + with_callbacks(:store, file_to_delete) do # for #store_versions! + new_file = storage.store!(file) + persist_object_store! + self.file = new_file + end + end + + file + rescue => e + # in case of failure delete new file + new_file.delete unless new_file.nil? + # revert back to the old file + self.object_store = from_object_store + self.file = file_to_delete + raise e + end end end |