Help us learn about your current experience with the documentation. Take the survey.

数据库审查指南

此页面专门针对数据库审查。有关代码审查的通用建议和最佳实践，请参阅我们的 代码审查指南。

一般流程

以下情况需要进行数据库审查：

  • 修改数据库架构或执行数据迁移的更改，包括以下文件中的内容：
    • db/
    • lib/gitlab/background_migration/
  • 修改数据库工具的更改。例如：
    • lib/gitlab/database/ 中的迁移或 ActiveRecord 辅助方法
    • 负载均衡
  • 产生明显超出常规的 SQL 查询的更改。通常由合并请求的作者决定是否引入了复杂查询以及是否需要数据库审查。
  • 使用 count、distinct_count、estimate_batch_distinct_count 和 sum 的 Service Data 指标更改。 这些指标可能涉及对大型表的复杂查询。 有关实现细节，请参阅 分析工具指南。
  • 在 ActiveRecord 对象上使用 update、upsert、delete、update_all、upsert_all、delete_all 或 destroy_all 方法的更改。

数据库审查员应关注更改中的过于复杂的查询，并进行更详细的审查。如果作者没有指出需要审查的特定查询，且没有过于复杂的查询，则只需专注于审查迁移即可。

必需项

当你请求 ~database 审查时，必须提供以下工件。如果合并请求描述中未包含这些项目，审查将被重新分配给作者。

迁移

如果引入了新的迁移，数据库审查员必须审查所有迁移的迁移（db:migrate）和回滚（db:rollback）输出。

我们有针对 GitLab 的自动化工具（由 db:check-migrations 管道作业提供），它在 CI 作业日志中提供此输出。 作者无需在合并请求描述中提供此输出，但这样做可能对审查者有帮助。机器人还会检查迁移是否正确可逆。

查询

如果引入了新查询或更新了现有查询，你必须提供：

  • 每个原始 SQL 查询的 查询计划，以及每个原始 SQL 代码片段后的查询计划链接。
  • 所有更改或添加的查询的 原始 SQL（从 ActiveRecord 查询转换而来）。
    • 如果更新了现有查询，应同时提供查询的旧版本和新版本的原始 SQL 以及它们的查询计划。

有关如何提供此信息，请参阅 添加或修改查询时的准备。

角色和流程

合并请求 作者 的角色是：

数据库 审查员 的角色是：

  • 确保 必需的 工件已提供且格式正确。如果不符合，将合并请求重新分配给作者。
  • 对 MR 进行初步审查并向作者提出改进建议。
  • 满意后，将 MR 重新标记为 ~“database::reviewed”，批准它，并 请求由 Reviewer Roulette 建议的数据库 管理员 进行审查。

数据库 管理员 的角色是：

  • 对 MR 进行最终数据库审查。
  • 与数据库审查员和 MR 作者讨论进一步的改进或其他相关更改。
  • 最终批准 MR 并将 MR 重新标记为 ~“database::approved”
  • 如果没有其他待批准的请求，则合并 MR，或根据需要将其移交给其他管理员（前端、后端、文档）。

分配审查工作量

审查工作量使用 reviewer roulette （示例）进行分配。 MR 作者应请求建议的数据库 审查员 进行审查。当他们签核后，他们会移交给 建议的数据库 管理员。

如果 reviewer roulette 没有建议数据库审查员和管理员，请确保你已应用 ~database 标签并重新运行 danger-review CI 作业，或从 @gl-database 团队 中选择某人。

如何为数据库审查准备合并请求

为了使审查更容易且更快，请考虑以下准备工作。

添加迁移时的准备

  • 确保 db/structure.sql 已按照 文档 更新，并确保 db/schema_migrations 下相关的版本文件已添加或删除。
  • 确保数据库字典已按照 文档 更新。
  • 通过使用 change 方法或在使用 up 时包含 down 方法使迁移可逆。
    • 包含回滚程序或描述如何回滚更改。
  • 检查 db:check-migrations 管道作业是否已成功运行，并且迁移回滚行为符合预期。
    • 确保 db:check-schema 作业已成功运行，并且在回滚中没有引入意外的架构更改。如果架构已更改，此作业可能只会触发警告。
    • 验证在审查过程中修改迁移时，上述作业继续成功运行。
  • 如有必要，在 spec/migrations 中为迁移添加测试。有关更多详细信息，请参阅 在 GitLab 中测试 Rails 迁移。
  • 锁定重试 默认为所有事务性迁移启用。对于非事务性迁移，请查看相关的 文档 了解用例和解决方案。
  • 确保不会禁用 RuboCop 检查，除非有充分的理由。
  • 当向 大型表 添加索引时， 在 Database Lab 中使用 CREATE INDEX CONCURRENTLY 测试其执行时间，并将执行时间添加到 MR 描述中：
    • Database Lab 和 GitLab.com 之间的执行时间差异很大，但 Database Lab 中较长的执行时间 可以提示 GitLab.com 上的执行时间也相当高。
    • 如果 Database Lab 中的执行时间超过 10 分钟，索引 应移至 后迁移。 请注意，在这种情况下，你可能需要将迁移和应用程序更改拆分为单独的发布，以确保索引 在需要它的代码部署时已就位。
  • 在 test 阶段手动触发 数据库测试 作业（db:gitlabcom-database-testing）。
    • 此作业在 Database Lab 克隆中运行迁移，并将其发现（查询、运行时间、大小变化）发布到 MR。
    • 审查迁移运行时间和任何警告。

