Merge branch 'prune-workers-refactoring' into 'master'

Refactor code in PruneOldEventsWorker & PruneWebHookLogsWorker

See merge request gitlab-org/gitlab!24075

app/models/concerns/delete_with_limit.rb

+# frozen_string_literal: true
+
+module DeleteWithLimit
+  extend ActiveSupport::Concern
+
+  class_methods do
+    def delete_with_limit(maximum)
+      limit(maximum).delete_all
+    end
+  end
+end

app/models/event.rb

@@ -4,6 +4,8 @@ class Event < ApplicationRecord
   include Sortable
   include FromUnion
   include Presentable
+  include DeleteWithLimit
+  include CreatedAtFilterable
 
   default_scope { reorder(nil) }
 

app/models/hooks/web_hook_log.rb

@@ -3,6 +3,8 @@
 class WebHookLog < ApplicationRecord
   include SafeUrl
   include Presentable
+  include DeleteWithLimit
+  include CreatedAtFilterable
 
   belongs_to :web_hook
 

app/workers/prune_old_events_worker.rb

@@ -6,18 +6,12 @@ class PruneOldEventsWorker
 
   feature_category_not_owned!
 
-  # rubocop: disable CodeReuse/ActiveRecord
+  DELETE_LIMIT = 10_000
+
   def perform
     # Contribution calendar shows maximum 12 months of events, we retain 3 years for data integrity.
-    # Double nested query is used because MySQL doesn't allow DELETE subqueries on the same table.
-    Event.unscoped.where(
-      '(id IN (SELECT id FROM (?) ids_to_remove))',
-      Event.unscoped.where(
-        'created_at < ?',
-        (3.years + 1.day).ago)
-      .select(:id)
-      .limit(10_000))
-    .delete_all
+    cutoff_date = (3.years + 1.day).ago
+
+    Event.unscoped.created_before(cutoff_date).delete_with_limit(DELETE_LIMIT)
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 end

app/workers/prune_web_hook_logs_worker.rb

@@ -11,20 +11,9 @@ class PruneWebHookLogsWorker
   # The maximum number of rows to remove in a single job.
   DELETE_LIMIT = 50_000
 
-  # rubocop: disable CodeReuse/ActiveRecord
   def perform
-    # MySQL doesn't allow "DELETE FROM ... WHERE id IN ( ... )" if the inner
-    # query refers to the same table. To work around this we wrap the IN body in
-    # another sub query.
-    WebHookLog
-      .where(
-        'id IN (SELECT id FROM (?) ids_to_remove)',
-        WebHookLog
-          .select(:id)
-          .where('created_at < ?', 90.days.ago.beginning_of_day)
-          .limit(DELETE_LIMIT)
-      )
-      .delete_all
+    cutoff_date = 90.days.ago.beginning_of_day
+
+    WebHookLog.created_before(cutoff_date).delete_with_limit(DELETE_LIMIT)
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 end

ee/app/models/concerns/geo/eventable.rb

@@ -4,6 +4,7 @@ module Geo
   module Eventable
     extend ActiveSupport::Concern
     include ::EachBatch
+    include ::DeleteWithLimit
 
     included do
       has_one :geo_event_log, class_name: 'Geo::EventLog'
@@ -14,10 +15,6 @@ module Geo
         joins(:geo_event_log)
           .where(Geo::EventLog.arel_table[:id].lteq(geo_event_log_id))
       end
-
-      def delete_with_limit(maximum)
-        limit(maximum).delete_all
-      end
     end
 
     def consumer_klass_name

spec/models/concerns/delete_with_limit_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe DeleteWithLimit do
+  describe '.delete_with_limit' do
+    it 'deletes a limited amount of rows' do
+      create_list(:web_hook_log, 4)
+
+      expect do
+        WebHookLog.delete_with_limit(2)
+      end.to change { WebHookLog.count }.by(-2)
+    end
+  end
+end