Merge branch 'ab/drop-non-partitioned-audit-events' into 'master'

Drop non-partitioned `audit_events_archived` table [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!53880

changelogs/unreleased/ab-drop-non-partitioned-audit-events.yml

+---
+title: Drop non-partitioned audit_events_archived table
+merge_request: 53880
+author:
+type: other

db/post_migrate/20210331105335_drop_non_partitioned_audit_events.rb

+# frozen_string_literal: true
+
+class DropNonPartitionedAuditEvents < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+  include Gitlab::Database::PartitioningMigrationHelpers::TableManagementHelpers
+
+  DOWNTIME = false
+
+  def up
+    drop_nonpartitioned_archive_table(:audit_events)
+  end
+
+  def down
+    execute(<<~SQL)
+      CREATE TABLE audit_events_archived (
+        id integer NOT NULL,
+        author_id integer NOT NULL,
+        entity_id integer NOT NULL,
+        entity_type character varying NOT NULL,
+        details text,
+        created_at timestamp without time zone,
+        ip_address inet,
+        author_name text,
+        entity_path text,
+        target_details text,
+        target_type text,
+        target_id bigint,
+        CONSTRAINT check_492aaa021d CHECK ((char_length(entity_path) <= 5500)),
+        CONSTRAINT check_82294106dd CHECK ((char_length(target_type) <= 255)),
+        CONSTRAINT check_83ff8406e2 CHECK ((char_length(author_name) <= 255)),
+        CONSTRAINT check_d493ec90b5 CHECK ((char_length(target_details) <= 5500))
+      );
+
+      ALTER TABLE ONLY audit_events_archived ADD CONSTRAINT audit_events_archived_pkey PRIMARY KEY (id);
+
+      CREATE INDEX analytics_index_audit_events_on_created_at_and_author_id ON audit_events_archived USING btree (created_at, author_id);
+      CREATE INDEX idx_audit_events_on_entity_id_desc_author_id_created_at ON audit_events_archived USING btree (entity_id, entity_type, id DESC, author_id, created_at);
+    SQL
+
+    with_lock_retries do
+      create_trigger_to_sync_tables(:audit_events, :audit_events_archived, 'id')
+    end
+  end
+end

db/schema_migrations/20210331105335

+2127018617082dbad341bcee68948afe111286fdc2ea9ce8b3d00d356f3c61e0
\ No newline at end of file

db/structure.sql

@@ -97,59 +97,6 @@ $$;
 
 COMMENT ON FUNCTION table_sync_function_29bc99d6db() IS 'Partitioning migration: table sync for web_hook_logs table';
 
-CREATE FUNCTION table_sync_function_2be879775d() RETURNS trigger
-    LANGUAGE plpgsql
-    AS $$
-BEGIN
-IF (TG_OP = 'DELETE') THEN
-  DELETE FROM audit_events_archived where id = OLD.id;
-ELSIF (TG_OP = 'UPDATE') THEN
-  UPDATE audit_events_archived
-  SET author_id = NEW.author_id,
-    entity_id = NEW.entity_id,
-    entity_type = NEW.entity_type,
-    details = NEW.details,
-    created_at = NEW.created_at,
-    ip_address = NEW.ip_address,
-    author_name = NEW.author_name,
-    entity_path = NEW.entity_path,
-    target_details = NEW.target_details,
-    target_type = NEW.target_type,
-    target_id = NEW.target_id
-  WHERE audit_events_archived.id = NEW.id;
-ELSIF (TG_OP = 'INSERT') THEN
-  INSERT INTO audit_events_archived (id,
-    author_id,
-    entity_id,
-    entity_type,
-    details,
-    created_at,
-    ip_address,
-    author_name,
-    entity_path,
-    target_details,
-    target_type,
-    target_id)
-  VALUES (NEW.id,
-    NEW.author_id,
-    NEW.entity_id,
-    NEW.entity_type,
-    NEW.details,
-    NEW.created_at,
-    NEW.ip_address,
-    NEW.author_name,
-    NEW.entity_path,
-    NEW.target_details,
-    NEW.target_type,
-    NEW.target_id);
-END IF;
-RETURN NULL;
-
-END
-$$;
-
-COMMENT ON FUNCTION table_sync_function_2be879775d() IS 'Partitioning migration: table sync for audit_events table';
-
 CREATE FUNCTION trigger_07c94931164e() RETURNS trigger
     LANGUAGE plpgsql
     AS $$
@@ -9742,25 +9689,6 @@ CREATE SEQUENCE atlassian_identities_user_id_seq
 
 ALTER SEQUENCE atlassian_identities_user_id_seq OWNED BY atlassian_identities.user_id;
 
-CREATE TABLE audit_events_archived (
-    id integer NOT NULL,
-    author_id integer NOT NULL,
-    entity_id integer NOT NULL,
-    entity_type character varying NOT NULL,
-    details text,
-    created_at timestamp without time zone,
-    ip_address inet,
-    author_name text,
-    entity_path text,
-    target_details text,
-    target_type text,
-    target_id bigint,
-    CONSTRAINT check_492aaa021d CHECK ((char_length(entity_path) <= 5500)),
-    CONSTRAINT check_82294106dd CHECK ((char_length(target_type) <= 255)),
-    CONSTRAINT check_83ff8406e2 CHECK ((char_length(author_name) <= 255)),
-    CONSTRAINT check_d493ec90b5 CHECK ((char_length(target_details) <= 5500))
-);
-
 CREATE SEQUENCE audit_events_id_seq
     START WITH 1
     INCREMENT BY 1
@@ -20231,9 +20159,6 @@ ALTER TABLE ONLY ar_internal_metadata
 ALTER TABLE ONLY atlassian_identities
     ADD CONSTRAINT atlassian_identities_pkey PRIMARY KEY (user_id);
 
-ALTER TABLE ONLY audit_events_archived
-    ADD CONSTRAINT audit_events_archived_pkey PRIMARY KEY (id);
-
 ALTER TABLE ONLY audit_events
     ADD CONSTRAINT audit_events_pkey PRIMARY KEY (id, created_at);
 
@@ -21731,8 +21656,6 @@ CREATE INDEX product_analytics_events_experi_project_id_collector_tstamp_idx ON
 
 CREATE INDEX active_billable_users ON users USING btree (id) WHERE (((state)::text = 'active'::text) AND ((user_type IS NULL) OR (user_type = ANY (ARRAY[NULL::integer, 6, 4]))) AND ((user_type IS NULL) OR (user_type <> ALL ('{2,6,1,3,7,8}'::smallint[]))));
 
-CREATE INDEX analytics_index_audit_events_on_created_at_and_author_id ON audit_events_archived USING btree (created_at, author_id);
-
 CREATE INDEX analytics_index_audit_events_part_on_created_at_and_author_id ON ONLY audit_events USING btree (created_at, author_id);
 
 CREATE INDEX analytics_index_events_on_created_at_and_author_id ON events USING btree (created_at, author_id);
@@ -21769,8 +21692,6 @@ CREATE INDEX finding_evidences_on_vulnerability_occurrence_id ON vulnerability_f
 
 CREATE INDEX finding_links_on_vulnerability_occurrence_id ON vulnerability_finding_links USING btree (vulnerability_occurrence_id);
 
-CREATE INDEX idx_audit_events_on_entity_id_desc_author_id_created_at ON audit_events_archived USING btree (entity_id, entity_type, id DESC, author_id, created_at);
-
 CREATE INDEX idx_audit_events_part_on_entity_id_desc_author_id_created_at ON ONLY audit_events USING btree (entity_id, entity_type, id DESC, author_id, created_at);
 
 CREATE INDEX idx_award_emoji_on_user_emoji_name_awardable_type_awardable_id ON award_emoji USING btree (user_id, name, awardable_type, awardable_id);
@@ -24701,8 +24622,6 @@ ALTER INDEX product_analytics_events_experimental_pkey ATTACH PARTITION gitlab_p
 
 CREATE TRIGGER table_sync_trigger_b99eb6998c AFTER INSERT OR DELETE OR UPDATE ON web_hook_logs FOR EACH ROW EXECUTE PROCEDURE table_sync_function_29bc99d6db();
 
-CREATE TRIGGER table_sync_trigger_ee39a25f9d AFTER INSERT OR DELETE OR UPDATE ON audit_events FOR EACH ROW EXECUTE PROCEDURE table_sync_function_2be879775d();
-
 CREATE TRIGGER trigger_07c94931164e BEFORE INSERT OR UPDATE ON push_event_payloads FOR EACH ROW EXECUTE PROCEDURE trigger_07c94931164e();
 
 CREATE TRIGGER trigger_69523443cc10 BEFORE INSERT OR UPDATE ON events FOR EACH ROW EXECUTE PROCEDURE trigger_69523443cc10();

lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb

@@ -223,6 +223,28 @@ module Gitlab
           replace_table(table_name, archived_table_name, partitioned_table_name, primary_key_name)
         end
 
+        def drop_nonpartitioned_archive_table(table_name)
+          assert_table_is_allowed(table_name)
+
+          archived_table_name = make_archived_table_name(table_name)
+
+          with_lock_retries do
+            drop_sync_trigger(table_name)
+          end
+
+          drop_table(archived_table_name)
+        end
+
+        def create_trigger_to_sync_tables(source_table_name, partitioned_table_name, unique_key)
+          function_name = make_sync_function_name(source_table_name)
+          trigger_name = make_sync_trigger_name(source_table_name)
+
+          create_sync_function(function_name, partitioned_table_name, unique_key)
+          create_comment('FUNCTION', function_name, "Partitioning migration: table sync for #{source_table_name} table")
+
+          create_sync_trigger(source_table_name, trigger_name, function_name)
+        end
+
         private
 
         def assert_table_is_allowed(table_name)
@@ -316,16 +338,6 @@ module Gitlab
           create_range_partition(partition_name, table_name, lower_bound, upper_bound)
         end
 
-        def create_trigger_to_sync_tables(source_table_name, partitioned_table_name, unique_key)
-          function_name = make_sync_function_name(source_table_name)
-          trigger_name = make_sync_trigger_name(source_table_name)
-
-          create_sync_function(function_name, partitioned_table_name, unique_key)
-          create_comment('FUNCTION', function_name, "Partitioning migration: table sync for #{source_table_name} table")
-
-          create_sync_trigger(source_table_name, trigger_name, function_name)
-        end
-
         def drop_sync_trigger(source_table_name)
           trigger_name = make_sync_trigger_name(source_table_name)
           drop_trigger(source_table_name, trigger_name)

rubocop/cop/migration/with_lock_retries_disallowed_method.rb

@@ -27,6 +27,7 @@ module RuboCop
           foreign_key_exists?
           index_exists?
           column_exists?
+          create_trigger_to_sync_tables
         ].sort.freeze
 
         MSG = "The method is not allowed to be called within the `with_lock_retries` block, the only allowed methods are: #{ALLOWED_MIGRATION_METHODS.join(', ')}"

