From 9fadd0b863d796b876ed0f627323e663602a9acb Mon Sep 17 00:00:00 2001 From: Aakriti Gupta Date: Wed, 9 Jun 2021 17:32:01 +0200 Subject: [PATCH] Add migrations for adding verification for MR diffs - Add verification columns to merge_requst_diff_details and merge_request_diff_registry tables - Add indexes for new columns Changelog: changed --- ...and_started_at_to_mr_diff_details_table.rb | 10 ++++++ ...xes_to_merge_request_diff_details_table.rb | 26 +++++++++++++++ db/schema_migrations/20210504143128 | 1 + db/schema_migrations/20210505170152 | 1 + db/structure.sql | 10 ++++++ ...fication_to_merge_request_diff_registry.rb | 27 +++++++++++++++ ...es_to_merge_request_diff_registry_table.rb | 33 +++++++++++++++++++ ee/db/geo/schema.rb | 7 ++-- 8 files changed, 113 insertions(+), 2 deletions(-) create mode 100644 db/migrate/20210504143128_add_verification_state_and_started_at_to_mr_diff_details_table.rb create mode 100644 db/migrate/20210505170152_add_verification_indexes_to_merge_request_diff_details_table.rb create mode 100644 db/schema_migrations/20210504143128 create mode 100644 db/schema_migrations/20210505170152 create mode 100644 ee/db/geo/migrate/20210504143244_add_verification_to_merge_request_diff_registry.rb create mode 100644 ee/db/geo/migrate/20210505170208_add_indexes_to_merge_request_diff_registry_table.rb diff --git a/db/migrate/20210504143128_add_verification_state_and_started_at_to_mr_diff_details_table.rb b/db/migrate/20210504143128_add_verification_state_and_started_at_to_mr_diff_details_table.rb new file mode 100644 index 00000000000000..7999ea14a12658 --- /dev/null +++ b/db/migrate/20210504143128_add_verification_state_and_started_at_to_mr_diff_details_table.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class AddVerificationStateAndStartedAtToMrDiffDetailsTable < ActiveRecord::Migration[6.0] + def change + change_table(:merge_request_diff_details) do |t| + t.integer :verification_state, default: 0, limit: 2, null: false + t.column :verification_started_at, :datetime_with_timezone + end + end +end diff --git a/db/migrate/20210505170152_add_verification_indexes_to_merge_request_diff_details_table.rb b/db/migrate/20210505170152_add_verification_indexes_to_merge_request_diff_details_table.rb new file mode 100644 index 00000000000000..e85a28e3fa7ee4 --- /dev/null +++ b/db/migrate/20210505170152_add_verification_indexes_to_merge_request_diff_details_table.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class AddVerificationIndexesToMergeRequestDiffDetailsTable < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + VERIFICATION_STATE_INDEX_NAME = "index_merge_request_diff_details_on_verification_state" + PENDING_VERIFICATION_INDEX_NAME = "index_merge_request_diff_details_pending_verification" + FAILED_VERIFICATION_INDEX_NAME = "index_merge_request_diff_details_failed_verification" + NEEDS_VERIFICATION_INDEX_NAME = "index_merge_request_diff_details_needs_verification" + + disable_ddl_transaction! + + def up + add_concurrent_index :merge_request_diff_details, :verification_state, name: VERIFICATION_STATE_INDEX_NAME + add_concurrent_index :merge_request_diff_details, :verified_at, where: "(verification_state = 0)", order: { verified_at: 'ASC NULLS FIRST' }, name: PENDING_VERIFICATION_INDEX_NAME + add_concurrent_index :merge_request_diff_details, :verification_retry_at, where: "(verification_state = 3)", order: { verification_retry_at: 'ASC NULLS FIRST' }, name: FAILED_VERIFICATION_INDEX_NAME + add_concurrent_index :merge_request_diff_details, :verification_state, where: "(verification_state = 0 OR verification_state = 3)", name: NEEDS_VERIFICATION_INDEX_NAME + end + + def down + remove_concurrent_index_by_name :merge_request_diff_details, VERIFICATION_STATE_INDEX_NAME + remove_concurrent_index_by_name :merge_request_diff_details, PENDING_VERIFICATION_INDEX_NAME + remove_concurrent_index_by_name :merge_request_diff_details, FAILED_VERIFICATION_INDEX_NAME + remove_concurrent_index_by_name :merge_request_diff_details, NEEDS_VERIFICATION_INDEX_NAME + end +end diff --git a/db/schema_migrations/20210504143128 b/db/schema_migrations/20210504143128 new file mode 100644 index 00000000000000..425120e633b71e --- /dev/null +++ b/db/schema_migrations/20210504143128 @@ -0,0 +1 @@ +e5b552b21c40b83b95442341838ad5951dcac7dd473194c49630d20ce6a46ae2 \ No newline at end of file diff --git a/db/schema_migrations/20210505170152 b/db/schema_migrations/20210505170152 new file mode 100644 index 00000000000000..d51d04bc6ab978 --- /dev/null +++ b/db/schema_migrations/20210505170152 @@ -0,0 +1 @@ +9b16e17189d4db708553ce0d9dada1ce097be75433c3a8c09a6102e897e3123a \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 192da6b04bb801..7d489fc3be5a48 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14626,6 +14626,8 @@ CREATE TABLE merge_request_diff_details ( verification_retry_count smallint, verification_checksum bytea, verification_failure text, + verification_state smallint DEFAULT 0 NOT NULL, + verification_started_at timestamp with time zone, CONSTRAINT check_81429e3622 CHECK ((char_length(verification_failure) <= 255)) ); @@ -23700,8 +23702,16 @@ CREATE UNIQUE INDEX index_merge_request_cleanup_schedules_on_merge_request_id ON CREATE INDEX index_merge_request_diff_commits_on_sha ON merge_request_diff_commits USING btree (sha); +CREATE INDEX index_merge_request_diff_details_failed_verification ON merge_request_diff_details USING btree (verification_retry_at NULLS FIRST) WHERE (verification_state = 3); + +CREATE INDEX index_merge_request_diff_details_needs_verification ON merge_request_diff_details USING btree (verification_state) WHERE ((verification_state = 0) OR (verification_state = 3)); + CREATE INDEX index_merge_request_diff_details_on_merge_request_diff_id ON merge_request_diff_details USING btree (merge_request_diff_id); +CREATE INDEX index_merge_request_diff_details_on_verification_state ON merge_request_diff_details USING btree (verification_state); + +CREATE INDEX index_merge_request_diff_details_pending_verification ON merge_request_diff_details USING btree (verified_at NULLS FIRST) WHERE (verification_state = 0); + CREATE INDEX index_merge_request_diffs_by_id_partial ON merge_request_diffs USING btree (id) WHERE ((files_count > 0) AND ((NOT stored_externally) OR (stored_externally IS NULL))); CREATE INDEX index_merge_request_diffs_on_external_diff_store ON merge_request_diffs USING btree (external_diff_store); diff --git a/ee/db/geo/migrate/20210504143244_add_verification_to_merge_request_diff_registry.rb b/ee/db/geo/migrate/20210504143244_add_verification_to_merge_request_diff_registry.rb new file mode 100644 index 00000000000000..defca641b6073b --- /dev/null +++ b/ee/db/geo/migrate/20210504143244_add_verification_to_merge_request_diff_registry.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class AddVerificationToMergeRequestDiffRegistry < ActiveRecord::Migration[6.0] + REGISTRY = :merge_request_diff_registry + + def up + add_column_unless_exists :verification_started_at, :datetime_with_timezone + add_column_unless_exists :verified_at, :datetime_with_timezone + add_column_unless_exists :verification_retry_at, :datetime_with_timezone + add_column_unless_exists :verification_retry_count, :integer + add_column_unless_exists :verification_state, :integer, limit: 2, default: 0, null: false + add_column_unless_exists :checksum_mismatch, :boolean + add_column_unless_exists :verification_checksum, :binary + add_column_unless_exists :verification_checksum_mismatched, :binary + add_column_unless_exists :verification_failure, :string, limit: 255 # rubocop:disable Migration/PreventStrings because https://gitlab.com/gitlab-org/gitlab/-/issues/323806 + end + + def add_column_unless_exists(column, *args) + return if column_exists?(:merge_request_diff_registry, column) + + add_column REGISTRY, column, *args + end + + def down + # no-op + end +end diff --git a/ee/db/geo/migrate/20210505170208_add_indexes_to_merge_request_diff_registry_table.rb b/ee/db/geo/migrate/20210505170208_add_indexes_to_merge_request_diff_registry_table.rb new file mode 100644 index 00000000000000..6d65e14a1122e7 --- /dev/null +++ b/ee/db/geo/migrate/20210505170208_add_indexes_to_merge_request_diff_registry_table.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +class AddIndexesToMergeRequestDiffRegistryTable < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + MERGE_REQUEST_DIFF_ID_INDEX_NAME = "index_merge_request_diff_registry_on_mr_diff_id" + FAILED_VERIFICATION_INDEX_NAME = "merge_request_diff_registry_failed_verification" + NEEDS_VERIFICATION_INDEX_NAME = "merge_request_diff_registry_needs_verification" + PENDING_VERIFICATION_INDEX_NAME = "merge_request_diff_registry_pending_verification" + + REGISTRY_TABLE = :merge_request_diff_registry + + disable_ddl_transaction! + + def up + # Re-adding index with `unique` constraint + remove_concurrent_index_by_name REGISTRY_TABLE, name: MERGE_REQUEST_DIFF_ID_INDEX_NAME + + add_concurrent_index REGISTRY_TABLE, :merge_request_diff_id, name: MERGE_REQUEST_DIFF_ID_INDEX_NAME, unique: true + add_concurrent_index REGISTRY_TABLE, :verification_retry_at, name: FAILED_VERIFICATION_INDEX_NAME, order: "NULLS FIRST", where: "((state = 2) AND (verification_state = 3))" + add_concurrent_index REGISTRY_TABLE, :verification_state, name: NEEDS_VERIFICATION_INDEX_NAME, where: "((state = 2) AND (verification_state = ANY (ARRAY[0, 3])))" + add_concurrent_index REGISTRY_TABLE, :verified_at, name: PENDING_VERIFICATION_INDEX_NAME, order: "NULLS FIRST", where: "((state = 2) AND (verification_state = 0))" + end + + def down + remove_concurrent_index_by_name REGISTRY_TABLE, name: MERGE_REQUEST_DIFF_ID_INDEX_NAME + add_concurrent_index REGISTRY_TABLE, :merge_request_diff_id, name: MERGE_REQUEST_DIFF_ID_INDEX_NAME + + remove_concurrent_index_by_name REGISTRY_TABLE, name: FAILED_VERIFICATION_INDEX_NAME + remove_concurrent_index_by_name REGISTRY_TABLE, name: NEEDS_VERIFICATION_INDEX_NAME + remove_concurrent_index_by_name REGISTRY_TABLE, name: PENDING_VERIFICATION_INDEX_NAME + end +end diff --git a/ee/db/geo/schema.rb b/ee/db/geo/schema.rb index 8f4dfe823e9b40..91b902b10da230 100644 --- a/ee/db/geo/schema.rb +++ b/ee/db/geo/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_04_20_180119) do +ActiveRecord::Schema.define(version: 2021_05_05_170208) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -127,9 +127,12 @@ t.binary "verification_checksum" t.binary "verification_checksum_mismatched" t.string "verification_failure", limit: 255 - t.index ["merge_request_diff_id"], name: "index_merge_request_diff_registry_on_mr_diff_id" + t.index ["merge_request_diff_id"], name: "index_merge_request_diff_registry_on_mr_diff_id", unique: true t.index ["retry_at"], name: "index_merge_request_diff_registry_on_retry_at" t.index ["state"], name: "index_merge_request_diff_registry_on_state" + t.index ["verification_retry_at"], name: "merge_request_diff_registry_failed_verification", order: "NULLS FIRST", where: "((state = 2) AND (verification_state = 3))" + t.index ["verification_state"], name: "merge_request_diff_registry_needs_verification", where: "((state = 2) AND (verification_state = ANY (ARRAY[0, 3])))" + t.index ["verified_at"], name: "merge_request_diff_registry_pending_verification", order: "NULLS FIRST", where: "((state = 2) AND (verification_state = 0))" end create_table "package_file_registry", id: :serial, force: :cascade do |t| -- GitLab