データベースレビューガイドライン

このページはデータベースレビューに特化したものです。コードレビュー全般に関するより広範なアドバイスやベストプラクティスについては、コードレビューガイドを参照してください。

一般的なプロセス

データベースのレビューは以下の場合に必要です：

  • データベーススキーマを変更する場合、またはデータマイグレーションを実行する場合：
    • db/
    • lib/gitlab/background_migration/
  • データベース・ツールの変更。たとえば
    • マイグレーションやActiveRecordヘルパーのlib/gitlab/database/
    • ロードバランシング
  • 自明を超える SQL クエリを生成する変更。複雑なクエリが導入されるかどうか、またデータベースのレビュアーが必要かどうかを判断するのは、一般的にマージリクエストの作成者次第です。
  • count 、distinct_count、estimate_batch_distinct_count 、sumを使用するサービス・データ・メトリクスの変更。これらのメトリクスには、大規模なテーブルに対する複雑なクエリが含まれる可能性があります。実装の詳細については、『Analytics Instrumentation Guide』を参照してください。
  • ActiveRecord オブジェクトでupdate,delete,update_all,delete_all,destroy_all メソッドを使用する変更。

データベースのレビュアーは、変更に含まれる複雑すぎるクエリに注意し、そのクエリに近いものをレビューすることが期待されています。作成者がレビューのために特定のクエリを指摘しておらず、過度に複雑なクエリがない場合は、マイグレーションのレビューだけに集中すれば十分です。

必須

データベース レビューをリクエストする際には、以下のアーティファクトを提供する必要があります。マージリクエストの説明にこれらの項目が含まれていない場合、レビューは作成者に差し戻されます。

マイグレーション

新しいマイグレーションが導入された場合、データベースのレビュアーはすべてのマイグレーションのマイグレーション(db:migrate)とロールバック(db:rollback)の両方の出力をレビューする必要があります。

GitLabの自動化ツール（db:check-migrations パイプラインジョブによって提供される）があり、CIジョブのログでこの出力を提供します。作成者がマージリクエストの説明でこの出力を提供する必要はありませんが、そうしておくとレビュアーにとって役に立つかもしれません。ボットはマイグレーションが正しく可逆であることもチェックします。

クエリ

新しいクエリが導入された場合、または既存のクエリが更新された場合は、クエリを提供する必要があります：

  • マージリクエストに含まれる各 raw SQL クエリのクエリプランと、各 raw SQL スニペットに続くクエリプランへのリンク。
  • 変更または追加されたすべてのクエリの生の SQL(ActiveRecord クエリから変換されたもの)。
    • 既存のクエリを更新する場合は、古いクエリと新しいクエリの両方の生のSQLをクエリプランとともに提供する必要があります。

この情報を提供する方法については、クエリの追加や修正時の準備を参照してください。

ロールとプロセス

マージリクエスト作成者のロールは以下のとおりです：

データベースのレビュアーは、以下の役割を果たします：

  • 必要なアーティファクトが適切な形式で提供されていることを確認します。必要な成果物が提供され、適切な形式で提供されていない場合、作成者にマージリクエストを再割り当てします。
  • MR のファーストパスレビューを行い、作成者に改善を提案します。
  • 満足したら、MRに~"database::reviewed "とラベルを付け直して承認し、レビュアー・ルーレットが提案するデータベースのメンテナーにレビューを依頼してください。これが完了したら、レビュアーとして削除してください。

データベースメンテナーのロールは以下の通りです：

  • MRの最終的なデータベースレビューを行うこと。
  • データベースのレビュアーおよび MR 作成者と、さらなる改善やその他の関連する変更について話し合います。
  • 最後に MR を承認し、~"database::approved" で MR のラベルを変更します。
  • 他の承認者が保留中でなければ MR をマージするか、必要に応じて他のメンテナーに渡します (フロントエンド、バックエンド、ドキュメント)。
    • マージしない場合は、レビュアーから外してください。

レビュアー作業負荷の分散

レビュー作業の負荷は、レビュアー・ルーレットを使用して分配されます（例）。MR作成者は、提案されたデータベースのレビュアーにレビューを依頼します。彼らがサインオフしたら、提案されたデータベースのメンテナーに引き継ぎます。

レビュアー・ルーレットでデータベースのレビュアーとメンテナーが提案されなかった場合は、~database のラベルを貼って、danger-review CI ジョブを再実行するか、@gl-database チームから誰かを選んでください。

データベースレビューのためのマージリクエストを準備する方法

