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 | |
parent | 3677c80c9b40d5b412cbbe127510a7d7b975a8e7 (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'doc')
-rw-r--r-- | doc/development/README.md | 1 | ||||
-rw-r--r-- | doc/development/database_merge_request_checklist.md | 15 | ||||
-rw-r--r-- | doc/development/database_review.md | 60 | ||||
-rw-r--r-- | doc/user/search/index.md | 1 |
4 files changed, 50 insertions, 27 deletions
diff --git a/doc/development/README.md b/doc/development/README.md index 53133366c78..684f6d01d12 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -111,7 +111,6 @@ description: 'Learn how to contribute to GitLab.' ### Best practices -- [Merge Request checklist](database_merge_request_checklist.md) - [Adding database indexes](adding_database_indexes.md) - [Foreign keys & associations](foreign_keys.md) - [Single table inheritance](single_table_inheritance.md) diff --git a/doc/development/database_merge_request_checklist.md b/doc/development/database_merge_request_checklist.md deleted file mode 100644 index 09dece27e8d..00000000000 --- a/doc/development/database_merge_request_checklist.md +++ /dev/null @@ -1,15 +0,0 @@ -# Merge Request Checklist - -When creating a merge request that performs database related changes (schema -changes, adjusting queries to optimize performance, etc) you should use the -merge request template called "Database changes". This template contains a -checklist of steps to follow to make sure the changes are up to snuff. - -To use the checklist, create a new merge request and click on the "Choose a -template" dropdown, then click "Database changes". - -An example of this checklist can be found at -<https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/12463>. - -The source code of the checklist can be found in at -<https://gitlab.com/gitlab-org/gitlab/blob/master/.gitlab/merge_request_templates/Database%20changes.md> 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 diff --git a/doc/user/search/index.md b/doc/user/search/index.md index bc31052b758..68aef567270 100644 --- a/doc/user/search/index.md +++ b/doc/user/search/index.md @@ -62,6 +62,7 @@ You can filter issues and merge requests by specific terms included in titles or - Limitation - For performance reasons, terms shorter than 3 chars are ignored. E.g.: searching issues for `included in titles` is same as `included titles` + - Search is limited to 4096 characters and 64 terms per query. ![filter issues by specific terms](img/issue_search_by_term.png) |