From ce675424f62578f68d2d8cf56330924ab738d550 Mon Sep 17 00:00:00 2001 From: Andreas Brandl <abrandl@gitlab.com> Date: Wed, 10 Feb 2021 12:17:52 +0100 Subject: [PATCH] Drop audit_events_archived table This is the last step of the partitioning operation. Relates to https://gitlab.com/gitlab-org/gitlab/-/issues/296644 --- .../ab-drop-non-partitioned-audit-events.yml | 5 ++ ...05335_drop_non_partitioned_audit_events.rb | 44 ++++++++++ db/schema_migrations/20210331105335 | 1 + db/structure.sql | 81 ------------------- .../table_management_helpers.rb | 32 +++++--- .../with_lock_retries_disallowed_method.rb | 1 + spec/db/schema_spec.rb | 1 - .../table_management_helpers_spec.rb | 66 +++++++++++++++ 8 files changed, 139 insertions(+), 92 deletions(-) create mode 100644 changelogs/unreleased/ab-drop-non-partitioned-audit-events.yml create mode 100644 db/post_migrate/20210331105335_drop_non_partitioned_audit_events.rb create mode 100644 db/schema_migrations/20210331105335 diff --git a/changelogs/unreleased/ab-drop-non-partitioned-audit-events.yml b/changelogs/unreleased/ab-drop-non-partitioned-audit-events.yml new file mode 100644 index 0000000000000..108b3c890e629 --- /dev/null +++ b/changelogs/unreleased/ab-drop-non-partitioned-audit-events.yml @@ -0,0 +1,5 @@ +--- +title: Drop non-partitioned audit_events_archived table +merge_request: 53880 +author: +type: other diff --git a/db/post_migrate/20210331105335_drop_non_partitioned_audit_events.rb b/db/post_migrate/20210331105335_drop_non_partitioned_audit_events.rb new file mode 100644 index 0000000000000..7c32fc61711fb --- /dev/null +++ b/db/post_migrate/20210331105335_drop_non_partitioned_audit_events.rb @@ -0,0 +1,44 @@ +# 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 diff --git a/db/schema_migrations/20210331105335 b/db/schema_migrations/20210331105335 new file mode 100644 index 0000000000000..5d9b8d3fea8c0 --- /dev/null +++ b/db/schema_migrations/20210331105335 @@ -0,0 +1 @@ +2127018617082dbad341bcee68948afe111286fdc2ea9ce8b3d00d356f3c61e0 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 3ff4d895aa686..560167e134e44 100644 --- a/db/structure.sql +++ b/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 @@ -20221,9 +20149,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); @@ -21718,8 +21643,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); @@ -21756,8 +21679,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_ci_pipelines_artifacts_locked ON ci_pipelines USING btree (ci_ref_id, id) WHERE (locked = 1); @@ -24680,8 +24601,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(); diff --git a/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb b/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb index 1c289391e215a..9ccbdc9930e61 100644 --- a/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb +++ b/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers.rb @@ -223,6 +223,28 @@ def rollback_replace_with_partitioned_table(table_name) 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 @@ def create_range_partition_safely(partition_name, table_name, lower_bound, upper 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) diff --git a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb index f423bde8343ee..cb36e7413aba5 100644 --- a/rubocop/cop/migration/with_lock_retries_disallowed_method.rb +++ b/rubocop/cop/migration/with_lock_retries_disallowed_method.rb @@ -27,6 +27,7 @@ class WithLockRetriesDisallowedMethod < RuboCop::Cop::Cop 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(', ')}" diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 0d8407dd7facc..05567430d6b5d 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -19,7 +19,6 @@ 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], diff --git a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb index b5d741fc5e9d9..5b2a29d1d2d27 100644 --- a/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb +++ b/spec/lib/gitlab/database/partitioning_migration_helpers/table_management_helpers_spec.rb @@ -704,6 +704,72 @@ def expect_table_to_be_replaced(&block) 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 -- GitLab