レビュアーがより簡単に、より速くレビューできるように、以下の準備を考慮してください。

マイグレーションを追加するときの準備

  • db/structure.sql がドキュメント通りに更新されていることを確認し、さらにdb/schema_migrations 以下の関連するバージョンファイルが追加または削除されていることを確認してください。
  • データベース辞書が文書どおりに更新されていることを確認してください。
  • change メソッドを使用してマイグレーションを可逆にするか、up を使用する場合はdown メソッドを含めます。
    • ロールバック手順を含めるか、変更をロールバックする方法を記述してください。
  • db:check-migrations パイプラインジョブが正常に実行され、マイグレーションロールバックが期待どおりに動作することを確認します。
    • db:check-schema ジョブが正常に実行され、ロールバックで予期しないスキーマ変更が発生しないことを確認します。このジョブはスキーマが変更された場合にのみ警告を表示します。
    • レビュープロセス中にマイグレーションを変更するたびに、前述のジョブが成功し続けることを確認してください。
  • 必要であれば、spec/migrations でマイグレーションのテストを追加します。詳しくはTesting Rails migrations at GitLabをご覧ください。
  • マイグレーションにトラフィックの多いテーブルが含まれる場合は、enable_lock_retries メソッドを使用してロックリトライを有効にしてください。使用例と解決策については、ドキュメントの関連する例をレビューしてください。
  • 正当な理由がない限り、RuboCopチェックが無効にならないようにしてください。
  • 大きなテーブルにインデックスを追加する場合は、Database LabのCREATE INDEX CONCURRENTLY を使用して実行をテストし、実行時間を MR の説明に追加してください：
    • 実行時間はDatabase LabとGitLab.comで大きく異なりますが、Database Labの実行時間が長ければ、GitLab.comの実行時間もかなり長いというヒントになります。
    • Database Labからの実行時間が10 minutes より長い場合は、インデックスを マイグレーション後に移行する必要があります。この場合、マイグレーションとアプリケーションの変更を別々のリリースに分割して、インデックスを必要とするコードがデプロイされたときにインデックスが配置されているようにする必要があるかもしれないことを覚えておいてください。
  • test ステージで、手動でデータベーステストジョブ(db:gitlabcom-database-testing) を起動します。
    • このジョブはDatabase Lab のクローンでマイグレーションを実行し、その結果 (クエリ、実行時間、サイズの変化) を MR に投稿します。
    • マイグレーションの実行時間と警告をレビューします。

データマイグレーションを追加する際の準備

データマイグレーションには本質的にリスクが伴います。本番データの破損や損失につながるエラーの可能性を減らすために、追加のアクションが必要です。

MR の説明に含めてください：

  • マイグレーション自体が元に戻せない場合、インシデントが発生した場合にデータの変更をどのように元に戻せるかの詳細。例えば、レコードを削除するマイグレーション（ほとんどの場合、このオペレーションは自動的に元に戻せません）の場合、削除されたレコードをどのように復元_できるか_。
  • マイグレーションがデータを削除する場合、~data-deletion というラベルを適用します。
  • 例えば、「エピックからイシューが予期せず消えてしまう」などです。
  • クエリが期待通りに動作したことを示すクエリ計画からの関連データ (変更または削除されたレコードのおおよその数など)。

クエリを追加または修正する際の準備

生のSQL
  • MRの説明に生のSQLを記述してください。できればpgFormatterか https://paste.depesz.com 、通常の引用符(例えば"projects"."id") "projects"."id"を使用し"projects"."id"、スマート引用符( "projects"."id"例えば)は使用しないでください。

  • パラメータを使用して動的に生成されたクエリの場合、各バリエーションに対して1つの生のSQLクエリが必要です。

    例えば、プロジェクトに対するオプションのフィルタをパラメータとするイシューの検索では、イシューに対するクエリと、イシューとプロジェクトを結合してフィルタを適用するクエリの両方を含める必要があります。

    非常に多くの順列を生成できるファインダーやその他のメソッドがあります。生成される可能性のあるクエリをすべて網羅的に追加する必要はありません。すべてのパラメータを含むクエリと、生成されるクエリの種類ごとに1つずつ追加してください。

    例えば、joinやgroup by句がオプションの場合、group by句のないバージョンやjoinの少ないバージョンも含める必要があります。

  • クエリが常にリミットとオフセットと共に使用される場合、使用される最大許容リミットと0以外のオフセットが常に含まれるべきです。
