From 9997c58fc0db6469cde1a21428e050aa7b550a9a Mon Sep 17 00:00:00 2001 From: blackst0ne <blackst0ne.ru@gmail.com> Date: Thu, 6 Apr 2017 09:53:57 +1100 Subject: [PATCH] Add remove_concurrent_index to database helper --- ...ve_concurrent_index_to_database_helper.yml | 4 ++ ...10_add_index_on_requested_at_to_members.rb | 1 + ...remove_keys_fingerprint_index_if_exists.rb | 1 + ...48_add_unique_index_to_keys_fingerprint.rb | 1 + ...60620115026_add_index_on_runners_locked.rb | 1 + ...15134306_add_index_for_pipeline_user_id.rb | 1 + ...25104020_merge_request_diff_remove_uniq.rb | 1 + ...0725104452_merge_request_diff_add_index.rb | 1 + ..._remove_builds_enable_index_on_projects.rb | 1 + ...1903_add_unique_index_to_lists_label_id.rb | 1 + ...0805041956_add_deleted_at_to_namespaces.rb | 1 + ...0160808085602_add_index_for_build_token.rb | 1 + ...20160810142633_remove_redundant_indexes.rb | 1 + ...9221631_add_index_to_note_discussion_id.rb | 1 + ...32256_add_incoming_email_token_to_users.rb | 1 + .../20160919145149_add_group_id_to_labels.rb | 1 + ...0160920160832_add_index_to_labels_title.rb | 1 + ...161017125927_add_unique_index_to_labels.rb | 1 + ...dd_pipeline_id_to_merge_request_metrics.rb | 1 + ...81638_add_unique_index_to_subscriptions.rb | 1 + ...0_add_project_import_data_project_index.rb | 1 + .../20161124111395_add_index_to_parent_id.rb | 1 + ...161128142110_remove_unnecessary_indexes.rb | 1 + .../20161202152035_add_index_to_routes.rb | 1 + ...9_remove_uniq_path_index_from_namespace.rb | 1 + ...61206153751_add_path_index_to_namespace.rb | 1 + ...3_remove_uniq_name_index_from_namespace.rb | 1 + ...61206153754_add_name_index_to_namespace.rb | 1 + ...21_create_environment_name_unique_index.rb | 1 + ...0_add_unique_index_for_environment_slug.rb | 1 + ...12142807_add_lower_path_index_to_routes.rb | 1 + ...ci_builds_for_status_runner_id_and_type.rb | 1 + ...5_add_index_to_ci_runners_for_is_shared.rb | 1 + ...620_add_index_to_project_authorizations.rb | 1 + ...1221752_add_relative_position_to_issues.rb | 1 + ...dd_index_to_labels_for_type_and_project.rb | 1 + ...d_index_to_labels_for_title_and_project.rb | 1 + ...ex_to_ci_trigger_requests_for_commit_id.rb | 1 + ...10103609_add_index_to_user_agent_detail.rb | 1 + ...dd_index_for_latest_successful_pipeline.rb | 1 + ...40_drop_index_for_builds_project_status.rb | 1 + ...222143500_remove_old_project_id_columns.rb | 1 + .../20170313213916_add_index_to_user_ghost.rb | 1 + doc/development/migration_style_guide.md | 16 ++++++- .../migration/create_table_migration.rb | 14 ++++--- .../active_record/migration/migration.rb | 14 ++++--- .../post_deployment_migration/migration.rb | 14 ++++--- lib/gitlab/database/migration_helpers.rb | 24 +++++++++++ rubocop/cop/migration/add_concurrent_index.rb | 2 +- .../cop/migration/remove_concurrent_index.rb | 29 +++++++++++++ rubocop/cop/migration/remove_index.rb | 26 ++++++++++++ rubocop/rubocop.rb | 2 + .../gitlab/database/migration_helpers_spec.rb | 42 +++++++++++++++++++ .../migration/remove_concurrent_index_spec.rb | 41 ++++++++++++++++++ .../cop/migration/remove_index_spec.rb | 35 ++++++++++++++++ 55 files changed, 284 insertions(+), 21 deletions(-) create mode 100644 changelogs/unreleased/add_remove_concurrent_index_to_database_helper.yml create mode 100644 rubocop/cop/migration/remove_concurrent_index.rb create mode 100644 rubocop/cop/migration/remove_index.rb create mode 100644 spec/rubocop/cop/migration/remove_concurrent_index_spec.rb create mode 100644 spec/rubocop/cop/migration/remove_index_spec.rb diff --git a/changelogs/unreleased/add_remove_concurrent_index_to_database_helper.yml b/changelogs/unreleased/add_remove_concurrent_index_to_database_helper.yml new file mode 100644 index 0000000000000..c7b06e4560789 --- /dev/null +++ b/changelogs/unreleased/add_remove_concurrent_index_to_database_helper.yml @@ -0,0 +1,4 @@ +--- +title: Add remove_concurrent_index to database helper +merge_request: 10441 +author: blackst0ne diff --git a/db/migrate/20160615142710_add_index_on_requested_at_to_members.rb b/db/migrate/20160615142710_add_index_on_requested_at_to_members.rb index 7a8ed99c68f01..178e4bf5ed376 100644 --- a/db/migrate/20160615142710_add_index_on_requested_at_to_members.rb +++ b/db/migrate/20160615142710_add_index_on_requested_at_to_members.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddIndexOnRequestedAtToMembers < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160616103005_remove_keys_fingerprint_index_if_exists.rb b/db/migrate/20160616103005_remove_keys_fingerprint_index_if_exists.rb index 4bb4204cebda0..081df23f394dd 100644 --- a/db/migrate/20160616103005_remove_keys_fingerprint_index_if_exists.rb +++ b/db/migrate/20160616103005_remove_keys_fingerprint_index_if_exists.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class RemoveKeysFingerprintIndexIfExists < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160616103948_add_unique_index_to_keys_fingerprint.rb b/db/migrate/20160616103948_add_unique_index_to_keys_fingerprint.rb index e35af38aac3a6..76bb6a096391f 100644 --- a/db/migrate/20160616103948_add_unique_index_to_keys_fingerprint.rb +++ b/db/migrate/20160616103948_add_unique_index_to_keys_fingerprint.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddUniqueIndexToKeysFingerprint < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160620115026_add_index_on_runners_locked.rb b/db/migrate/20160620115026_add_index_on_runners_locked.rb index 6ca486c63d1ea..48f4495b0a47e 100644 --- a/db/migrate/20160620115026_add_index_on_runners_locked.rb +++ b/db/migrate/20160620115026_add_index_on_runners_locked.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class AddIndexOnRunnersLocked < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb b/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb index a05a4c679e3e9..fec17ffb7f6c2 100644 --- a/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb +++ b/db/migrate/20160715134306_add_index_for_pipeline_user_id.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddIndexForPipelineUserId < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160725104020_merge_request_diff_remove_uniq.rb b/db/migrate/20160725104020_merge_request_diff_remove_uniq.rb index 75a3eb15124cf..12e11bc3fbe45 100644 --- a/db/migrate/20160725104020_merge_request_diff_remove_uniq.rb +++ b/db/migrate/20160725104020_merge_request_diff_remove_uniq.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class MergeRequestDiffRemoveUniq < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20160725104452_merge_request_diff_add_index.rb b/db/migrate/20160725104452_merge_request_diff_add_index.rb index 6d04242dd25af..60d81e0bdc079 100644 --- a/db/migrate/20160725104452_merge_request_diff_add_index.rb +++ b/db/migrate/20160725104452_merge_request_diff_add_index.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class MergeRequestDiffAddIndex < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20160802010328_remove_builds_enable_index_on_projects.rb b/db/migrate/20160802010328_remove_builds_enable_index_on_projects.rb index 5fd51cb65f16f..6d7733762c811 100644 --- a/db/migrate/20160802010328_remove_builds_enable_index_on_projects.rb +++ b/db/migrate/20160802010328_remove_builds_enable_index_on_projects.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class RemoveBuildsEnableIndexOnProjects < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160803161903_add_unique_index_to_lists_label_id.rb b/db/migrate/20160803161903_add_unique_index_to_lists_label_id.rb index baf2e70b12769..9c1511963f78c 100644 --- a/db/migrate/20160803161903_add_unique_index_to_lists_label_id.rb +++ b/db/migrate/20160803161903_add_unique_index_to_lists_label_id.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddUniqueIndexToListsLabelId < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb b/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb index 3f074723b4ac5..30d98a0124e81 100644 --- a/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb +++ b/db/migrate/20160805041956_add_deleted_at_to_namespaces.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddDeletedAtToNamespaces < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160808085602_add_index_for_build_token.rb b/db/migrate/20160808085602_add_index_for_build_token.rb index 6c5d7268e7201..0446b2f2e15ab 100644 --- a/db/migrate/20160808085602_add_index_for_build_token.rb +++ b/db/migrate/20160808085602_add_index_for_build_token.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddIndexForBuildToken < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160810142633_remove_redundant_indexes.rb b/db/migrate/20160810142633_remove_redundant_indexes.rb index 8641c6ffa8f9f..d7ab022d7bc06 100644 --- a/db/migrate/20160810142633_remove_redundant_indexes.rb +++ b/db/migrate/20160810142633_remove_redundant_indexes.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class RemoveRedundantIndexes < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160819221631_add_index_to_note_discussion_id.rb b/db/migrate/20160819221631_add_index_to_note_discussion_id.rb index 8f693e97a5895..843643c4e9533 100644 --- a/db/migrate/20160819221631_add_index_to_note_discussion_id.rb +++ b/db/migrate/20160819221631_add_index_to_note_discussion_id.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class AddIndexToNoteDiscussionId < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160819232256_add_incoming_email_token_to_users.rb b/db/migrate/20160819232256_add_incoming_email_token_to_users.rb index bcad3416d043a..a004a3802a250 100644 --- a/db/migrate/20160819232256_add_incoming_email_token_to_users.rb +++ b/db/migrate/20160819232256_add_incoming_email_token_to_users.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class AddIncomingEmailTokenToUsers < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160919145149_add_group_id_to_labels.rb b/db/migrate/20160919145149_add_group_id_to_labels.rb index e20e693f3aa73..917c2b0c521f3 100644 --- a/db/migrate/20160919145149_add_group_id_to_labels.rb +++ b/db/migrate/20160919145149_add_group_id_to_labels.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddGroupIdToLabels < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20160920160832_add_index_to_labels_title.rb b/db/migrate/20160920160832_add_index_to_labels_title.rb index 19f7b1076a7be..e38c655baeee1 100644 --- a/db/migrate/20160920160832_add_index_to_labels_title.rb +++ b/db/migrate/20160920160832_add_index_to_labels_title.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddIndexToLabelsTitle < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161017125927_add_unique_index_to_labels.rb b/db/migrate/20161017125927_add_unique_index_to_labels.rb index f2b56ebfb7b5e..b8f6a803a0a4f 100644 --- a/db/migrate/20161017125927_add_unique_index_to_labels.rb +++ b/db/migrate/20161017125927_add_unique_index_to_labels.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddUniqueIndexToLabels < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb b/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb index 35ad22b6c0138..b77daf12f6822 100644 --- a/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb +++ b/db/migrate/20161020083353_add_pipeline_id_to_merge_request_metrics.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class AddPipelineIdToMergeRequestMetrics < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161031181638_add_unique_index_to_subscriptions.rb b/db/migrate/20161031181638_add_unique_index_to_subscriptions.rb index 4b1b29e12650e..f263377fbc68a 100644 --- a/db/migrate/20161031181638_add_unique_index_to_subscriptions.rb +++ b/db/migrate/20161031181638_add_unique_index_to_subscriptions.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddUniqueIndexToSubscriptions < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161106185620_add_project_import_data_project_index.rb b/db/migrate/20161106185620_add_project_import_data_project_index.rb index 94b8ddd46f537..b3746dc4f6c57 100644 --- a/db/migrate/20161106185620_add_project_import_data_project_index.rb +++ b/db/migrate/20161106185620_add_project_import_data_project_index.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddProjectImportDataProjectIndex < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161124111395_add_index_to_parent_id.rb b/db/migrate/20161124111395_add_index_to_parent_id.rb index 73f9d92bb2275..065643e058d0e 100644 --- a/db/migrate/20161124111395_add_index_to_parent_id.rb +++ b/db/migrate/20161124111395_add_index_to_parent_id.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class AddIndexToParentId < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161128142110_remove_unnecessary_indexes.rb b/db/migrate/20161128142110_remove_unnecessary_indexes.rb index 8100287ef48b1..699a9368eb313 100644 --- a/db/migrate/20161128142110_remove_unnecessary_indexes.rb +++ b/db/migrate/20161128142110_remove_unnecessary_indexes.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class RemoveUnnecessaryIndexes < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20161202152035_add_index_to_routes.rb b/db/migrate/20161202152035_add_index_to_routes.rb index 6d6c8906204a2..552b5fab68c2f 100644 --- a/db/migrate/20161202152035_add_index_to_routes.rb +++ b/db/migrate/20161202152035_add_index_to_routes.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class AddIndexToRoutes < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161206153749_remove_uniq_path_index_from_namespace.rb b/db/migrate/20161206153749_remove_uniq_path_index_from_namespace.rb index 2977917f2d105..7d39c2ae6268e 100644 --- a/db/migrate/20161206153749_remove_uniq_path_index_from_namespace.rb +++ b/db/migrate/20161206153749_remove_uniq_path_index_from_namespace.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class RemoveUniqPathIndexFromNamespace < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161206153751_add_path_index_to_namespace.rb b/db/migrate/20161206153751_add_path_index_to_namespace.rb index b0bac7d121e37..623037e35cdad 100644 --- a/db/migrate/20161206153751_add_path_index_to_namespace.rb +++ b/db/migrate/20161206153751_add_path_index_to_namespace.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class AddPathIndexToNamespace < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161206153753_remove_uniq_name_index_from_namespace.rb b/db/migrate/20161206153753_remove_uniq_name_index_from_namespace.rb index cc9d4974baaff..9296ae36aa562 100644 --- a/db/migrate/20161206153753_remove_uniq_name_index_from_namespace.rb +++ b/db/migrate/20161206153753_remove_uniq_name_index_from_namespace.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class RemoveUniqNameIndexFromNamespace < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161206153754_add_name_index_to_namespace.rb b/db/migrate/20161206153754_add_name_index_to_namespace.rb index b3f3cb68a99d7..2bbd039ff2756 100644 --- a/db/migrate/20161206153754_add_name_index_to_namespace.rb +++ b/db/migrate/20161206153754_add_name_index_to_namespace.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class AddNameIndexToNamespace < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161207231621_create_environment_name_unique_index.rb b/db/migrate/20161207231621_create_environment_name_unique_index.rb index 5ff0f5bae4d4b..15093350f1204 100644 --- a/db/migrate/20161207231621_create_environment_name_unique_index.rb +++ b/db/migrate/20161207231621_create_environment_name_unique_index.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class CreateEnvironmentNameUniqueIndex < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161209153400_add_unique_index_for_environment_slug.rb b/db/migrate/20161209153400_add_unique_index_for_environment_slug.rb index ede0316e86076..42a90091b876f 100644 --- a/db/migrate/20161209153400_add_unique_index_for_environment_slug.rb +++ b/db/migrate/20161209153400_add_unique_index_for_environment_slug.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class AddUniqueIndexForEnvironmentSlug < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20161212142807_add_lower_path_index_to_routes.rb b/db/migrate/20161212142807_add_lower_path_index_to_routes.rb index 53f4c6bbb18a8..76db517979527 100644 --- a/db/migrate/20161212142807_add_lower_path_index_to_routes.rb +++ b/db/migrate/20161212142807_add_lower_path_index_to_routes.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class AddLowerPathIndexToRoutes < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170121123724_add_index_to_ci_builds_for_status_runner_id_and_type.rb b/db/migrate/20170121123724_add_index_to_ci_builds_for_status_runner_id_and_type.rb index 4ea953f2b78ad..c006098fafd07 100644 --- a/db/migrate/20170121123724_add_index_to_ci_builds_for_status_runner_id_and_type.rb +++ b/db/migrate/20170121123724_add_index_to_ci_builds_for_status_runner_id_and_type.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddIndexToCiBuildsForStatusRunnerIdAndType < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170121130655_add_index_to_ci_runners_for_is_shared.rb b/db/migrate/20170121130655_add_index_to_ci_runners_for_is_shared.rb index 620befcf4d77c..00aa0b311b139 100644 --- a/db/migrate/20170121130655_add_index_to_ci_runners_for_is_shared.rb +++ b/db/migrate/20170121130655_add_index_to_ci_runners_for_is_shared.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddIndexToCiRunnersForIsShared < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170130204620_add_index_to_project_authorizations.rb b/db/migrate/20170130204620_add_index_to_project_authorizations.rb index 629b49436e3ac..aded62e08b3aa 100644 --- a/db/migrate/20170130204620_add_index_to_project_authorizations.rb +++ b/db/migrate/20170130204620_add_index_to_project_authorizations.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddIndexToProjectAuthorizations < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170131221752_add_relative_position_to_issues.rb b/db/migrate/20170131221752_add_relative_position_to_issues.rb index 1baad0893e345..fd18d8b6a60f1 100644 --- a/db/migrate/20170131221752_add_relative_position_to_issues.rb +++ b/db/migrate/20170131221752_add_relative_position_to_issues.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class AddRelativePositionToIssues < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170204181513_add_index_to_labels_for_type_and_project.rb b/db/migrate/20170204181513_add_index_to_labels_for_type_and_project.rb index 31ef458c44f6e..b1b0a60100771 100644 --- a/db/migrate/20170204181513_add_index_to_labels_for_type_and_project.rb +++ b/db/migrate/20170204181513_add_index_to_labels_for_type_and_project.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddIndexToLabelsForTypeAndProject < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170210062829_add_index_to_labels_for_title_and_project.rb b/db/migrate/20170210062829_add_index_to_labels_for_title_and_project.rb index 70fb0ef12f9bf..2c20f6a48ab04 100644 --- a/db/migrate/20170210062829_add_index_to_labels_for_title_and_project.rb +++ b/db/migrate/20170210062829_add_index_to_labels_for_title_and_project.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddIndexToLabelsForTitleAndProject < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170210075922_add_index_to_ci_trigger_requests_for_commit_id.rb b/db/migrate/20170210075922_add_index_to_ci_trigger_requests_for_commit_id.rb index 07d4f8af27fe5..c31057f2617c4 100644 --- a/db/migrate/20170210075922_add_index_to_ci_trigger_requests_for_commit_id.rb +++ b/db/migrate/20170210075922_add_index_to_ci_trigger_requests_for_commit_id.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddIndexToCiTriggerRequestsForCommitId < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170210103609_add_index_to_user_agent_detail.rb b/db/migrate/20170210103609_add_index_to_user_agent_detail.rb index 2d8329b786217..ba4976a5ce8e2 100644 --- a/db/migrate/20170210103609_add_index_to_user_agent_detail.rb +++ b/db/migrate/20170210103609_add_index_to_user_agent_detail.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class AddIndexToUserAgentDetail < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/db/migrate/20170216135621_add_index_for_latest_successful_pipeline.rb b/db/migrate/20170216135621_add_index_for_latest_successful_pipeline.rb index 8a96a784c9727..884c4e569d685 100644 --- a/db/migrate/20170216135621_add_index_for_latest_successful_pipeline.rb +++ b/db/migrate/20170216135621_add_index_for_latest_successful_pipeline.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class AddIndexForLatestSuccessfulPipeline < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers DOWNTIME = false diff --git a/db/migrate/20170216141440_drop_index_for_builds_project_status.rb b/db/migrate/20170216141440_drop_index_for_builds_project_status.rb index a2839f52d89c1..56ad566ca6760 100644 --- a/db/migrate/20170216141440_drop_index_for_builds_project_status.rb +++ b/db/migrate/20170216141440_drop_index_for_builds_project_status.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class DropIndexForBuildsProjectStatus < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers DOWNTIME = false diff --git a/db/migrate/20170222143500_remove_old_project_id_columns.rb b/db/migrate/20170222143500_remove_old_project_id_columns.rb index eac93e8e407c1..268144a25529a 100644 --- a/db/migrate/20170222143500_remove_old_project_id_columns.rb +++ b/db/migrate/20170222143500_remove_old_project_id_columns.rb @@ -1,3 +1,4 @@ +# rubocop:disable RemoveIndex class RemoveOldProjectIdColumns < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers disable_ddl_transaction! diff --git a/db/migrate/20170313213916_add_index_to_user_ghost.rb b/db/migrate/20170313213916_add_index_to_user_ghost.rb index c429039c275a2..fe5847ed225f4 100644 --- a/db/migrate/20170313213916_add_index_to_user_ghost.rb +++ b/db/migrate/20170313213916_add_index_to_user_ghost.rb @@ -1,6 +1,7 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. +# rubocop:disable RemoveIndex class AddIndexToUserGhost < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md index fd8335d251e25..587922d0136fe 100644 --- a/doc/development/migration_style_guide.md +++ b/doc/development/migration_style_guide.md @@ -58,10 +58,22 @@ migration was tested. ## Removing indices -If you need to remove index, please add a condition like in following example: +When removing an index make sure to use the method `remove_concurrent_index` instead +of the regular `remove_index` method. The `remove_concurrent_index` method +automatically drops concurrent indexes when using PostgreSQL, removing the +need for downtime. To use this method you must disable transactions by calling +the method `disable_ddl_transaction!` in the body of your migration class like +so: ```ruby -remove_index :namespaces, column: :name if index_exists?(:namespaces, :name) +class MyMigration < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + + def up + remove_concurrent_index :table_name, :column_name if index_exists?(:table_name, :column_name) + end +end ``` ## Adding indices diff --git a/generator_templates/active_record/migration/create_table_migration.rb b/generator_templates/active_record/migration/create_table_migration.rb index aad8626a72063..59a9d37df0fc4 100644 --- a/generator_templates/active_record/migration/create_table_migration.rb +++ b/generator_templates/active_record/migration/create_table_migration.rb @@ -12,12 +12,14 @@ class <%= migration_class_name %> < ActiveRecord::Migration # migration requires downtime. # DOWNTIME_REASON = '' - # When using the methods "add_concurrent_index" or "add_column_with_default" - # you must disable the use of transactions as these methods can not run in an - # existing transaction. When using "add_concurrent_index" make sure that this - # method is the _only_ method called in the migration, any other changes - # should go in a separate migration. This ensures that upon failure _only_ the - # index creation fails and can be retried or reverted easily. + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. # # To disable transactions uncomment the following line and remove these # comments: diff --git a/generator_templates/active_record/migration/migration.rb b/generator_templates/active_record/migration/migration.rb index 825bc8bdf6154..08752b3af5057 100644 --- a/generator_templates/active_record/migration/migration.rb +++ b/generator_templates/active_record/migration/migration.rb @@ -12,12 +12,14 @@ class <%= migration_class_name %> < ActiveRecord::Migration # migration requires downtime. # DOWNTIME_REASON = '' - # When using the methods "add_concurrent_index" or "add_column_with_default" - # you must disable the use of transactions as these methods can not run in an - # existing transaction. When using "add_concurrent_index" make sure that this - # method is the _only_ method called in the migration, any other changes - # should go in a separate migration. This ensures that upon failure _only_ the - # index creation fails and can be retried or reverted easily. + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. # # To disable transactions uncomment the following line and remove these # comments: diff --git a/generator_templates/rails/post_deployment_migration/migration.rb b/generator_templates/rails/post_deployment_migration/migration.rb index 1a7b8d5bf3510..f2dff84b61837 100644 --- a/generator_templates/rails/post_deployment_migration/migration.rb +++ b/generator_templates/rails/post_deployment_migration/migration.rb @@ -6,12 +6,14 @@ class <%= migration_class_name %> < ActiveRecord::Migration DOWNTIME = false - # When using the methods "add_concurrent_index" or "add_column_with_default" - # you must disable the use of transactions as these methods can not run in an - # existing transaction. When using "add_concurrent_index" make sure that this - # method is the _only_ method called in the migration, any other changes - # should go in a separate migration. This ensures that upon failure _only_ the - # index creation fails and can be retried or reverted easily. + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. # # To disable transactions uncomment the following line and remove these # comments: diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index fc445ab948312..525aa9203285d 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -26,6 +26,30 @@ def add_concurrent_index(table_name, column_name, options = {}) add_index(table_name, column_name, options) end + # Removes an existed index, concurrently when supported + # + # On PostgreSQL this method removes an index concurrently. + # + # Example: + # + # remove_concurrent_index :users, :some_column + # + # See Rails' `remove_index` for more info on the available arguments. + def remove_concurrent_index(table_name, column_name, options = {}) + if transaction_open? + raise 'remove_concurrent_index can not be run inside a transaction, ' \ + 'you can disable transactions by calling disable_ddl_transaction! ' \ + 'in the body of your migration class' + end + + if Database.postgresql? + options = options.merge({ algorithm: :concurrently }) + disable_statement_timeout + end + + remove_index(table_name, options.merge({ column: column_name })) + end + # Adds a foreign key with only minimal locking on the tables involved. # # This method only requires minimal locking when using PostgreSQL. When diff --git a/rubocop/cop/migration/add_concurrent_index.rb b/rubocop/cop/migration/add_concurrent_index.rb index 332fb7dcbd796..69852f4d58026 100644 --- a/rubocop/cop/migration/add_concurrent_index.rb +++ b/rubocop/cop/migration/add_concurrent_index.rb @@ -9,7 +9,7 @@ class AddConcurrentIndex < RuboCop::Cop::Cop include MigrationHelpers MSG = '`add_concurrent_index` is not reversible so you must manually define ' \ - 'the `up` and `down` methods in your migration class, using `remove_index` in `down`'.freeze + 'the `up` and `down` methods in your migration class, using `remove_concurrent_index` in `down`'.freeze def on_send(node) return unless in_migration?(node) diff --git a/rubocop/cop/migration/remove_concurrent_index.rb b/rubocop/cop/migration/remove_concurrent_index.rb new file mode 100644 index 0000000000000..268c49865cb16 --- /dev/null +++ b/rubocop/cop/migration/remove_concurrent_index.rb @@ -0,0 +1,29 @@ +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks if `remove_concurrent_index` is used with `up`/`down` methods + # and not `change`. + class RemoveConcurrentIndex < RuboCop::Cop::Cop + include MigrationHelpers + + MSG = '`remove_concurrent_index` is not reversible so you must manually define ' \ + 'the `up` and `down` methods in your migration class, using `add_concurrent_index` in `down`'.freeze + + def on_send(node) + return unless in_migration?(node) + return unless node.children[1] == :remove_concurrent_index + + node.each_ancestor(:def) do |def_node| + add_offense(def_node, :name) if method_name(def_node) == :change + end + end + + def method_name(node) + node.children[0] + end + end + end + end +end diff --git a/rubocop/cop/migration/remove_index.rb b/rubocop/cop/migration/remove_index.rb new file mode 100644 index 0000000000000..613b35dd00df7 --- /dev/null +++ b/rubocop/cop/migration/remove_index.rb @@ -0,0 +1,26 @@ +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + # Cop that checks if indexes are removed in a concurrent manner. + class RemoveIndex < RuboCop::Cop::Cop + include MigrationHelpers + + MSG = '`remove_index` requires downtime, use `remove_concurrent_index` instead'.freeze + + def on_def(node) + return unless in_migration?(node) + + node.each_descendant(:send) do |send_node| + add_offense(send_node, :selector) if method_name(send_node) == :remove_index + end + end + + def method_name(node) + node.children[1] + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index a50a522cf9ded..d580aa6857a91 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -5,3 +5,5 @@ require_relative 'cop/migration/add_concurrent_foreign_key' require_relative 'cop/migration/add_concurrent_index' require_relative 'cop/migration/add_index' +require_relative 'cop/migration/remove_concurrent_index' +require_relative 'cop/migration/remove_index' diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index e007044868c1a..4ac79454647e7 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -58,6 +58,48 @@ end end + describe '#remove_concurrent_index' do + context 'outside a transaction' do + before do + allow(model).to receive(:transaction_open?).and_return(false) + end + + context 'using PostgreSQL' do + before do + allow(Gitlab::Database).to receive(:postgresql?).and_return(true) + allow(model).to receive(:disable_statement_timeout) + end + + it 'removes the index concurrently' do + expect(model).to receive(:remove_index). + with(:users, { algorithm: :concurrently, column: :foo }) + + model.remove_concurrent_index(:users, :foo) + end + end + + context 'using MySQL' do + it 'removes an index' do + expect(Gitlab::Database).to receive(:postgresql?).and_return(false) + + expect(model).to receive(:remove_index). + with(:users, { column: :foo }) + + model.remove_concurrent_index(:users, :foo) + end + end + end + + context 'inside a transaction' do + it 'raises RuntimeError' do + expect(model).to receive(:transaction_open?).and_return(true) + + expect { model.remove_concurrent_index(:users, :foo) }. + to raise_error(RuntimeError) + end + end + end + describe '#add_concurrent_foreign_key' do context 'inside a transaction' do it 'raises an error' do diff --git a/spec/rubocop/cop/migration/remove_concurrent_index_spec.rb b/spec/rubocop/cop/migration/remove_concurrent_index_spec.rb new file mode 100644 index 0000000000000..a714bf4e5d543 --- /dev/null +++ b/spec/rubocop/cop/migration/remove_concurrent_index_spec.rb @@ -0,0 +1,41 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/remove_concurrent_index' + +describe RuboCop::Cop::Migration::RemoveConcurrentIndex do + include CopHelper + + subject(:cop) { described_class.new } + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + it 'registers an offense when remove_concurrent_index is used inside a change method' do + inspect_source(cop, 'def change; remove_concurrent_index :table, :column; end') + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end + + it 'registers no offense when remove_concurrent_index is used inside an up method' do + inspect_source(cop, 'def up; remove_concurrent_index :table, :column; end') + + expect(cop.offenses.size).to eq(0) + end + end + + context 'outside of migration' do + it 'registers no offense' do + inspect_source(cop, 'def change; remove_concurrent_index :table, :column; end') + + expect(cop.offenses.size).to eq(0) + end + end +end diff --git a/spec/rubocop/cop/migration/remove_index_spec.rb b/spec/rubocop/cop/migration/remove_index_spec.rb new file mode 100644 index 0000000000000..31923cb7429c3 --- /dev/null +++ b/spec/rubocop/cop/migration/remove_index_spec.rb @@ -0,0 +1,35 @@ +require 'spec_helper' + +require 'rubocop' +require 'rubocop/rspec/support' + +require_relative '../../../../rubocop/cop/migration/remove_index' + +describe RuboCop::Cop::Migration::RemoveIndex do + include CopHelper + + subject(:cop) { described_class.new } + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + it 'registers an offense when remove_index is used' do + inspect_source(cop, 'def change; remove_index :table, :column; end') + + aggregate_failures do + expect(cop.offenses.size).to eq(1) + expect(cop.offenses.map(&:line)).to eq([1]) + end + end + end + + context 'outside of migration' do + it 'registers no offense' do + inspect_source(cop, 'def change; remove_index :table, :column; end') + + expect(cop.offenses.size).to eq(0) + end + end +end -- GitLab