Finalize conversion to bigint for events
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
toid_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)
Merge request reports
Activity
changed milestone to %14.1
assigned to @krasio
- A deleted user
added backend label
1 Warning You've made some app changes, but didn't add any tests.
That's OK as long as you're refactoring existing code,
but please consider adding any of the tooling, ~"tooling::pipelines", ~"tooling::workflow", documentation, QA labels.Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer database Alina Mihaila ( @alinamihaila
) (UTC+3, 9 hours behind@krasio
)Tiger Watson ( @tigerwnz
) (UTC+10, 2 hours behind@krasio
)~migration No reviewer available No maintainer available To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
If needed, you can retry the
danger-review
job that generated this comment.Generated by
Dangermentioned in epic &4785 (closed)
added 5 commits
-
33b7e614...73b764b8 - 3 commits from branch
288005-finalize-push-events-payloads-bigint-conversion
- e398644c - Improve add_concurrent_foreign_key to support custom target column
- 0f5cc3fc - Finalize conversion to bigint for events
-
33b7e614...73b764b8 - 3 commits from branch
mentioned in issue #288005 (closed)
added 560 commits
-
0f5cc3fc...ea525f3b - 558 commits from branch
master
- 31f6fd19 - Improve add_concurrent_foreign_key to support custom target column
- 2f483c22 - Finalize conversion to bigint for events
-
0f5cc3fc...ea525f3b - 558 commits from branch
Database migrations
3 Warnings 20210622045705 - FinalizeEventsBigintConversion had a query that exceeded timing guidelines.
Run time should not exceed 100ms, but was it was 1693168.93ms. Please consider possible options to
improve the query performance.ALTER TABLE push_event_payloads VALIDATE CONSTRAINT
fk_36c74129da_tmp20210622045705 - FinalizeEventsBigintConversion took 28.5min. Please add a comment that
mentions Release Managers (@gitlab-org/release/managers
) so they are informed.20210622045705 - FinalizeEventsBigintConversion may need a background migration to comply
with timing guidelines. It took 28.5min, but should not exceed 10.0minMigrations included in this change have been executed on gitlab.com data for testing purposes. For details, please see the migration testing pipeline (limited access).
Migration Type Total runtime Result DB size change 20210622045705 - FinalizeEventsBigintConversion Post deploy 1710.6 s -56.19 GiB Runtime Histogram for all migrations
Query Runtime Count 0 seconds - 0.01 seconds 0 0.01 seconds - 0.1 seconds 100 0.1 seconds - 1 second 1 1 second - 5 minutes 1 5 minutes + 1 Migration: 20210622045705 - FinalizeEventsBigintConversion- Type: Post deploy
- Duration: 1710.6 s
- Database size change: -56.19 GiB
Query Calls Total Time Max Time Mean Time Rows ALTER TABLE push_event_payloads VALIDATE CONSTRAINT fk_36c74129da_tmp
1 1693168.9 ms 1693168.9 ms 1693168.9 ms 0 ALTER TABLE push_event_payloads ADD CONSTRAINT fk_36c74129da_tmp FOREIGN KEY (event_id) REFERENCES events (id_convert_to_bigint) ON DELETE CASCADE NOT VALID
1 38.7 ms 38.7 ms 38.7 ms 0 ALTER TABLE "events" ALTER COLUMN "id" SET DEFAULT nextval('events_id_seq'::regclass) /*application:test*/
1 31.0 ms 31.0 ms 31.0 ms 0 ALTER TABLE events DROP CONSTRAINT events_pkey CASCADE /*application:test*/
1 23.3 ms 23.3 ms 23.3 ms 0 ALTER SEQUENCE events_id_seq OWNED BY events.id /*application:test*/
1 17.4 ms 17.4 ms 17.4 ms 0 ALTER TABLE events ADD CONSTRAINT events_pkey PRIMARY KEY USING INDEX events_pkey /*application:test*/
1 16.2 ms 16.2 ms 16.2 ms 0 ALTER TABLE "events" RENAME COLUMN "id_convert_to_bigint" TO "id" /*application:test*/
1 14.6 ms 14.6 ms 14.6 ms 0 ALTER TABLE "events" RENAME COLUMN "id" TO "id_tmp" /*application:test*/
1 9.4 ms 9.4 ms 9.4 ms 0 ALTER FUNCTION "trigger_69523443cc10" RESET ALL /*application:test*/
1 4.6 ms 4.6 ms 4.6 ms 0 DROP INDEX index_events_on_project_id_and_id /*application:test*/
1 4.6 ms 4.6 ms 4.6 ms 0 DROP INDEX index_events_on_project_id_and_id_desc_on_merged_action /*application:test*/
1 4.5 ms 4.5 ms 4.5 ms 0 ALTER INDEX "index_events_on_id_convert_to_bigint" RENAME TO "events_pkey" /*application:test*/
1 4.2 ms 4.2 ms 4.2 ms 0 ALTER INDEX "index_events_on_project_id_and_id_convert_to_bigint" RENAME TO "index_events_on_project_id_and_id" /*application:test*/
1 3.5 ms 3.5 ms 3.5 ms 0 SELECT "batched_background_migrations".*
FROM "batched_background_migrations"
WHERE "batched_background_migrations"."job_class_name" = $1 AND "batched_background_migrations"."table_name" = $2 AND "batched_background_migrations"."column_name" = $3 AND (job_arguments = $4)
ORDER BY "batched_background_migrations"."id" ASC
LIMIT $5 /*application:test*/1 3.2 ms 3.2 ms 3.2 ms 1 ALTER TABLE "events" ALTER COLUMN "id_convert_to_bigint" SET DEFAULT 0 /*application:test*/
1 3.1 ms 3.1 ms 3.1 ms 0 ALTER TABLE "events" RENAME COLUMN "id_tmp" TO "id_convert_to_bigint" /*application:test*/
1 2.7 ms 2.7 ms 2.7 ms 0 LOCK TABLE events, push_event_payloads IN SHARE ROW EXCLUSIVE MODE /*application:test*/
1 0.4 ms 0.4 ms 0.4 ms 0 ALTER TABLE "push_event_payloads" RENAME CONSTRAINT "fk_36c74129da_tmp" TO "fk_36c74129da"/*application:test*/
1 0.4 ms 0.4 ms 0.4 ms 0 ALTER INDEX "index_events_on_project_id_and_id_bigint_desc_on_merged_action" RENAME TO "index_events_on_project_id_and_id_desc_on_merged_action" /*application:test*/
1 0.3 ms 0.3 ms 0.3 ms 0 LOCK TABLE events, push_event_payloads IN ACCESS EXCLUSIVE MODE /*application:test*/
1 0.2 ms 0.2 ms 0.2 ms 0 Histogram for FinalizeEventsBigintConversion
Query Runtime Count 0 seconds - 0.01 seconds 0 0.01 seconds - 0.1 seconds 100 0.1 seconds - 1 second 1 1 second - 5 minutes 1 5 minutes + 1
Other migrations pending on GitLab.com
Migration Type Total runtime Result DB size change 20210706112800 - RemoveCloudLicenseEnabledFromApplicationSettings Post deploy 1.3 s +0.00 B Clone Details
Clone ID Clone Created At Clone Data Timestamp Expected Removal Time database-testing-757600
2021-08-24 02:36:05 UTC 2021-08-24 00:01:03 UTC 2021-08-24 15:06:33 +0000 Artifacts
Brought to you by gitlab-org/database-team/gitlab-com-database-testing. Epic
- Resolved by Andreas Brandl
added workflowin review label and removed workflowin dev label
mentioned in issue gitlab-com/gl-infra/production#5034 (closed)
added typemaintenance label
added typefeature label
- A deleted user
added database-testing-automation label
- Resolved by Andreas Brandl
Created gitlab-com/gl-infra/production#5034 (closed) for creating indexes upfront
- Resolved by Andreas Brandl
Database looks good, too!
I only added a cosmetic suggestion.I'll get gitlab-com/gl-infra/production#5034 (closed) going and once indexes are there, we can merge here.
Thanks @krasio!
mentioned in issue #334823 (closed)
removed review request for @engwan
Indexes created per gitlab-com/gl-infra/production#5034 (closed)
marked the checklist item Execute upon gitlab-com/gl-infra/production#5034 (closed) as completed
- Resolved by Patrick Bair
@gitlab-org/release/managers Heads-up for this change per !64779 (comment 613785586)
We have extracted index creation into gitlab-com/gl-infra/production#5034 (closed), but adding the foreign key is still going to take a few minutes.
Based on the index creation times, I expect about 20 minutes for that. It's a post-deploy migration.
added 539 commits
-
0c130139...a5c95756 - 536 commits from branch
master
- 12a07084 - Improve add_concurrent_foreign_key to support custom target column
- 23e53a66 - Finalize conversion to bigint for events
- afc9b1e5 - Apply 2 suggestion(s) to 1 file(s)
Toggle commit list-
0c130139...a5c95756 - 536 commits from branch
I switched the draft status to avoid this being accidentally merged and moving to %14.2 per #288005 (comment 616613886)