データベースレビューガイドライン
このページはデータベースレビューに特化したもので、コードレビュー全般に関するより広範なアドバイスやベストプラクティスについては、コードレビューガイドを参照してください。
一般プロセス
データベースのレビュアーが必要です:
- データベーススキーマに触れたり、データ移行を実行する変更:
db/
lib/gitlab/background_migration/
- 例えば、データベースツールの変更:
- マイグレーションやActiveRecordヘルパーを
lib/gitlab/database/
- 負荷分散
- マイグレーションやActiveRecordヘルパーを
- 自明を超える SQL クエリを生成する変更。 通常、複雑なクエリが導入されるかどうか、またデータベースのレビュアーが必要かどうかを判断するのは、マージリクエストの作成者次第です。
データベースのレビュアーは、変更の中に明らかに複雑なクエリがないかを探し、そのクエリに近いものをレビューすることが期待されています。 作成者がレビューのために特定のクエリを指摘しておらず、明らかに複雑なクエリがない場合は、移行のレビューだけに集中すれば十分です。
SQL形式のクエリをレビュアーすることが望ましく、ActiveRecordのクエリをSQL形式に翻訳してレビュアーするよう作成者に依頼することが一般的です。
ロールとプロセス
マージリクエスト作成者のロールです:
- データベースのレビュアーが必要かどうかを決定します。
- データベースのレビュアーが必要な場合は、~databaseラベルを追加してください。
- データベースのレビュアー用にマージリクエストを準備します。
データベースのレビュアーは、以下のようなロールがあります:
- MRのファーストパスレビューを行い、作成者に改善点を提案します。
- 満足したら、MRのラベルを~”database::reviewed “に変更し、承認者にして、レビュアー・ルーレットが提案したデータベースのメンテナーにMRを再割り当てします。
データベースメンテナーのロールは以下の通りです:
- MRの最終データベースレビューを実行します。
- データベースのレビュアーやMR作成者と、さらなる改善やその他の関連する変更について話し合います。
- 最後にMRを承認し、MRのラベルを~”database::approved “に変更します。
- 他の承認者が保留中でなければ MR をマージするか、必要に応じて他のメンテナーに渡します (フロントエンド、バックエンド、ドキュメント)。
ディストリビューション業務の負担軽減
ディストリビューションは、レビュアー・ルーレット(例)を用いて行います。 MR作成者は、提案されたデータベースのレビュアーを共同でアサインします。 彼らがサインをしたら、提案されたデータベースのメンテナーに引き継ぎます。
レビュアー・ルーレットでデータベースのレビュアーとメンテナーが提案されなかった場合は、~databaseラベルが適用されていることを確認し、danger-review
CIジョブを再実行するか、@gl-database
チームから誰かを選んでください。
データベース・レビューのためのマージリクエストの作成方法
レビュアーがより簡単に、より早くレビュアーになれるよう、以下の点にご留意ください。
マイグレーション追加時の準備
-
db/structure.sql
が文書通りに更新されていることを確認します。 -
change
メソッドを使用して移行を可逆的にするか、up
を使用する場合はdown
メソッドを含めます。- ロールバック手順を含めるか、変更をロールバックする方法を記述してください。
- すべてのマイグレーションについて、マイグレーションとロールバックの両方の出力を MR の説明に追加します。
- ダウン・メソッドが
db/structure.sql
の変更を元に戻すことを確認してください。 - レビュアーがマイグレーションを修正するたびに、マイグレーション出力を更新します。
- ダウン・メソッドが
- 必要であれば、
spec/migrations
でマイグレーションのテストを追加しましょう。 詳細は GitLabの Rails マイグレーションのテストを参照してください。 -
トラフィックの多いテーブルを移行する場合は、
with_lock_retries
ヘルパーメソッドを使用します。 使用例やソリューションについては、ドキュメントの関連する例を参照してください。 - 正当な理由がない限り、RuboCopチェックが無効にならないようにしてください。
-
大規模なテーブルにインデックスを追加する場合は、
#database-lab
Slack チャンネルでCREATE INDEX CONCURRENTLY
を使用して実行をテストし、実行時間を MR の説明に追加します:- 実行時間は
#database-lab
、GitLab.comによって大きく異なりますが、#database-lab
、GitLab.comでの実行時間が長くなれば、GitLab.comでの実行時間もかなり長くなることを示唆します。 -
#database-lab
からの実行が1h
より長い場合、インデックスは移行後に移動されるべきです。 この場合、インデックスを必要とするコードがデプロイされるときにインデックスが確実に配置されるように、移行とアプリケーションの変更を別々のリリースに分割する必要があるかもしれないことに留意してください。
- 実行時間は
クエリを追加または修正する際の準備
- できれば、pgFormatterかpaste.depesz.comできれいに整形してください。
- 関連するクエリの
EXPLAIN (ANALYZE, BUFFERS)
の出力を説明文に含めてください。出力が長すぎる場合は、<details>
ブロックで囲むか、GitLab スニペットに貼り付けるか、計画へのリンクを:explain.depesz.comで提供してください。 - クエリプランを提供する際には、十分なデータをヒットするようにしてください:
- GitLabのプロダクションレプリカを使って、
#database-lab
Slackチャンネルやchatopsを通じて大規模にクエリをテストすることができます。 - 通常、
gitlab-org
ネームスペース(namespace_id = 9970
)、gitlab-org/gitlab-foss
(project_id = 13083
)、gitlab-org/gitlab
(project_id = 278964
)プロジェクトは、良い例として十分なデータを提供しています。
- GitLabのプロダクションレプリカを使って、
- クエリの変更については、SQLクエリを変更_前と_ _変更後の_プランとともに提供するのが最善です。 これは、相違を素早く発見するのに役立ちます。
- パフォーマンスの向上を示すデータを、できればベンチマークの形で添付してください。
既存のテーブルに外部キーを追加する際の準備
- 外部キーを追加する前に、ソース・テーブルの孤児行を削除するマイグレーションを含めます。
- 不要になった
dependent: ...
のインスタンスを削除します。
テーブル追加時の準備
- テーブルの列の順序に関するガイドラインに基づいて列を順序付けます。
- インデックスを含め、他のテーブルのデータを指すカラムに外部キーを追加します。
-
WHERE
、ORDER BY
、GROUP BY
、JOIN
などのステートメントで使用されるフィールドのインデックスを追加します。
カラム、テーブル、インデックス、その他の構造を削除する際の準備
- 列の削除に関するガイドラインに従ってください。
- 一般的に、デプロイ後のマイグレーションではインデックスと外部キーを削除するのがベストプラクティスです(しかし、難しいルールではありません)。
- 小さなテーブルのインデックスや外部キーの削除は例外です。
- 例えば、
index(column_A, column_B, column_C)
を追加すると、index(column_A, column_B)
とindex(column_A)
のインデックスが冗長になります。
データベースのレビュアー方法
- マイグレーションの確認
- リレーショナル・モデリングと設計の選択の見直し
- レビュアーはデータベース移行スタイルガイドに従って移行を行います。
- マイグレーションがトランザクション内で実行されるか、(トランザクションを無効にして)同時インデックス/外部キーヘルパーだけを含むようにします。
- ラージテーブルへのインデックスが追加され、その実行時間が
#database-lab
で増加(1時間以上)した場合:- マイグレーション後に追加されたことを確認してください。
- メンテナー: マージリクエストがマージされたら、そのことを
#f_upcoming_release
Slack チャンネルでリリースマネージャに通知してください。
-
db/structure.sql
との整合性をチェックし、移行が可逆的であることを確認します。 - クエリのタイミングを確認する(もしあれば):マイグレーションで実行されるクエリは、GitLab.comの
15s
、できればそれ以下に快適に収まる必要があります。 - カラムの削除については、以前のリリースでカラムが無視されていることを確認してください。
-
バックグラウンドの移行を確認します:
- GitLab.com での実行時間の見積もりを設定します。 歴史的な目的のために、マージリクエストの説明にこの見積もりを含めることを強く推奨します。
- 一つの
update
が1s
よりも下にある場合、クエリは通常のマイグレーション(db/migrate
内部)に直接置くことができます。 - バックグランド・マイグレーションは通常使用されますが、これに限定されるものではありません:
- 大きなテーブルのデータの移行
- データセットのレコードごとに多数のSQLクエリを実行します。
- クエリーのレビュアー(バッチサイズに問題がないかなど)
- 実行時間は通常のマイグレーションよりも長くなる可能性があるため、バックグラウンドマイグレーションをポストマイグレーションとして扱うことを推奨します:
db/migrate
の代わりにdb/post_migrate
に配置します。 ポストマイグレーションは本番環境へのデプロイ後に実行されることに留意してください。
- 移行のタイミングガイドラインの確認
- 移行が可逆的であることを確認し、
#down
メソッドを実装します。 - データ移行を確認します:
- GitLab.comでの実行時間の見積もりを確立します。
- タイミングに応じて、データマイグレーションを定期的、デプロイ後、またはバックグラウンドマイグレーションに配置することができます。
- データマイグレーションは、可能な限り、可逆的であるか、または可逆的な方法の説明が付属している必要があります。 これは、すべてのタイプのマイグレーション(通常、ポストデプロイ、バックグラウンド)に適用されます。
- クエリーパフォーマンス
- 明らかに複雑なクエリや、作成者が特にレビュアーに指摘したクエリがないかをチェックします(もしあれば)。
- まだ存在しない場合は、作成者にSQLクエリとクエリプランを提供するよう依頼します(例えば、chatopsやデータベースへの直接アクセスを使用するなど)。
- 指定されたクエリについて、データのディストリビューションに関するパラメータをレビュアー。
- クエリプランをチェックし、クエリの改善(クエリ、スキーマの変更、インデックスの追加など)を提案します。
- 一般的なガイドラインは、クエリの実行時間が100ms以下になることです。
- クエリが本番環境にはまだ存在しない以前のマイグレーションに依存している場合 (インデックスやカラムなど)、適切なテスト環境を確立するためにリストアパイプラインから単発のインスタンスを使用することができます。 このプロジェクトにアクセスできない場合は、Slack の #database に連絡して進め方のアドバイスをもらってください。
- N+1問題を回避し、クエリー数を最小限に抑えます。
移行のタイミング
一般的に、GitLab.comでは1回のデプロイのためのマイグレーションに1時間以上かかることはありません。以下のガイドラインは厳密なルールではなく、マイグレーションの時間を最小限に抑えるために見積もられたものです。
マイグレーションタイプ | 推奨実行時間 | 備考 |
---|---|---|
定期的なマイグレーションdb/migrate
| 3 minutes
| インデックスの作成は時間がかかるので例外です。 |
db/post_migrate にマイグレーションを投稿します。
| 10 minutes
| |
バックグラウンド移行 | — | これらは大規模なテーブルに適しているため、正確なタイミングガイドラインを設定することはできませんが、コールドキャッシュを使用した場合、どのクエリも実行時間を1 second 以下に抑える必要があります。
|