Introduce `use_fast_destroy` helper for parent associations. Rename method...

Introduce `use_fast_destroy` helper for parent associations.  Rename method names in build_trace_chunks. Forbid `destroy` method when `FastDestroyAll` included.

app/models/ci/build_trace_chunk.rb

@@ -6,7 +6,7 @@ module Ci
     belongs_to :build, class_name: "Ci::Build", foreign_key: :build_id
 
     default_value_for :data_store, :redis
-    fast_destroy_all_with :delete_all_redis_data, :redis_all_data_keys
+    fast_destroy_all_with :redis_delete_data, :redis_data_keys
 
     WriteError = Class.new(StandardError)
 
@@ -26,17 +26,17 @@ module Ci
         "gitlab:ci:trace:#{build_id}:chunks:#{chunk_index}"
       end
 
-      def redis_all_data_keys
+      def redis_data_keys
         redis.pluck(:build_id, :chunk_index).map do |data|
           redis_data_key(data.first, data.second)
         end
       end
 
-      def delete_all_redis_data(redis_keys)
-        if redis_keys.any?
-          Gitlab::Redis::SharedState.with do |redis|
-            redis.del(redis_keys)
-          end
+      def redis_delete_data(keys)
+        return if keys.empty?
+
+        Gitlab::Redis::SharedState.with do |redis|
+          redis.del(keys)
         end
       end
     end
@@ -80,7 +80,8 @@ module Ci
         break unless size > 0
 
         self.update!(raw_data: data, data_store: :db)
-        redis_delete_data
+        key = self.class.redis_data_key(build_id, chunk_index)
+        self.class.redis_delete_data([key])
       end
     end
 
@@ -138,12 +139,6 @@ module Ci
       end
     end
 
-    def redis_delete_data
-      Gitlab::Redis::SharedState.with do |redis|
-        redis.del(self.class.redis_data_key(build_id, chunk_index))
-      end
-    end
-
     def redis_lock_key
       "trace_write:#{build_id}:chunks:#{chunk_index}"
     end

app/models/concerns/fast_destroy_all.rb

@@ -4,7 +4,7 @@
 # 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  
+# 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.
 #
 # This module introduces a protocol to support the adoption with easy way.
@@ -12,12 +12,18 @@
 # - 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.delete_all_external_data_proc) }``
+#   e.g. `before_destroy -> { run_after_commit(&build_trace_chunks.after_commit_cleanup_proc) }``
 module FastDestroyAll
   extend ActiveSupport::Concern
 
+  ForbiddenActionError = Class.new(StandardError)
+
   included do
     class_attribute :_delete_method, :_delete_params_generator
+
+    before_destroy do
+      raise ForbiddenActionError, '`destroy` is forbbiden, please use `fast_destroy_all`'
+    end  
   end
 
   class_methods do
@@ -35,13 +41,13 @@ module FastDestroyAll
     # This method generates a proc to delete external data.
     # It's useful especially when you want to hook parent record's deletion event.
     #
-    # e.g. before_destroy -> { run_after_commit(&build_trace_chunks.delete_all_external_data_proc) }
-    def delete_all_external_data_proc
-      params = send self._delete_params_generator
-      callee = self # Preserve the callee class, otherwise `proc` uses a different class
+    # 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
-        callee.send callee._delete_method, params
+        subject.send subject._delete_method, params # rubocop:disable GitlabSecurity/PublicSend
 
         true
       end
@@ -52,10 +58,32 @@ module FastDestroyAll
     # 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
-      delete_all_external_data_proc.tap do |delete_all_external_data|
+      after_commit_cleanup_proc.tap do |delete_all_external_data|
         delete_all
         delete_all_external_data.call
       end
     end
   end
+
+  module Helpers
+    extend ActiveSupport::Concern
+
+    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, depenedent: :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
+      def use_fast_destroy(relation)
+        before_destroy do
+          subject = public_send(relation)
+          after_commit_proc = subject.after_commit_cleanup_proc
+          run_after_commit(&after_commit_proc)
+        end
+      end
+    end
+  end
 end

app/models/project.rb

@@ -22,6 +22,7 @@ class Project < ActiveRecord::Base
   include DeploymentPlatform
   include ::Gitlab::Utils::StrongMemoize
   include ChronicDurationAttribute
+  include FastDestroyAll::Helpers
 
   extend Gitlab::ConfigHelper
 
@@ -79,10 +80,10 @@ class Project < ActiveRecord::Base
 
   before_destroy :remove_private_deploy_keys
 
-  # This has to be defined before `has_many :builds, depenedent: :destroy`,
-  # otherwise we will not delete any data, due to trace chunks
-  # going through :builds
-  before_destroy -> { run_after_commit(&build_trace_chunks.delete_all_external_data_proc) }
+  ##
+  # `use_fast_destroy` must be defined **before** `has_many` and `has_one` such as `has_many :relation, depenedent: :destroy`
+  # Otherwise `use_fast_destroy` performs against **deleted** rows, which fails to get identifiers of external data
+  use_fast_destroy :build_trace_chunks
 
   after_destroy -> { run_after_commit { remove_pages } }
   after_destroy :remove_exports