diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-12-27 00:07:49 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-12-27 00:07:49 +0300 |
commit | b558264c9d10841f46a8ffeb15f18d0cee60fa7a (patch) | |
tree | 48733878d8c1351038ec21146dadef50a86b14b4 /doc/development/database_review.md | |
parent | 3677c80c9b40d5b412cbbe127510a7d7b975a8e7 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'doc/development/database_review.md')
-rw-r--r-- | doc/development/database_review.md | 60 |
1 files changed, 49 insertions, 11 deletions
diff --git a/doc/development/database_review.md b/doc/development/database_review.md index b1c3ed47976..5ca77579eec 100644 --- a/doc/development/database_review.md +++ b/doc/development/database_review.md @@ -32,12 +32,10 @@ for review. ### Roles and process -A Merge Request author's role is to: +A Merge Request **author**'s role is to: - Decide whether a database review is needed. - If database review is needed, add the ~database label. -- Use the [database changes](https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/merge_request_templates/Database%20changes.md) - merge request template, or include the appropriate items in the MR description. - [Prepare the merge request for a database review](#how-to-prepare-the-merge-request-for-a-database-review). A database **reviewer**'s role is to: @@ -78,15 +76,54 @@ make sure you have applied the ~database label and rerun the ### How to prepare the merge request for a database review -In order to make reviewing easier and therefore faster, please consider preparing a comment -and details for a database reviewer: +In order to make reviewing easier and therefore faster, please take +the following preparations into account. -- Provide queries in SQL form rather than ActiveRecord. -- Format any queries with a SQL query formatter, for example with [sqlformat.darold.net](http://sqlformat.darold.net). -- Consider providing query plans via a link to [explain.depesz.com](https://explain.depesz.com) or another tool instead of textual form. -- For query changes, it is best to provide the SQL query along with a plan *before* and *after* the change. This helps to spot differences quickly. -- When providing query plans, make sure to use good parameter values, so that the query executed is a good example and also hits enough data. - - Usually, the `gitlab-org` namespace (`namespace_id = 9970`) and the `gitlab-org/gitlab-foss` (`project_id = 13083`) or the `gitlab-org/gitlab` (`project_id = 278964`) projects provide enough data to serve as a good example. +#### Preparation when adding migrations + +- Ensure `db/schema.rb` is updated. +- Make migrations reversible by using the `change` method or include a `down` method when using `up`. + - Include either a rollback procedure or describe how to rollback changes. +- Add the output of the migration(s) to the MR description. +- Add tests for the migration in `spec/migrations` if necessary. See [Testing Rails migrations at GitLab](testing_guide/testing_migrations_guide.html) for more details. + +#### Preparation when adding or modifying queries + +- Write the raw SQL in the MR description. Preferably formatted + nicely with [sqlformat.darold.net](http://sqlformat.darold.net) or + [paste.depesz.com](https://paste.depesz.com). +- Include the output of `EXPLAIN (ANALYZE, BUFFERS)` of the relevant + queries in the description. If the output is too long, wrap it in + `<details>` blocks, paste it in a GitLab Snippet, or provide the + link to the plan at: [explain.depesz.com](https://explain.depesz.com). +- When providing query plans, make sure it hits enough data: + - You can use a GitLab production replica to test your queries on a large scale, + through the `#database-lab` Slack channel or through [chatops](understanding_explain_plans.md#chatops). + - Usually, the `gitlab-org` namespace (`namespace_id = 9970`) and the + `gitlab-org/gitlab-foss` (`project_id = 13083`) or the `gitlab-org/gitlab` (`project_id = 278964`) + projects provide enough data to serve as a good example. +- For query changes, it is best to provide the SQL query along with a + plan _before_ and _after_ the change. This helps to spot differences + quickly. +- Include data that shows the performance improvement, preferably in + the form of a benchmark. + +#### Preparation when adding foreign keys to existing tables + +- Include a migration to remove orphaned rows in the source table **before** adding the foreign key. +- Remove any instances of `dependent: ...` that may no longer be necessary. + +#### Preparation when adding tables + +- Order columns based on the [Ordering Table Columns](ordering_table_columns.md) guidelines. +- Add foreign keys to any columns pointing to data in other tables, including [an index](migration_style_guide.md#adding-foreign-key-constraints). +- Add indexes for fields that are used in statements such as `WHERE`, `ORDER BY`, `GROUP BY`, and `JOIN`s. + +#### Preparation when removing columns, tables, indexes or other structures + +- Follow the [guidelines on dropping columns](what_requires_downtime.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. ### How to review for database @@ -136,6 +173,7 @@ and details for a database reviewer: (eg indexes, columns), you can use a [one-off instance from the restore pipeline](https://ops.gitlab.net/gitlab-com/gl-infra/gitlab-restore/postgres-gprd) in order to establish a proper testing environment. + - Avoid N+1 problems and minimalize the [query count](merge_request_performance_guidelines.md#query-counts). ### Timing guidelines for migrations |