添加数据迁移时的准备

数据迁移本质上是有风险的。需要额外的操作来减少导致生产数据损坏或丢失的错误可能性。

在 MR 描述中包含：

  • 如果迁移本身不可逆，说明在发生事件时如何回滚数据更改。例如，对于删除记录的迁移（大多数情况下不是自动可逆的操作），如何恢复已删除的记录。
  • 如果迁移删除数据，应用 ~data-deletion 标签。
  • 对错误可能造成的用户体验影响的简洁描述；例如，“Issue 会意外地从 Epic 中消失”。
  • 表明查询按预期工作的相关 查询计划 数据；例如，修改或删除的记录的近似数量。

添加或修改查询时的准备

原始 SQL

  • 在 MR 描述中编写原始 SQL。最好使用 pgFormatter 或 https://paste.depesz.com 进行格式化，并使用常规引号 （例如，"projects"."id"）并避免智能引号（例如，"projects"."id"）。

  • 对于使用参数动态生成的查询，每个变体应有一个原始 SQL 查询。

    例如，一个可能采用可选项目参数的 issue 查找器， 应同时包含 issue 上的查询版本以及连接 issue 和项目并应用过滤器的版本。

    有些查找器或其他方法可以生成大量排列组合。 无需详尽添加所有可能的生成查询，只需包含包含所有参数的查询以及每种类型的生成查询各一个。

    例如，如果连接或 group by 子句是可选的，也应包含没有 group by 子句和较少连接的版本， 同时保持对剩余表的适当过滤器。

  • 如果查询始终与限制和偏移量一起使用，则应始终包含最大允许的限制和非零偏移量。

查询计划
  • 每个包含在合并请求中的原始 SQL 查询的查询计划，以及每个原始 SQL 代码片段后的查询计划链接。
  • 提供使用 postgres.ai 聊天机器人中的 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）项目，用于涉及项目的查询。
        • 对于涉及项目成员资格的查询，可能需要这些项目的 project_namespace_id 来创建查询计划。这些是 15846663（对于 gitlab-org/gitlab）和 15846626（对于 gitlab-org/gitlab-foss）
      • gitlab-qa 用户（user_id = 1614863），用于涉及用户的查询。
        • 或者，你也可以使用你自己的 user_id，或在用于生成查询计划的项目或组中具有长期历史的用户的 user_id。

    • 这意味着没有查询计划应返回 0 条记录或比提供的限制更少的记录（如果包含限制）。如果查询用于批处理，应识别并提供包含适当结果的适当示例批处理。

      UPDATE 语句始终返回 0 条记录。要识别它更新的行，我们需要检查下面的以下行。

      例如，UPDATE 语句返回 0 条记录，但我们可以看到它从以 -> Index scan 开头的行更新了 1 行：

      EXPLAIN UPDATE p_ci_pipelines SET updated_at = current_timestamp WHERE id = 1606117348;

 ModifyTable on public.p_ci_pipelines  (cost=0.58..3.60 rows=0 width=0) (actual time=5.977..5.978 rows=0 loops=1)
  Buffers: shared hit=339 read=4 dirtied=4
  WAL: records=20 fpi=4 bytes=21800
  I/O Timings: read=4.920 write=0.000
  ->  Index Scan using ci_pipelines_pkey on public.ci_pipelines p_ci_pipelines_1  (cost=0.58..3.60 rows=1 width=18) (actual time=0.041..0.044 rows=1 loops=1)
        Index Cond: (p_ci_pipelines_1.id = 1606117348)
        Buffers: shared hit=8
        I/O Timings: read=0.000 write=0.000

    • 如果你的查询属于 GitLab.com 中的新功能，因此在生产环境中不返回数据：

      • 你可以分析查询并提供来自本地环境的计划。
      • postgres.ai 允许更新数据（exec UPDATE issues SET ...）和创建新表和列（exec ALTER TABLE issues ADD COLUMN ...）。

    • 有关如何查找实际返回记录数的更多信息，请参阅 理解 EXPLAIN 计划

  • 对于查询更改，最好同时提供更改前后的 SQL 查询和 计划。这有助于快速发现差异。
  • 包含显示性能改进的数据，最好是 以基准的形式。
  • 评估查询计划时，我们需要最终查询 在数据库上执行。我们不需要分析作为 ActiveRecord::Relation 从查找器和作用域返回的中间 查询。 PostgreSQL 查询计划依赖于所有最终参数， 包括限制和其他可能在最终执行前添加的内容。 确保实际执行的查询的一种方法是检查 log/development.log。

向现有表添加外键时的准备

  • 在添加外键之前，包含一个迁移以删除源表中的孤立行。
  • 删除任何可能不再必要的 dependent: ... 实例。

