Finalize conversion to bigint for events (!64779) · Merge requests · GitLab.org / GitLab

Krasimir Angelov requested to merge 288005-finalize-events-bigint-conversion into master Jun 24, 2021

What does this MR do?

This MR finalize the conversion to bigint for events table.

At a high level, the operation takes the following steps:

Ensure the migration of id to id_convert_to_bigint is completed.
Copy indexes and FKs
Swap columns

Cleanup (removing the old int columns and the triggers) will be done in later MR. This leaves us with a trigger that copy values from bigint column to integer column, but this works fine, as long we do not hit the limit for integer.

Related to #288005 (closed).

❗ Before merging

Make sure !68426 (merged) is deployed to production and indexes are created.

Database migrations

⏱ Timing

From DB Lab:

ALTER TABLE push_event_payloads VALIDATE CONSTRAINT fk_36c74129da_tmp

Duration: 22.554 min - https://console.postgres.ai/gitlab/gitlab-production-tunnel-pg12/sessions/4907/commands/17161

Indexes were pre-created with !68426 (comment 657250665), and now the only time consuming statement is the FK validation. This is also confirmed from the migration testing pipeline - !64779 (comment 613785586).

🗒 Output

⬆ Up

$ bundle exec rails db:migrate:up VERSION=20210622045705
== 20210622045705 FinalizeEventsBigintConversion: migrating ===================
-- transaction_open?()
   -> 0.0000s
-- index_exists?("events", :id_convert_to_bigint, {:unique=>true, :name=>"index_events_on_id_convert_to_bigint", :algorithm=>:concurrently})
   -> 0.0046s
-- execute("SET statement_timeout TO 0")
   -> 0.0005s
-- add_index("events", :id_convert_to_bigint, {:unique=>true, :name=>"index_events_on_id_convert_to_bigint", :algorithm=>:concurrently})
   -> 0.0100s
-- execute("RESET statement_timeout")
   -> 0.0006s
-- transaction_open?()
   -> 0.0000s
-- index_exists?("events", [:project_id, :id_convert_to_bigint], {:name=>"index_events_on_project_id_and_id_convert_to_bigint", :algorithm=>:concurrently})
   -> 0.0042s
-- add_index("events", [:project_id, :id_convert_to_bigint], {:name=>"index_events_on_project_id_and_id_convert_to_bigint", :algorithm=>:concurrently})
   -> 0.0027s
-- transaction_open?()
   -> 0.0000s
-- index_exists?("events", [:project_id, :id_convert_to_bigint], {:order=>{:id_convert_to_bigint=>:desc}, :where=>"action = 7", :name=>"index_events_on_project_id_and_id_bigint_desc_on_merged_action", :algorithm=>:concurrently})
   -> 0.0049s
-- add_index("events", [:project_id, :id_convert_to_bigint], {:order=>{:id_convert_to_bigint=>:desc}, :where=>"action = 7", :name=>"index_events_on_project_id_and_id_bigint_desc_on_merged_action", :algorithm=>:concurrently})
   -> 0.0035s
-- transaction_open?()
   -> 0.0000s
-- foreign_keys(:push_event_payloads)
   -> 0.0043s
-- execute("LOCK TABLE events, push_event_payloads IN SHARE ROW EXCLUSIVE MODE")
   -> 0.0007s
-- execute("ALTER TABLE push_event_payloads\nADD CONSTRAINT fk_36c74129da_tmp\nFOREIGN KEY (event_id)\nREFERENCES events (id_convert_to_bigint)\nON DELETE CASCADE\nNOT VALID;\n")
   -> 0.0085s
-- execute("ALTER TABLE push_event_payloads VALIDATE CONSTRAINT fk_36c74129da_tmp;")
   -> 0.0082s
-- execute("LOCK TABLE events, push_event_payloads IN ACCESS EXCLUSIVE MODE")
   -> 0.0006s
-- quote_table_name("events")
   -> 0.0000s
-- quote_column_name(:id)
   -> 0.0000s
-- quote_column_name("id_tmp")
   -> 0.0000s
-- execute("ALTER TABLE \"events\" RENAME COLUMN \"id\" TO \"id_tmp\"")
   -> 0.0006s
-- quote_table_name("events")
   -> 0.0000s
-- quote_column_name(:id_convert_to_bigint)
   -> 0.0000s
-- quote_column_name(:id)
   -> 0.0000s
-- execute("ALTER TABLE \"events\" RENAME COLUMN \"id_convert_to_bigint\" TO \"id\"")
   -> 0.0006s
-- quote_table_name("events")
   -> 0.0000s
