Merge branch 'web-hook-logs-partitioning-without-ff' into 'master'

Register web_hook_logs for partitioning in initializer

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/184043



Merged-by: Prabakaran Murugesan <pmurugesan@gitlab.com>
Approved-by: Max Orefice <morefice@gitlab.com>
Approved-by: Prabakaran Murugesan <pmurugesan@gitlab.com>
Reviewed-by: Max Orefice <morefice@gitlab.com>
Co-authored-by: Simon Tomlinson <stomlinson@gitlab.com>

app/models/hooks/web_hook_log.rb

@@ -19,7 +19,12 @@ class WebHookLog < ApplicationRecord
 
   self.primary_key = :id
 
-  partitioned_by :created_at, strategy: :monthly, retain_for: 1.month
+  # The partitioning definition here has been temporarily moved to config/initializers/postgres_partitioning.rb
+  # so that it does not interact with the changing WebHookLog.table_name as influenced by the
+  # ::Ci::Partitionable::Switch module.
+  # In config/initializers/postgres_partitioning.rb:
+  #   - web_hook_logs_daily is registered for daily partitioning for when the flag is ON
+  #   - web_hook_logs is registered for monthly partitioning for when the flag is OFF
 
   belongs_to :web_hook
 

config/initializers/postgres_partitioning.rb

@@ -35,8 +35,9 @@
     Gitlab::Database::BackgroundMigration::BatchedJobTransitionLog,
     LooseForeignKeys::DeletedRecord,
     Users::GroupVisit,
-    Users::ProjectVisit,
-    WebHookLog
+    Users::ProjectVisit
+    # WebHookLog is temporarily removed from this list and managed without a model
+    # during the switch from web_hook_logs to web_hook_logs_daily
   ])
 
 if Gitlab.ee?
@@ -84,12 +85,20 @@
 
 # Enable partition management for the backfill table during web_hook_logs partitioning.
 # This way new partitions will be created as the trigger syncs new rows across to this table.
+# We're controlling the table backing WebHookLog with the feature flag web_hook_logs_daily_enabled.
+# So that the feature flag does not interact with the partition manager, register both web_hook_logs tables here,
+# disconnected from the feature flag.
 Gitlab::Database::Partitioning.register_tables(
   [
     {
       limit_connection_names: %i[main],
       table_name: 'web_hook_logs_daily',
       partitioned_column: :created_at, strategy: :daily, retain_for: 14.days
+    },
+    {
+      limit_connection_names: %i[main],
+      table_name: 'web_hook_logs',
+      partitioned_column: :created_at, strategy: :monthly, retain_for: 1.month
     }
   ]
 )

spec/models/hooks/web_hook_log_spec.rb

@@ -292,10 +292,37 @@
   end
 
   describe 'routing table switch' do
+    shared_examples 'both tables are set up for partitioning' do
+      let(:tables_registered_for_sync) { Gitlab::Database::Partitioning.send(:registered_for_sync) }
+      it 'registers both web_hook_logs and web_hook_logs_daily for partitioning' do
+        expect(tables_registered_for_sync.map(&:table_name)).to include('web_hook_logs_daily', 'web_hook_logs')
+      end
+
+      it 'registers web_hook_logs for monthly partitioning' do
+        web_hook_logs_entries = tables_registered_for_sync.select { |model| model.table_name == 'web_hook_logs' }
+        expect(web_hook_logs_entries.count).to eq(1)
+        web_hook_logs_entry = web_hook_logs_entries.first
+        expect(web_hook_logs_entry.partitioning_strategy).to be_a(Gitlab::Database::Partitioning::Time::MonthlyStrategy)
+        expect(web_hook_logs_entry.partitioning_strategy.retain_for).to eq(1.month)
+      end
+
+      it 'registers web_hook_logs_daily for daily partitioning' do
+        web_hook_logs_daily_entries = tables_registered_for_sync
+                                        .select { |model| model.table_name == 'web_hook_logs_daily' }
+        expect(web_hook_logs_daily_entries.count).to eq(1)
+        web_hook_logs_daily_entry = web_hook_logs_daily_entries.first
+        expect(web_hook_logs_daily_entry.partitioning_strategy)
+          .to be_a(Gitlab::Database::Partitioning::Time::DailyStrategy)
+        expect(web_hook_logs_daily_entry.partitioning_strategy.retain_for).to eq(14.days)
+      end
+    end
+
     context 'with ff enabled' do
       it 'returns daily partitioned table' do
         expect(described_class.table_name).to eq('web_hook_logs_daily')
       end
+
+      it_behaves_like 'both tables are set up for partitioning'
     end
 
     context 'with ff disabled' do
@@ -306,6 +333,8 @@
       it 'returns monthly partitioned table' do
         expect(described_class.table_name).to eq('web_hook_logs')
       end
+
+      it_behaves_like 'both tables are set up for partitioning'
     end
   end
 end