Add latest changes from gitlab-org/gitlab@master

14 files changed, 25 insertions, 103 deletions

diff --git a/doc/development/application_limits.md b/doc/development/application_limits.md
index c42e9224105..3c1c91e0d2e 100644
--- a/doc/development/application_limits.md
+++ b/doc/development/application_limits.md
@@ -43,8 +43,6 @@ It's recommended to create two separate migration script files.
    class InsertProjectHooksPlanLimits < ActiveRecord::Migration[5.2]
      include Gitlab::Database::MigrationHelpers
 
-     DOWNTIME = false
-
      def up
        create_or_update_plan_limit('project_hooks', 'default', 0)
        create_or_update_plan_limit('project_hooks', 'free', 10)
diff --git a/doc/development/what_requires_downtime.md b/doc/development/avoiding_downtime_in_migrations.md
index 63a7ea4dfbd..d8981ce0999 100644
--- a/doc/development/what_requires_downtime.md
+++ b/doc/development/avoiding_downtime_in_migrations.md
@@ -4,12 +4,13 @@ group: Database
 info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments
 ---
 
-# What requires downtime?
+# Avoiding downtime in migrations
 
-When working with a database certain operations can be performed without taking
-GitLab offline, others do require a downtime period. This guide describes
-various operations, their impact, and how to perform them without requiring
-downtime.
+When working with a database certain operations may require downtime. Since we
+cannot have downtime in migrations we need to use a set of steps to get the
+same end result without downtime. This guide describes various operations that
+may appear to need downtime, their impact, and how to perform them without
+requiring downtime.
 
 ## Dropping Columns
 
diff --git a/doc/development/database/add_foreign_key_to_existing_column.md b/doc/development/database/add_foreign_key_to_existing_column.md
index 65693d2f675..f83dc35b4a6 100644
--- a/doc/development/database/add_foreign_key_to_existing_column.md
+++ b/doc/development/database/add_foreign_key_to_existing_column.md
@@ -69,8 +69,6 @@ Migration file for adding `NOT VALID` foreign key:
 class AddNotValidForeignKeyToEmailsUser < ActiveRecord::Migration[5.2]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   def up
     # safe to use: it requires short lock on the table since we don't validate the foreign key
     add_foreign_key :emails, :users, on_delete: :cascade, validate: false
@@ -97,8 +95,6 @@ Example for cleaning up records in the `emails` table within a database migratio
 class RemoveRecordsWithoutUserFromEmailsTable < ActiveRecord::Migration[5.2]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   disable_ddl_transaction!
 
   class Email < ActiveRecord::Base
@@ -133,8 +129,6 @@ Migration file for validating the foreign key:
 class ValidateForeignKeyOnEmailUsers < ActiveRecord::Migration[5.2]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   def up
     validate_foreign_key :emails, :user_id
   end
diff --git a/doc/development/database/database_reviewer_guidelines.md b/doc/development/database/database_reviewer_guidelines.md
index a242a3d5fd0..de131ddffbc 100644
--- a/doc/development/database/database_reviewer_guidelines.md
+++ b/doc/development/database/database_reviewer_guidelines.md
@@ -66,7 +66,7 @@ Finally, you can find various guides in the [Database guides](index.md) page tha
 topics and use cases. The most frequently required during database reviewing are the following:
 
 - [Migrations style guide](../migration_style_guide.md) for creating safe SQL migrations.
-- [What requires downtime?](../what_requires_downtime.md).
+- [Avoiding downtime in migrations](../avoiding_downtime_in_migrations.md).
 - [SQL guidelines](../sql.md) for working with SQL queries.
 
 ## How to apply for becoming a database maintainer
diff --git a/doc/development/database/index.md b/doc/development/database/index.md
index c749b796fab..01f6753e7a0 100644
--- a/doc/development/database/index.md
+++ b/doc/development/database/index.md
@@ -22,7 +22,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 
 ## Migrations
 
-- [What requires downtime?](../what_requires_downtime.md)
+- [Avoiding downtime in migrations](../avoiding_downtime_in_migrations.md)
 - [SQL guidelines](../sql.md) for working with SQL queries
 - [Migrations style guide](../migration_style_guide.md) for creating safe SQL migrations
 - [Testing Rails migrations](../testing_guide/testing_migrations_guide.md) guide
