diff options
author | Shinya Maeda <shinya@gitlab.com> | 2018-05-03 11:08:05 +0300 |
---|---|---|
committer | Shinya Maeda <shinya@gitlab.com> | 2018-05-03 11:08:05 +0300 |
commit | c80949fdcdb60003fa3d7dc6e1d7008c4cff5b5d (patch) | |
tree | e34db6bd931812c22433dbe102e57036fab05a09 /app/models/concerns/fast_destroy_all.rb | |
parent | b97d08494cab30a7b0168091f7b3fc17527cfda3 (diff) |
Simplify FastDestroyAll module
Diffstat (limited to 'app/models/concerns/fast_destroy_all.rb')
-rw-r--r-- | app/models/concerns/fast_destroy_all.rb | 94 |
1 files changed, 48 insertions, 46 deletions
diff --git a/app/models/concerns/fast_destroy_all.rb b/app/models/concerns/fast_destroy_all.rb index 5df23a74a41..6954db258bb 100644 --- a/app/models/concerns/fast_destroy_all.rb +++ b/app/models/concerns/fast_destroy_all.rb @@ -4,23 +4,33 @@ # In general, `destroy_all` is inefficient because it calls each callback with `DELETE` queries i.e. O(n), whereas, # `delete_all` is efficient as it deletes all rows with a single `DELETE` query. # -# It's better to use `delete_all` as much as possible, however, in the real cases, it's hard to adopt because -# in some cases, external data (in ObjectStorage/FileStorage/Redis) is assosiated with rows. +# It's better to use `delete_all` as our best practice, however, +# if external data (e.g. ObjectStorage, FileStorage or Redis) are assosiated with database records, +# it is difficult to accomplish it. # -# This module introduces a protocol to support the adoption with easy way. -# You can use in the following scnenes -# - When calling `destroy_all` explicitly -# e.g. `job.trace_chunks.fast_destroy_all`` -# - When a parent record is deleted and all children are deleted subsequently (cascade delete) -# e.g. `before_destroy -> { run_after_commit(&build_trace_chunks.after_commit_cleanup_proc) }`` +# This module defines a format to use `delete_all` and delete associated external data. +# Here is an exmaple +# +# Situation +# - `Project` has many `Ci::BuildTraceChunk` through `Ci::Build` +# - `Ci::BuildTraceChunk` stores associated data in Redis, so it relies on `dependent: :destroy` and `before_destroy` for the deletion +# +# How to use +# - Define `use_fast_destroy :build_trace_chunks` in `Project` model. +# - Define `begin_fast_destroy` and `finalize_fast_destroy(params)` in `Ci::BuildTraceChunk` model. +# - Use `fast_destroy_all` instead of `destroy` and `destroy_all` +# - Remove `dependent: :destroy` and `before_destroy` as it's no longer need +# +# Expectation +# - When a project is `destroy`ed, the associated trace_chunks will be deleted by `delete_all`, +# and the associated data will be removed, too. +# - When `fast_destroy_all` is called, it also performns as same. module FastDestroyAll extend ActiveSupport::Concern ForbiddenActionError = Class.new(StandardError) included do - class_attribute :_delete_method, :_delete_params_generator - before_destroy do raise ForbiddenActionError, '`destroy` and `destroy_all` are forbbiden. Please use `fast_destroy_all`' end @@ -28,40 +38,31 @@ module FastDestroyAll class_methods do ## - # This method is for registering :delete_method and :delete_params_generator - # You have to define the method if you want to use `FastDestroyAll`` module. + # This method delete rows and associated external data efficiently # - # e.g. fast_destroy_all_with :delete_all_redis_data, :redis_all_data_keys - def fast_destroy_all_with(delete_method, delete_params_generator) - self._delete_method = delete_method - self._delete_params_generator = delete_params_generator + # This method can replace `destroy` and `destroy_all` withtout having `after_destroy` hook + def fast_destroy_all + params = begin_fast_destroy + + delete_all + + finalize_fast_destroy(params) end ## - # This method generates a proc to delete external data. - # It's useful especially when you want to hook parent record's deletion event. + # This method returns identifiers to delete associated external data (e.g. file paths, redis keys) # - # e.g. before_destroy -> { run_after_commit(&build_trace_chunks.after_commit_cleanup_proc) } - def after_commit_cleanup_proc - params = send self._delete_params_generator # rubocop:disable GitlabSecurity/PublicSend - subject = self # Preserve the subject class, otherwise `proc` uses a different class - - proc do - subject.send subject._delete_method, params # rubocop:disable GitlabSecurity/PublicSend - - true - end + # This method must be defined in fast destroyable model + def begin_fast_destroy + raise NotImplementedError end ## - # This method deletes all rows at first and delete all external data at second. - # Before deleting the rows, it generates a proc to delete external data. - # So it won't lose the track of deleting external data, even if it happened after rows had been deleted. - def fast_destroy_all - after_commit_cleanup_proc.tap do |delete_all_external_data| - delete_all - delete_all_external_data.call - end + # This method deletes associated external data with the identifiers returned by `begin_fast_destroy` + # + # This method must be defined in fast destroyable model + def finalize_fast_destroy(params) + raise NotImplementedError end end @@ -70,20 +71,21 @@ module FastDestroyAll class_methods do ## - # This is a helper method for performaning fast_destroy_all when parent relations are deleted - # Their children must include `FastDestroyAll` module. - # - # `use_fast_destroy` must be defined **before** `has_many` and `has_one`, such as `has_many :relation, dependent: :destroy` - # Otherwise `use_fast_destroy` performs against **deleted** rows, which fails to get identifiers of external data - # - # e.g. use_fast_destroy :build_trace_chunks + # This method is to be defined on models which have fast destroyable models as children, + # and let us avoid to use `dependent: :destroy` hook def use_fast_destroy(relation) before_destroy do - subject = public_send(relation) # rubocop:disable GitlabSecurity/PublicSend - after_commit_proc = subject.after_commit_cleanup_proc - run_after_commit(&after_commit_proc) + perform_fast_destroy(public_send(relation)) # rubocop:disable GitlabSecurity/PublicSend end end end + + def perform_fast_destroy(subject) + params = subject.begin_fast_destroy + + run_after_commit do + subject.finalize_fast_destroy(params) + end + end end end |