Backfill diff commits missed by original migration

Why

The original BackfillMergeRequestDiffCommitsToPartitioned left a gap of rows that were not copied to the new commits table because an amendment excluded any row with a NULL sha, commit_author_id or committer_id. This was added because these columns are not nullable in the new table and were causing failures. It was intended to exclude an identified small set of records that needed to be restored (from Gitaly) in a follow-up migration, but unintentionally excluded valid records which saved these columns to the table merge_request_commits_metadata (after enabling merge_request_diff_commits_dedup). The fix should've added the filter to metadata_keys_cte, which protects the NOT NULL columns on merge_request_commits_metadata, but it instead included it in filtered_diff_commits_cte and so also removed rows from the final row copy, which only needs the pointer already on the row.

What

To address this gap, this MR queues a backfill covering the diff IDs in the specific time window between enabling the flags merge_request_diff_commits_dedup and merge_request_diff_commits_partition: the first one started to write to the metadata table, leaving the commit's metadata columns empty; the second one made the sync trigger copy new commits into the partitioned table, so later rows do not need copying.

The BBM runs on GitLab.com only, SM cannot be narrowed to a specific time window and needs to run on the whole table, so it will be handled separately.

Verifying the cursor bounds

Cursor Value Diff created at Chosen because
min_cursor [1548627880, 0] 2025-11-01 00:00:00 UTC First diff on or after 2025-11-01, ahead of the first merge_request_diff_commits_dedup enablement on 2025-11-03 (a test project, then gitlab-org/gitlab on 2025-11-26, log). No row created before that can carry the deduplicated shape
max_cursor [1879503824, 999999] 2026-06-30 23:59:59 UTC Last diff before 2026-07-01, after merge_request_diff_commits_partition was removed (3c7ae0da). From then on the forward sync trigger copied every row with a pointer and a project_id, so rows written after that are already in the partitioned table.

The cursor bounds decide which rows get repaired, so each one was checked on Database Lab rather than taken from the flag dates alone.

Bounds verification queries and results

Each cursor straddles its date

merge_request_diffs has no created_at index, so the boundary IDs were found by binary search on id. The diff just outside each bound must fall on the other side of the date.

Screenshot_2026-09-23_at_12.13.03

Nothing to repair below min_cursor

No deduplicated row can exist before the flag was first enabled. Expect 0 over the 100,000 diff IDs just below the bound.

Screenshot_2026-09-23_at_12.16.55

Nothing missing above max_cursor

Rows above the bound were copied by the forward sync trigger. Expect 0 over the 100,000 diff IDs just above it.

Screenshot_2026-09-23_at_12.17.56

The same query over the 100,000 diff IDs just below the bound is a control: it should find missing rows, which shows the query detects the gap it reports absent above.

Screenshot_2026-09-23_at_12.19.39

Additional details

The query

Every missing row still carries its merge_request_commits_metadata_id pointer, so nothing is inserted into merge_request_commits_metadata; this copies four reference columns.

WITH sub_batch AS MATERIALIZED (
  SELECT "merge_request_diff_commits_archived"."merge_request_diff_id",
         "merge_request_diff_commits_archived"."relative_order",
         "merge_request_diff_commits_archived"."merge_request_commits_metadata_id"
  FROM "merge_request_diff_commits_archived"
  WHERE ("merge_request_diff_id", "relative_order") <= (1879503824, 999999)
    AND ("merge_request_diff_id", "relative_order") >= (1548627880, 0)
  ORDER BY "merge_request_diff_id", "relative_order"
  LIMIT 5000
),
missing_commits AS MATERIALIZED (
  SELECT
    archived.merge_request_diff_id,
    archived.relative_order,
    archived.merge_request_commits_metadata_id,
    mr_diffs.project_id
  FROM sub_batch AS archived
  INNER JOIN merge_request_diffs AS mr_diffs
    ON mr_diffs.id = archived.merge_request_diff_id
  WHERE archived.merge_request_commits_metadata_id IS NOT NULL
  LIMIT 5000
)
INSERT INTO merge_request_diff_commits (
  merge_request_commits_metadata_id, merge_request_diff_id, project_id, relative_order
)
SELECT
  merge_request_commits_metadata_id, merge_request_diff_id, project_id, relative_order
FROM missing_commits
ON CONFLICT (merge_request_diff_id, relative_order, project_id) DO NOTHING

Query plan for one 5,000-row sub-batch from the middle of the window, on a cold clone.

project_id is read from merge_request_diffs because the archived table's own project_id was never backfilled and is NULL across this window. The INNER JOIN also skips commits whose diff has been deleted, as the original backfill did.

Batch sizes and runtime

batch_size 500,000, sub_batch_size 5,000, max_batch_size 1,000,000.

The database testing pipeline on this MR samples below min_cursor, because TestBatchedBackgroundRunner does not offset its sample points by min_cursor, so its batches insert nothing. The numbers below come from a scratch MR, Draft: Do not merge: DB testing scratch for !25... (!257246), that patches the runner to sample inside the cursor window. It is not meant to be merged.

sub_batch_size 10,000 sub_batch_size 5,000
Result note note
Batches sampled / failed 17 / 0 15 / 0
Average batch time 102.02s 113.65s
Insert query, mean / max 1,174.5ms / 6,040.9ms 536.2ms / 4,386.1ms
Scan query, mean 463.1ms 246.0ms
Queries over 1s / over 5s 31% / 55 16% / 0
Scanned rows inserted 56% 53%

Most of the cost is index maintenance on the fully populated partitioned table. In the plan, 6.44s of 6.65s is disk reads, about 5.4s of it in the insert, and nearly every inserted row touches a different page, which also produced 32.5MB of WAL for 5,000 rows. That is the worst case on a cold cache: sampled batches averaged 536ms for the same query. Halving sub_batch_size keeps the cost per row the same but halves each statement, bringing the mean under 1s. It adds about 12s per batch, putting batches at about 95% of the 120s interval. sub_batch_size cannot be changed after queueing.

max_batch_size is capped because batches in sparse parts of the window take about 33s, so the optimizer would otherwise grow them towards its 2,000,000 default. A batch that size in a dense part would run for several minutes and write all of its WAL before a health check can hold the migration.

total_tuple_count is set to the 3,351,569,145 rows between the cursors, since queue_batched_background_migration would otherwise use the whole archived table. At 53-56% inserted, that is roughly 1.8B rows. The pipeline estimates about 9 days 7 hours (total_tuple_count / batch_size * interval).

Reverse sync triggers

This does not depend on !251603 (merged) dropping the reverse sync triggers. Every copied row was read from the archived table, so the reverse insert hits a conflict on its primary key and writes nothing. If the trigger is still present it adds about 22ms per 10,000 rows, roughly 1% of batch time.

Differences from the original backfill

This iterates the physical archived table, so it needs neither the original's custom batching strategy nor its sub_batch_relation override, both of which existed for the views dropped in 20260730091929. It uses the shared each_sub_batch path, which includes the single-lower-bound fix from !236536 (merged); the pipeline SQL shows each sub-batch with one lower and one upper bound.

Testing

The spec covers a deduplicated row being copied, an idempotent re-run, a partially copied diff, a diff spanning several sub-batches, and rows skipped when they have no pointer, no parent diff, or sit above the end cursor. Source rows use the real shape, with a NULL project_id. Each example disables the forward sync triggers, which off GitLab.com would copy every insert into the partitioned table and make the broken state impossible to build. Restoring the original predicate makes the copy examples fail.

References

Edited by Eugenia Grieff

Merge request reports

Loading
Loading