diff --git a/doc/development/database/not_null_constraints.md b/doc/development/database/not_null_constraints.md
index 743ad87ee87..9d1850f5504 100644
--- a/doc/development/database/not_null_constraints.md
+++ b/doc/development/database/not_null_constraints.md
@@ -26,8 +26,6 @@ For example, consider a migration that creates a table with two `NOT NULL` colum
 
 ```ruby
 class CreateDbGuides < ActiveRecord::Migration[6.0]
-  DOWNTIME = false
-
   def change
     create_table :db_guides do |t|
       t.bigint :stars, default: 0, null: false
@@ -47,8 +45,6 @@ For example, consider a migration that adds a new `NOT NULL` column `active` to
 
 ```ruby
 class AddExtendedTitleToSprints < ActiveRecord::Migration[6.0]
-  DOWNTIME = false
-
   def change
     add_column :db_guides, :active, :boolean, default: true, null: false
   end
@@ -117,7 +113,6 @@ with `validate: false` in a post-deployment migration,
 ```ruby
 class AddNotNullConstraintToEpicsDescription < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
-  DOWNTIME = false
 
   disable_ddl_transaction!
 
@@ -191,7 +186,6 @@ migration helper in a final post-deployment migration,
 ```ruby
 class ValidateNotNullConstraintOnEpicsDescription < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
-  DOWNTIME = false
 
   disable_ddl_transaction!
 
diff --git a/doc/development/database/strings_and_the_text_data_type.md b/doc/development/database/strings_and_the_text_data_type.md
index f338520c6ca..6fbb95c4f60 100644
--- a/doc/development/database/strings_and_the_text_data_type.md
+++ b/doc/development/database/strings_and_the_text_data_type.md
@@ -52,8 +52,6 @@ For example, consider a migration that creates a table with two text columns,
 class CreateDbGuides < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   def up
     create_table_with_constraints :db_guides do |t|
       t.bigint :stars, default: 0, null: false
@@ -89,7 +87,6 @@ For example, consider a migration that adds a new text column `extended_title` t
 
 ```ruby
 class AddExtendedTitleToSprints < ActiveRecord::Migration[6.0]
-  DOWNTIME = false
 
   # rubocop:disable Migration/AddLimitToTextColumns
   # limit is added in 20200501000002_add_text_limit_to_sprints_extended_title
@@ -106,8 +103,6 @@ A second migration should follow the first one with a limit added to `extended_t
 ```ruby
 class AddTextLimitToSprintsExtendedTitle < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
-  DOWNTIME = false
-
   disable_ddl_transaction!
 
   def up
@@ -185,8 +180,6 @@ in a post-deployment migration,
 class AddTextLimitMigration < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   disable_ddl_transaction!
 
   def up
@@ -266,7 +259,6 @@ helper in a final post-deployment migration,
 ```ruby
 class ValidateTextLimitMigration < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
-  DOWNTIME = false
 
   disable_ddl_transaction!
 
diff --git a/doc/development/database_review.md b/doc/development/database_review.md
index beca25a7e43..a25714ca6cf 100644
--- a/doc/development/database_review.md
+++ b/doc/development/database_review.md
@@ -176,7 +176,7 @@ test its execution using `CREATE INDEX CONCURRENTLY` in the `#database-lab` Slac
 
 #### Preparation when removing columns, tables, indexes, or other structures
 