クエリ計画
  • マージリクエストに含まれる各 raw SQL クエリのクエリプランと、各 raw SQL スニペットに続くクエリプランへのリンク。
  • explain postgres.aiチャットボットのコマンドを explain使用して生成された計画へのリンクを提供してください。explain コマンドは explain EXPLAIN ANALYZE で実行されます。
    • Database Lab で正確なイメージが得られない場合は、開発環境をシードする必要があるかもしれません。その代わりに、EXPLAIN ANALYZEからの出力を提供してください。explain.depesz.comまたはexplain.dalibo.comを使用してプランへのリンクを作成します。プランとクエリの両方を必ずフォームに貼り付けてください。
  • クエリプランを提供する際には、十分なデータがヒットするようにしてください：
    • 十分なデータのあるクエリプランを作成するには、IDを使用します：
      • gitlab-org ネームスペース (namespace_id = 9970) は、グループを含むクエリに使用します。
      • gitlab-org/gitlab-foss (project_id = 13083) またはgitlab-org/gitlab (project_id = 278964) プロジェクト。プロジェクトを含むクエリ用。
      • gitlab-qa ユーザー (user_id = 1614863) は、ユーザーを含むクエリ用です。
        • オプションで、クエリプランを生成するために使用するプロジェクトやグループ内で長い履歴を持つユーザのuser_id 、 user_idを使用することもできます。
    • つまり、どのクエリプランも、0レコードを返すか、指定された制限値よりも少ないレコードしか返してはならないということです（制限値が含まれている場合）。クエリがバッチ処理で使用される場合、適切な結果が含まれる適切なバッチ例が特定され、提供されなければなりません。
    • あなたのクエリがGitLab.comの新機能に属しており、そのため本番環境ではデータを返さない場合：
      • クエリを分析し、ローカル環境からプランを提供することができます。
      • postgres.aiでは、データの更新(exec UPDATE issues SET ...)や新しいテーブルや列の作成(exec ALTER TABLE issues ADD COLUMN ...)が可能です。
    • 実際に返されたレコードの数を調べる方法の詳細については、Understanding EXPLAINplansを参照してください。
  • クエリの変更については、変更_前と_変更_後の_計画とともにSQLクエリの両方を提供するのが最善です。これにより、違いを素早く見つけることができます。
  • 性能の向上を示すデータを、できればベンチマークの形で含めてください。
  • クエリプランを評価する場合、データベースに対して実行される最終クエリが必要です。ファインダやスコープからActiveRecord::Relation として返される内部クエリを分析する必要はありません。PostgreSQLの問い合わせ計画は、最終的な実行前に追加される制限やその他のものを含む、全ての最終パラメータに依存します。実際に実行されたクエリを確認する1つの方法は、log/development.log。

既存のテーブルに外部キーを追加する際の準備

  • 外部キーを追加する前に、ソーステーブルの孤児行を削除するマイグレーションを含めます。
  • 不要になったdependent: ... のインスタンスを削除してください。

テーブルを追加する際の準備

  • テーブルの列の順序のガイドラインに基づいて列を順序付けます。
  • インデックスを含む、他のテーブルのデータを指すカラムに外部キーを追加します。
  • WHERE 、ORDER BY、GROUP BY 、JOINなどのステートメントで使用されるフィールドにインデックスを追加します。
  • 新しいテーブルやカラムは必ずしもリスクが高いわけではありませんが、時間の経過とともに、本質的にスケールが難しくなるアクセスパターンもあります。このようなリスクのあるパターンを事前に特定するためには、アクセスやサイズに対する期待値を文書化する必要があります。MR の説明の中に、以下の質問に対する答えを含めてください：
    • 今後3ヶ月、6ヶ月、1年の間に、新しいテーブルの成長予測は？どのような仮定に基づいていますか？
    • 3ヶ月後、6ヶ月後、1年後、このテーブルの1時間あたりの読み取りと書き込みは何回になると予想されますか?どのような状況で行が更新されますか?どのような仮定に基づいていますか?
    • 予想されるデータ量とアクセスパターンに基づいて、新しいテーブルはGitLab.comや自己管理インスタンスに可用性リスクをもたらしますか？提案されているデザインは、GitLab.comとセルフマネージド・カスタマーのニーズをサポートするために拡張可能ですか？

カラム、テーブル、インデックス、その他の構造を削除する際の準備

  • カラムの削除に関するガイドラインに従ってください。
  • 一般的に、デプロイ後のマイグレーションではインデックスと外部キーを削除するのがベストプラクティスです（しかし、難しいルールではありません）。
    • 小さなテーブルのインデックスや外部キーの削除は例外です。
  • 複合インデックスを追加する場合、別のインデックスが冗長になる可能性があります。例えば、index(column_A, column_B, column_C) を追加すると、index(column_A, column_B) とindex(column_A) のインデックスが冗長になります。