spec/db/schema_spec.rb

@@ -19,7 +19,6 @@ RSpec.describe 'Database schema' do
     approver_groups: %w[target_id],
     approvers: %w[target_id user_id],
     audit_events: %w[author_id entity_id target_id],
-    audit_events_archived: %w[author_id entity_id target_id],
     award_emoji: %w[awardable_id user_id],
     aws_roles: %w[role_external_id],
     boards: %w[milestone_id iteration_id],

spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb

@@ -704,6 +704,72 @@ RSpec.describe Gitlab::Database::PartitioningMigrationHelpers::TableManagementHe
     end
   end
 
+  describe '#drop_nonpartitioned_archive_table' do
+    subject { migration.drop_nonpartitioned_archive_table source_table }
+
+    let(:archived_table) { "#{source_table}_archived" }
+
+    before do
+      migration.partition_table_by_date source_table, partition_column, min_date: min_date, max_date: max_date
+      migration.replace_with_partitioned_table source_table
+    end
+
+    it 'drops the archive table' do
+      expect(table_type(archived_table)).to eq('normal')
+
+      subject
+
+      expect(table_type(archived_table)).to eq(nil)
+    end
+
+    it 'drops the trigger on the source table' do
+      expect_valid_function_trigger(source_table, trigger_name, function_name, after: %w[delete insert update])
+
+      subject
+
+      expect_trigger_not_to_exist(source_table, trigger_name)
+    end
+
+    it 'drops the sync function' do
+      expect_function_to_exist(function_name)
+
+      subject
+
+      expect_function_not_to_exist(function_name)
+    end
+  end
+
+  describe '#create_trigger_to_sync_tables' do
+    subject { migration.create_trigger_to_sync_tables(source_table, target_table, :id) }
+
+    let(:target_table) { "#{source_table}_copy" }
+
+    before do
+      migration.create_table target_table do |t|
+        t.string :name, null: false
+        t.integer :age, null: false
+        t.datetime partition_column
+        t.datetime :updated_at
+      end
+    end
+
+    it 'creates the sync function' do
+      expect_function_not_to_exist(function_name)
+
+      subject
+
+      expect_function_to_exist(function_name)
+    end
+
+    it 'installs the trigger' do
+      expect_trigger_not_to_exist(source_table, trigger_name)
+
+      subject
+
+      expect_valid_function_trigger(source_table, trigger_name, function_name, after: %w[delete insert update])
+    end
+  end
+
   def filter_columns_by_name(columns, names)
     columns.reject { |c| names.include?(c.name) }
   end