添加表时的准备

  • 根据 排序表列 指南对列进行排序。
  • 向指向其他表中数据的任何列添加外键，包括 索引。
  • 为在 WHERE、ORDER BY、GROUP BY 和 JOIN 等语句中使用的字段添加索引。
  • 新表必须由 db/fixtures/development/ 中的文件填充。这些 fixture 还用于 确保 升级成功完成， 因此始终填充新表非常重要。
  • 确保你不使用数据库表来存储 静态数据。
  • 新表和新列不一定有风险，但随着时间的推移，某些访问模式本质上 难以扩展。为了提前识别这些有风险的模式，我们必须记录访问和大小的期望。 在 MR 描述中包含对这些问题的回答：
    • 新表在未来 3 个月、6 个月、1 年的预期增长是多少？这些基于什么假设？
    • 你预计此表在 3 个月、6 个月、1 年内每小时有多少次读写？在什么情况下会更新行？这些基于什么假设？
    • 基于预期的数据量和访问模式，新表是否对 GitLab.com 或 GitLab Self-Managed 实例构成可用性风险？所提出的设计是否能够扩展以支持 GitLab.com 和 GitLab Self-Managed 客户的需求？

删除列、表、索引或其他结构时的准备

  • 遵循 删除列指南。
  • 通常最佳实践（但不是硬性规定）是在后部署迁移中删除索引和外键。
    • 例外情况包括删除小型表的索引和外键。
  • 如果你添加复合索引，另一个索引可能变得冗余，因此在同一迁移中删除它。 例如，添加 index(column_A, column_B, column_C) 会使索引 index(column_A, column_B) 和 index(column_A) 变得冗余。

使用批量更新操作时的准备

使用 update、upsert、delete、update_all、upsert_all、delete_all 或 destroy_all ActiveRecord 方法需要格外小心，因为它们会修改数据并且可能性能不佳，或者如果范围不正确可能会 破坏数据。这些方法也 与公共表表达式 (CTE) 语句不兼容。 当使用这些方法时，Danger 会对合并请求差异发表评论。

遵循 添加或修改查询时的准备 文档，将原始 SQL 查询和查询计划添加到合并请求描述中，并请求数据库审查。

如何进行数据库审查

  • 检查迁移
    • 审查关系建模和设计选择
    • 审查迁移是否遵循 数据库迁移风格指南， 例如
    • 确保迁移在事务中执行或仅包含 并发索引/外键辅助方法（禁用事务）
    • 如果向大型表添加了索引，并且其在 Database Lab 上的执行时间较长（超过 1 小时）：
      • 确保它是在后迁移中添加的。
      • 管理员：合并请求合并后，在 #f_upcoming_release Slack 频道上通知发布经理。
    • 检查与 db/structure.sql 的一致性，并且迁移是 可逆的
    • 检查 db/schema_migrations 下相关的版本文件是否已添加或删除。
    • 检查查询时间（如果有）：在单个事务中，迁移中执行的累积查询时间 需要在 GitLab.com 上舒适地适应 15s - 最好远少于该时间 -。
    • 对于列删除，确保该列已在 之前的发布中被忽略
  • 检查 批处理后台迁移：
    • 估计在 GitLab.com 上执行的时间。出于历史原因， 强烈建议在合并请求描述中包含此估计。 这可以是预期批次数乘以延迟间隔。
    • 在 test 阶段手动触发 数据库测试 作业（db:gitlabcom-database-testing）。
    • 如果单个 update 低于 1s，则查询可以直接放在 常规迁移中（在 db/migrate 内）。
    • 后台迁移通常用于，但不限于：
      • 迁移大型表中的数据。
      • 对数据集中的每条记录进行大量 SQL 查询。
    • 审查查询（例如，确保批处理大小合适）
    • 由于执行时间可能比常规迁移长， 建议将后台迁移视为 后迁移： 将它们放在 db/post_migrate 而不是 db/migrate 中。
  • 检查 迁移的时间指南
  • 检查迁移是否可逆并实现 #down 方法
  • 检查新表迁移：
    • 声明的访问模式和体积是否合理？它们基于的假设是否合理？这些模式是否对稳定性构成风险？
    • 列是否 排序以节省空间？
    • 是否有对其他表引用的外键？
    • 表在 db/fixtures/development/ 中是否有 fixture？
  • 检查数据迁移：
    • 估计在 GitLab.com 上执行的时间。
    • 根据时间，数据迁移可以放在常规、部署后或后台迁移中。
    • 数据迁移也应可逆，或者尽可能附带如何回滚的描述。 这适用于所有类型的迁移（常规、部署后、后台）。
  • 查询性能
    • 检查任何过于复杂的查询以及作者特别 指出需要审查的查询（如果有）
    • 如果没有，请作者提供 SQL 查询和查询计划 使用 Database Lab
    • 对于给定的查询，审查有关数据分布的参数
    • 检查查询计划 并建议改进 查询（更改查询、架构或添加索引等）
    • 一般准则是查询执行时间低于 100ms
    • 避免 N+1 问题并最小化 查询计数。

有用提示

  • 如果你发现自己经常从特定分支应用和回滚迁移，你可能想尝试使用 scripts/database/migrate.rb 使此过程更高效。