-- quote_column_name("id_tmp")
   -> 0.0000s
-- quote_column_name(:id_convert_to_bigint)
   -> 0.0000s
-- execute("ALTER TABLE \"events\" RENAME COLUMN \"id_tmp\" TO \"id_convert_to_bigint\"")
   -> 0.0005s
-- quote_table_name("trigger_69523443cc10")
   -> 0.0000s
-- execute("ALTER FUNCTION \"trigger_69523443cc10\" RESET ALL")
   -> 0.0006s
-- execute("ALTER SEQUENCE events_id_seq OWNED BY events.id")
   -> 0.0010s
-- change_column_default("events", :id, #<Proc:0x00007feb36f15378 /Users/krasio/projects/gitlab/ee/gitlab/db/post_migrate/20210622045705_finalize_events_bigint_conversion.rb:68 (lambda)>)
   -> 0.0048s
-- change_column_default("events", :id_convert_to_bigint, 0)
   -> 0.0019s
-- execute("ALTER TABLE events DROP CONSTRAINT events_pkey CASCADE")
   -> 0.0015s
-- rename_index("events", "index_events_on_id_convert_to_bigint", "events_pkey")
   -> 0.0007s
-- execute("ALTER TABLE events ADD CONSTRAINT events_pkey PRIMARY KEY USING INDEX events_pkey")
   -> 0.0013s
-- execute("DROP INDEX index_events_on_project_id_and_id")
   -> 0.0007s
-- rename_index("events", "index_events_on_project_id_and_id_convert_to_bigint", "index_events_on_project_id_and_id")
   -> 0.0012s
-- execute("DROP INDEX index_events_on_project_id_and_id_desc_on_merged_action")
   -> 0.0007s
-- rename_index("events", "index_events_on_project_id_and_id_bigint_desc_on_merged_action", "index_events_on_project_id_and_id_desc_on_merged_action")
   -> 0.0007s
-- quote_table_name(:push_event_payloads)
   -> 0.0000s
-- quote_column_name("fk_36c74129da_tmp")
   -> 0.0000s
-- quote_column_name("fk_36c74129da")
   -> 0.0000s
-- execute("ALTER TABLE \"push_event_payloads\"\nRENAME CONSTRAINT \"fk_36c74129da_tmp\" TO \"fk_36c74129da\"\n")
   -> 0.0006s
== 20210622045705 FinalizeEventsBigintConversion: migrated (0.1288s) ==========

⬇ Down

$ bundle exec rails db:migrate:down VERSION=20210622045705
== 20210622045705 FinalizeEventsBigintConversion: reverting ===================
-- transaction_open?()
   -> 0.0000s
-- index_exists?("events", :id_convert_to_bigint, {:unique=>true, :name=>"index_events_on_id_convert_to_bigint", :algorithm=>:concurrently})
   -> 0.0048s
-- execute("SET statement_timeout TO 0")
   -> 0.0005s
-- add_index("events", :id_convert_to_bigint, {:unique=>true, :name=>"index_events_on_id_convert_to_bigint", :algorithm=>:concurrently})
   -> 0.0043s
-- execute("RESET statement_timeout")
   -> 0.0005s
-- transaction_open?()
   -> 0.0000s
-- index_exists?("events", [:project_id, :id_convert_to_bigint], {:name=>"index_events_on_project_id_and_id_convert_to_bigint", :algorithm=>:concurrently})
   -> 0.0033s
-- add_index("events", [:project_id, :id_convert_to_bigint], {:name=>"index_events_on_project_id_and_id_convert_to_bigint", :algorithm=>:concurrently})
   -> 0.0021s
-- transaction_open?()
   -> 0.0000s
-- index_exists?("events", [:project_id, :id_convert_to_bigint], {:order=>{:id_convert_to_bigint=>:desc}, :where=>"action = 7", :name=>"index_events_on_project_id_and_id_bigint_desc_on_merged_action", :algorithm=>:concurrently})
   -> 0.0038s
-- add_index("events", [:project_id, :id_convert_to_bigint], {:order=>{:id_convert_to_bigint=>:desc}, :where=>"action = 7", :name=>"index_events_on_project_id_and_id_bigint_desc_on_merged_action", :algorithm=>:concurrently})
   -> 0.0022s
-- transaction_open?()
   -> 0.0000s
-- foreign_keys(:push_event_payloads)
   -> 0.0027s
-- execute("LOCK TABLE events, push_event_payloads IN SHARE ROW EXCLUSIVE MODE")
   -> 0.0006s
-- execute("ALTER TABLE push_event_payloads\nADD CONSTRAINT fk_36c74129da_tmp\nFOREIGN KEY (event_id)\nREFERENCES events (id_convert_to_bigint)\nON DELETE CASCADE\nNOT VALID;\n")
   -> 0.0013s
-- execute("ALTER TABLE push_event_payloads VALIDATE CONSTRAINT fk_36c74129da_tmp;")
   -> 0.0027s
-- execute("LOCK TABLE events, push_event_payloads IN ACCESS EXCLUSIVE MODE")
   -> 0.0005s
-- quote_table_name("events")
   -> 0.0000s
-- quote_column_name(:id)
   -> 0.0000s
-- quote_column_name("id_tmp")
   -> 0.0000s
-- execute("ALTER TABLE \"events\" RENAME COLUMN \"id\" TO \"id_tmp\"")
   -> 0.0005s
-- quote_table_name("events")
   -> 0.0000s
-- quote_column_name(:id_convert_to_bigint)
   -> 0.0000s
-- quote_column_name(:id)
   -> 0.0000s
-- execute("ALTER TABLE \"events\" RENAME COLUMN \"id_convert_to_bigint\" TO \"id\"")
   -> 0.0005s
-- quote_table_name("events")
   -> 0.0000s
-- quote_column_name("id_tmp")
   -> 0.0000s
-- quote_column_name(:id_convert_to_bigint)
   -> 0.0000s
-- execute("ALTER TABLE \"events\" RENAME COLUMN \"id_tmp\" TO \"id_convert_to_bigint\"")
   -> 0.0006s
-- quote_table_name("trigger_69523443cc10")
   -> 0.0000s
-- execute("ALTER FUNCTION \"trigger_69523443cc10\" RESET ALL")
   -> 0.0007s
-- execute("ALTER SEQUENCE events_id_seq OWNED BY events.id")
   -> 0.0006s
-- change_column_default("events", :id, #<Proc:0x00007fcaeedc3e60 /Users/krasio/projects/gitlab/ee/gitlab/db/post_migrate/20210622045705_finalize_events_bigint_conversion.rb:68 (lambda)>)
   -> 0.0030s
-- change_column_default("events", :id_convert_to_bigint, 0)
   -> 0.0018s
-- execute("ALTER TABLE events DROP CONSTRAINT events_pkey CASCADE")
   -> 0.0009s
-- rename_index("events", "index_events_on_id_convert_to_bigint", "events_pkey")
   -> 0.0006s
-- execute("ALTER TABLE events ADD CONSTRAINT events_pkey PRIMARY KEY USING INDEX events_pkey")
   -> 0.0006s
-- execute("DROP INDEX index_events_on_project_id_and_id")
   -> 0.0006s
-- rename_index("events", "index_events_on_project_id_and_id_convert_to_bigint", "index_events_on_project_id_and_id")
   -> 0.0006s
-- execute("DROP INDEX index_events_on_project_id_and_id_desc_on_merged_action")
   -> 0.0006s
-- rename_index("events", "index_events_on_project_id_and_id_bigint_desc_on_merged_action", "index_events_on_project_id_and_id_desc_on_merged_action")
   -> 0.0006s
-- quote_table_name(:push_event_payloads)
   -> 0.0000s
-- quote_column_name("fk_36c74129da_tmp")
   -> 0.0000s
-- quote_column_name("fk_36c74129da")
   -> 0.0000s
-- execute("ALTER TABLE \"push_event_payloads\"\nRENAME CONSTRAINT \"fk_36c74129da_tmp\" TO \"fk_36c74129da\"\n")
   -> 0.0006s
== 20210622045705 FinalizeEventsBigintConversion: reverted (0.0680s) ==========

Does this MR meet the acceptance criteria?

Conformity

I have included changelog trailers, or none are needed. (Does this MR need a changelog?)
I have added/updated documentation, or it's not needed. (Is documentation required?)
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?)
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?)
I have self-reviewed this MR per code review guidelines.
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines)
I have followed the style guides.
This change is backwards compatible across updates, or this does not apply.

Availability and Testing

I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.)
I have tested this MR in all supported browsers, or it's not needed.
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.

Security

Does this MR contain changes to processing or storing of credentials or tokens, authorization and authentication methods or other items described in the security review guidelines? If not, then delete this Security section.

Label as security and @ mention @gitlab-com/gl-security/appsec
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
Security reports checked/validated by a reviewer from the AppSec team

Related to #288005 (closed)

Edited Aug 24, 2021 by Krasimir Angelov

Finalize conversion to bigint for events