Database Review Guidelines

Database Review Guidelines

此页面特定于数据库评论. 请参阅我们的 ,以获取有关代码审查的更广泛建议和最佳实践.

A database review is required for:

  • 涉及数据库架构或执行数据迁移的更改,包括以下文件:
    • lib/gitlab/background_migration/
  • 对数据库工具的更改. 例如:
    • lib/gitlab/database/迁移或 ActiveRecord 助手
    • 负载均衡
  • 产生 SQL 查询的更改超出了显而易见的范围. 通常由合并请求的作者决定是否引入复杂查询以及是否需要数据库审查.

期望数据库审阅者在更改中查找明显复杂的查询,并仔细检查那些查询. 如果作者没有指出要审核的特定查询,并且没有明显复杂的查询,那么仅专注于审核迁移就足够了.

合并请求作者的角色是:

  • 确定是否需要数据库审查.
  • 如果需要数据库检查,请添加〜database 标签.
  • .

数据库审阅者的角色是:

  • 一旦满意,请用〜” database :: reviewed”重新标记 MR,批准,然后将 MR 重新分配给 Reviewer Roulette 建议的数据库维护者 .

数据库维护者的作用是:

  • 在 MR 上执行最终数据库审查.
  • 与数据库审阅者和 MR 作者讨论进一步的改进或其他相关更改.
  • 最后批准 MR,并使用〜” database :: approved”重新标记 MR
  • 如果没有其他待批准的批准,则合并 MR,或根据需要将其传递给其他维护者(前端,后端,文档).

如果审阅者轮盘赌没有建议数据库审阅者和维护者,请确保已应用〜database 标签并重新运行danger-review CI 作业,或从@gl-database团队中选择某人.

为了使审核更加轻松快捷,请考虑以下准备工作.

Preparation when adding migrations

  • 确保db/structure.sql已按照记录进行更新.
  • 通过使用change方法使迁移可逆,或者在使用up时包括down方法.
    • 包括回滚过程或描述如何回滚更改.
  • 将所有迁移的迁移和回滚的输出添加到 MR 描述中.
    • 确保 down 方法还原db/structure.sql的更改.
    • 在查看过程中,只要您修改迁移,就更新迁移输出.
  • 如有必要,在spec/migrations添加迁移测试. 有关更多详细信息,请参见 .
  • 当迁移中涉及高流量表时,请使用帮助程序方法. 请查看我们文档中的相关获取用例和解决方案.
  • Ensure RuboCop checks are not disabled unless there’s a valid reason to.
  • 将索引添加到大表时 ,请在#database-lab Slack 通道中使用CREATE INDEX CONCURRENTLY测试其执行,并将执行时间添加到 MR 描述中:
    • #database-lab和 GitLab.com 之间的执行时间差异很大,但是#database-lab执行时间增加,可能暗示 GitLab.com 上的执行量也很高.
    • 如果#database-lab的执行时间超过 ,则应将索引移至 . 请记住,在这种情况下,您可能需要将迁移和应用程序更改分为不同的版本,以确保在部署需要索引的代码时索引就位.

Preparation when adding or modifying queries

  • 在 MR 说明中编写原始 SQL. 最好用或paste.depesz.com很好地格式化.
  • 在描述中包括相关查询的EXPLAIN (ANALYZE, BUFFERS)输出. 如果输出太长,请将其包装在<details>块中,将其粘贴到 GitLab 代码片段中,或在以下位置提供指向计划的链接: .
  • 提供查询计划时,请确保其命中足够的数据:
    • 您可以通过#database-lab Slack 通道或chatops使用 GitLab 生产副本大规模测试查询.
    • 通常, gitlab-org命名空间( namespace_id = 9970 )和gitlab-org/gitlab-fossproject_id = 13083 )或gitlab-org/gitlabproject_id = 278964 )项目提供了足够的数据作为一个很好的例子.
  • 对于查询的变化,最好是与和变更的方案一起提供的 SQL 查询. 这有助于快速发现差异.
  • 包括显示性能改善的数据,最好以基准形式显示.

Preparation when adding foreign keys to existing tables

  • 添加外键之前,进行迁移以删除源表中的孤立行.
  • 删除所有dependent: ...实例dependent: ...可能不再需要.

Preparation when adding tables

  • 根据” 准则订购列 .
  • 将外键添加到指向其他表中数据的任何列,包括 .
  • 为在诸如WHEREORDER BYGROUP BYJOIN的语句中使用的字段添加索引.

Preparation when removing columns, tables, indexes, or other structures

  • 遵循的准则 .
  • 通常,最佳实践(但不是硬性规定)是在部署后迁移中删除索引和外键.
    • 例外包括删除小型表的索引和外键.
  • 如果要添加复合索引,则另一个索引可能会变得多余,因此请在同一迁移中将其删除. 例如,添加index(column_A, column_B, column_C)会使索引和index(column_A)冗余.
  • Check :
    • 建立在 GitLab.com 上执行的时间估计. 出于历史目的,强烈建议在合并请求描述中包括此估计.
    • 如果单个update低于1s ,则可以将查询直接放入常规迁移中(在db/migrate内部).
    • 通常使用后台迁移,但不限于:
      • 在较大的表中迁移数据.
      • 对数据集中的每条记录进行大量 SQL 查询.
    • 查看查询(例如,确保批次大小合适)
    • 因为执行时间可能比常规迁移要长,所以建议将后台迁移视为后期迁移:将它们放在db/post_migrate而不是db/migrate . 请记住,后迁移是在生产中的部署后执行的.
  • Check timing guidelines for migrations
  • 检查迁移是可逆的,并实现#down方法
  • 检查数据迁移:
    • 建立在 GitLab.com 上执行的时间估计.
    • 根据时间的不同,可以将数据迁移放在常规,部署后或后台迁移上.
    • 数据迁移也应该是可逆的,或者在可能的情况下附带有关如何逆向的描述. 这适用于所有类型的迁移(常规,部署后,后台).
  • 查询效果
    • 检查是否有任何明显复杂的查询,以及作者特别指出要进行审查的查询(如果有)
    • 如果尚不存在,请要求作者提供 SQL 查询和查询计划(例如,通过使用或直接数据库访问)
    • 对于给定的查询,请查看有关数据分配的参数
    • 检查查询计划并提出对查询的改进建议(更改查询,架构或添加索引等)
    • 一般准则是查询执行时间少于 100 毫秒
    • 如果查询依赖于尚未在生产环境中进行的先前迁移(例如,索引,列),则可以使用中的一次性实例来建立适当的测试环境. 如果您无权访问此项目,请访问 Slack 上的#database 以获取有关如何进行的建议.
    • 避免 N + 1 问题并最大程度地减少 .

通常,对于单个部署,GitLab.com 的迁移时间不应超过 1 个小时. 以下准则不是硬性规定,据估计,这些准则可将迁移时间降到最低.