update,delete,update_all,delete_all またはdestroy_all

これらの ActiveRecord メソッドを使用すると、データが変更されてパフォーマンスが低下したり、不適切にスコープされるとデータが破壊されたりする可能性があるため、特に注意が必要です。また、これらのメソッドはCommon Table Expression(CTE) ステートメントと互換性がありません。これらのメソッドが使用された場合、Danger はマージリクエスト差分についてコメントします。

生の SQL クエリおよびクエリ プランをマージ リクエスト記述に追加し、データベース レビューをリクエストするために、クエリを追加または変更する場合の準備に関するドキュメントに従ってください。

データベースのレビュー方法

  • マイグレーションの確認
    • リレーショナルモデリングと設計の選択のレビュー
    • マイグレーションをデータベースマイグレーションスタイルガイドに従ってレビューします。
    • マイグレーションがトランザクション内で実行されるか、（トランザクションを無効にして）同時実行のインデックス/外部キーヘルパーだけが含まれていることを確認します。
    • 大きなテーブルへのインデックスが追加され、その実行時間がDatabase Lab：
      • マイグレーション後に追加されたことを確認してください。
      • メンテナー：マージリクエストがマージされたら、#f_upcoming_release Slack チャンネルでリリースマネージャに通知してください。
    • db/structure.sql との整合性をチェックし、マイグレーションが可逆であることを確認してください。
    • db/schema_migrations 以下の関連するバージョンファイルが追加または削除されたことを確認します。
    • クエリのタイミングを確認します（もしあれば）：1回のトランザクションで、マイグレーションで実行されるクエリの累積時間がGitLab.comの15s 、できればそれ以下に収まる必要があります。
    • カラムの削除については、そのカラムが以前のリリースで無視されていたことを確認してください。
  • バッチバックグランドマイグレーションを確認してください：
    • GitLab.com での実行時間の見積もりを設定します。歴史的な目的のために、マージリクエストの説明にこの見積もりを含めることを強く推奨します。これは、予想されるバッチ数に遅延間隔を掛けたものになります。
    • test ステージで、手動でデータベーステストジョブ(db:gitlabcom-database-testing) を起動します。
    • 単一のupdate が1s よりも下にある場合、クエリは通常のマイグレーション (db/migrate 内部) に直接配置できます。
    • バックグラウンドマイグレーションは通常使用されますが、これに限定されません：
      • 大きなテーブルのデータのマイグレーション。
      • データセットのレコードごとに多数のSQLクエリを実行。
    • クエリのレビュー（バッチサイズに問題がないかなど）
    • 実行時間が通常のマイグレーションよりも長くなる可能性があるため、バックグラウンドマイグレーションをポストマイグレーションとして扱うことを推奨します:db/migrate ではなくdb/post_migrate に配置します。
  • マイグレーションのタイミングガイドラインの確認
  • マイグレーションが可逆的であり、#down メソッドを実装していることを確認。
  • 新しいテーブルのマイグレーションをチェックします：
    • 記載されているアクセスパターンやボリュームは妥当ですか？その前提条件は健全ですか？これらのパターンが安定性にリスクをもたらさないか？
    • 柱はスペースを節約するように配置されていますか？
    • 他のテーブルを参照するための外部キーはありますか？
  • データマイグレーションを確認してください：
    • GitLab.comでの実行時間の見積もりを確立します。
    • タイミングに応じて、データマイグレーションを通常マイグレーション、ポストデプロイマイグレーション、バックグラウンドマイグレーションに配置することができます。
    • データマイグレーションは、可能な限り、可逆的であるか、または可逆的な方法の説明が付属している必要があります。これは、すべてのタイプのマイグレーション（レギュラー、ポストデプロイ、バックグラウンド）に当てはまります。
  • クエリのパフォーマンス
    • 過度に複雑なクエリや、作成者が特にレビュアーとして指摘したクエリがないかをチェックします。
    • ない場合は、Database Labを使用してSQLクエリとクエリプランを提供するよう作成者に依頼します。
    • 与えられたクエリについて、データディストリビューションに関するパラメータのレビュー
    • クエリプランをチェックし、クエリの改善（クエリ、スキーマの変更、インデックスの追加など）を提案します。
    • 一般的なガイドラインは、クエリの実行時間が100ms以下になることです。
    • N+1問題を避け、クエリ数を最小限にします。