-- Follow the [guidelines on dropping columns](what_requires_downtime.md#dropping-columns).
+- Follow the [guidelines on dropping columns](avoiding_downtime_in_migrations.md#dropping-columns).
 - Generally it's best practice (but not a hard rule) to remove indexes and foreign keys in a post-deployment migration.
   - Exceptions include removing indexes and foreign keys for small tables.
 - If you're adding a composite index, another index might become redundant, so remove that in the same migration.
@@ -199,7 +199,7 @@ test its execution using `CREATE INDEX CONCURRENTLY` in the `#database-lab` Slac
   - Check that the relevant version files under `db/schema_migrations` were added or removed.
   - Check queries timing (If any): In a single transaction, cumulative query time executed in a migration
     needs to fit comfortably within `15s` - preferably much less than that - on GitLab.com.
-  - For column removals, make sure the column has been [ignored in a previous release](what_requires_downtime.md#dropping-columns)
+  - For column removals, make sure the column has been [ignored in a previous release](avoiding_downtime_in_migrations.md#dropping-columns)
 - Check [background migrations](background_migrations.md):
   - Establish a time estimate for execution on GitLab.com. For historical purposes,
     it's highly recommended to include this estimation on the merge request description.
diff --git a/doc/development/geo/framework.md b/doc/development/geo/framework.md
index 9a8b94bdf64..071d6aa0456 100644
--- a/doc/development/geo/framework.md
+++ b/doc/development/geo/framework.md
@@ -266,8 +266,6 @@ For example, to add support for files referenced by a `Widget` model with a
    class CreateWidgetRegistry < ActiveRecord::Migration[6.0]
      include Gitlab::Database::MigrationHelpers
 
-     DOWNTIME = false
-
      disable_ddl_transaction!
 
      def up
@@ -399,8 +397,6 @@ can track verification state.
    # frozen_string_literal: true
 
    class AddVerificationStateToWidgets < ActiveRecord::Migration[6.0]
-     DOWNTIME = false
-
      def change
        change_table(:widgets) do |t|
          t.integer :verification_state, default: 0, limit: 2, null: false
@@ -427,8 +423,6 @@ can track verification state.
    class AddVerificationFailureLimitToWidgets < ActiveRecord::Migration[6.0]
      include Gitlab::Database::MigrationHelpers
 
-     DOWNTIME = false
-
      disable_ddl_transaction!
 
      CONSTRAINT_NAME = 'widget_verification_failure_text_limit'
@@ -453,8 +447,6 @@ can track verification state.
    class AddVerificationIndexesToWidgets < ActiveRecord::Migration[6.0]
      include Gitlab::Database::MigrationHelpers
 
-     DOWNTIME = false
-     VERIFICATION_STATE_INDEX_NAME = "index_widgets_verification_state"
      PENDING_VERIFICATION_INDEX_NAME = "index_widgets_pending_verification"
      FAILED_VERIFICATION_INDEX_NAME = "index_widgets_failed_verification"
      NEEDS_VERIFICATION_INDEX_NAME = "index_widgets_needs_verification"
@@ -497,8 +489,6 @@ can track verification state.
    class CreateWidgetStates < ActiveRecord::Migration[6.0]
      include Gitlab::Database::MigrationHelpers
 
-     DOWNTIME = false
-
      disable_ddl_transaction!
 
      def up
@@ -926,8 +916,6 @@ For example, to add support for files referenced by a `Gizmos` model with a
    class CreateGizmoRegistry < ActiveRecord::Migration[6.0]
      include Gitlab::Database::MigrationHelpers
 
-     DOWNTIME = false
-
      disable_ddl_transaction!
 
      def up
diff --git a/doc/development/merge_request_performance_guidelines.md b/doc/development/merge_request_performance_guidelines.md
index 497a8a7aabc..772ce1d6b59 100644
--- a/doc/development/merge_request_performance_guidelines.md
+++ b/doc/development/merge_request_performance_guidelines.md
@@ -18,7 +18,7 @@ To measure the impact of a merge request you can use
 the following guides:
 
 - [Performance Guidelines](performance.md)
-- [What requires downtime?](what_requires_downtime.md)
+- [Avoiding downtime in migrations](avoiding_downtime_in_migrations.md)
 
 ## Definition
 
diff --git a/doc/development/migration_style_guide.md b/doc/development/migration_style_guide.md
index 8c1420a3630..d4ea5f927d6 100644
--- a/doc/development/migration_style_guide.md
+++ b/doc/development/migration_style_guide.md
@@ -16,16 +16,12 @@ migrations are written carefully, can be applied online, and adhere to the style
 guide below.
 
 Migrations are **not** allowed to require GitLab installations to be taken
-offline unless _absolutely necessary_.
-
-When downtime is necessary the migration has to be approved by:
-
-1. The VP of Engineering
-1. A Backend Maintainer
-1. A Database Maintainer
-
-An up-to-date list of people holding these titles can be found at
-<https://about.gitlab.com/company/team/>.
+offline ever. Migrations always must be written in such a way to avoid
+downtime. In the past we had a process for defining migrations that allowed for
+downtime by setting a `DOWNTIME` constant. You may see this when looking at
+older migrations. This process was in place for 4 years without every being
+used and as such we've learnt we can always figure out how to write a migration
+differently to avoid downtime.
 
 When writing your migrations, also consider that databases might have stale data
 or inconsistencies and guard for that. Try to make as few assumptions as
@@ -65,47 +61,16 @@ scripts/regenerate-schema
 TARGET=12-9-stable-ee scripts/regenerate-schema
 ```
 
-## What Requires Downtime?
-
-The document ["What Requires Downtime?"](what_requires_downtime.md) specifies
-various database operations, such as
-
-- [dropping and renaming columns](what_requires_downtime.md#dropping-columns)
-- [changing column constraints and types](what_requires_downtime.md#changing-column-constraints)
-- [adding and dropping indexes, tables, and foreign keys](what_requires_downtime.md#adding-indexes)
-
-and whether they require downtime and how to work around that whenever possible.
-
-## Downtime Tagging
-
-Every migration must specify if it requires downtime or not, and if it should
-require downtime it must also specify a reason for this. This is required even
-if 99% of the migrations don't require downtime as this makes it easier to find
-the migrations that _do_ require downtime.
-
-To tag a migration, add the following two constants to the migration class'
-body:
-
-- `DOWNTIME`: a boolean that when set to `true` indicates the migration requires
-  downtime.
-- `DOWNTIME_REASON`: a String containing the reason for the migration requiring
-  downtime. This constant **must** be set when `DOWNTIME` is set to `true`.
+## Avoiding downtime
 
-For example:
+The document ["Avoiding downtime in migrations"](avoiding_downtime_in_migrations.md) specifies
+various database operations, such as:
 
-```ruby
-class MyMigration < ActiveRecord::Migration[6.0]
-  DOWNTIME = true
-  DOWNTIME_REASON = 'This migration requires downtime because ...'
-
-  def change
-    ...
-  end
-end
-```
+- [dropping and renaming columns](avoiding_downtime_in_migrations.md#dropping-columns)
+- [changing column constraints and types](avoiding_downtime_in_migrations.md#changing-column-constraints)
+- [adding and dropping indexes, tables, and foreign keys](avoiding_downtime_in_migrations.md#adding-indexes)
 
-It is an error (that is, CI fails) if the `DOWNTIME` constant is missing
-from a migration class.
+and explains how to perform them without requiring downtime.
 
 ## Reversibility
 
@@ -512,8 +477,6 @@ class like so:
 class MyMigration < ActiveRecord::Migration[6.0]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   disable_ddl_transaction!
 
   INDEX_NAME = 'index_name'
@@ -640,8 +603,6 @@ Take the following migration as an example:
 
 ```ruby
 class DefaultRequestAccessGroups < ActiveRecord::Migration[5.2]
-  DOWNTIME = false
-
   def change
     change_column_default(:namespaces, :request_access_enabled, from: false, to: true)
   end
@@ -849,8 +810,6 @@ Example migration adding this column:
 
 ```ruby
 class AddOptionsToBuildMetadata < ActiveRecord::Migration[5.0]
-  DOWNTIME = false
-
   def change
     add_column :ci_builds_metadata, :config_options, :jsonb
   end
diff --git a/doc/development/prometheus_metrics.md b/doc/development/prometheus_metrics.md
index 51c5c4954e4..09efb70f279 100644
--- a/doc/development/prometheus_metrics.md
+++ b/doc/development/prometheus_metrics.md
@@ -39,8 +39,6 @@ Or, you can create a database migration:
 class ImportCommonMetrics < ActiveRecord::Migration[4.2]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   def up
     ::Gitlab::DatabaseImporters::CommonMetrics::Importer.new.execute
   end
diff --git a/doc/development/scalability.md b/doc/development/scalability.md
index a9b8fb4389f..8ee6e57e4d1 100644
--- a/doc/development/scalability.md
+++ b/doc/development/scalability.md
@@ -36,7 +36,7 @@ application starts, Rails queries the database schema, caching the tables and
 column types for the data requested. Because of this schema cache, dropping a
 column or table while the application is running can produce 500 errors to the
 user. This is why we have a [process for dropping columns and other
-no-downtime changes](what_requires_downtime.md).
+no-downtime changes](avoiding_downtime_in_migrations.md).
 
 #### Multi-tenancy
 
diff --git a/doc/development/sidekiq_style_guide.md b/doc/development/sidekiq_style_guide.md
index 151194e87c1..4bcd5e50fae 100644
--- a/doc/development/sidekiq_style_guide.md
+++ b/doc/development/sidekiq_style_guide.md
@@ -831,8 +831,6 @@ as shown in this example:
 class MigrateTheRenamedSidekiqQueue < ActiveRecord::Migration[5.0]
   include Gitlab::Database::MigrationHelpers
 
-  DOWNTIME = false
-
   def up
     sidekiq_queue_migrate 'old_queue_name', to: 'new_queue_name'
   end