Reinsert Duo merge requests instead of mutating them

Norman Debaldrequested to merge
gitlab-community/gitlab-org/gitlab:630852-fix-backfill-created-by-duo-nondeterministic-mutations into master

What does this MR do and why?

Fixes the ClickHouse post-deployment migration 20260910104402_backfill_created_by_duo_on_merge_requests, which fails on self-managed replicated ClickHouse clusters and aborts gitlab-rake gitlab:db:configure during upgrades to 19.4.1.

The migration ran ALTER TABLE merge_requests UPDATE ... WHERE id IN (SELECT ...). On replicated tables ClickHouse rejects mutations with subqueries, because each replica could evaluate the subquery differently. Besides that, merge_requests is replicated by Siphon and must not be changed in place: mutations break the _siphon_replicated_at and _siphon_watermark mechanics and rewrite large parts of a big table.

This MR follows the Siphon guideline for updating rows: the migration reads the affected merge request rows from merge_requests and inserts them again into siphon_merge_requests (Null engine) with _siphon_replicated_at raised by one microsecond. This triggers the existing merge request materialized views, which compute created_by_duo. The migration no longer sets the flag itself.

Details
  • The affected merge requests come from siphon_duo_workflows_workflow_merge_requests (link_type = 1), in batches of 100,000 link IDs with ClickHouse::Iterator.
  • The column list is read from system.columns, so it always matches siphon_merge_requests.
  • All row versions move forward by the same microsecond, so the latest version still wins and deleted merge requests stay deleted. Any later change replicated by Siphon still wins over the reinserted row.
  • INSERT ... SELECT is not a mutation, so allow_nondeterministic_mutations is not involved.
  • Running the migration again only adds new row versions with the same result. Instances where it already succeeded do not run it again. Instances where it failed pick up the fixed version on the next migration run.

References

Screenshots or screen recordings

Not applicable, no UI change. Behavior on a replicated cluster:

Before After
Code: 36. DB::Exception: ALTER UPDATE/ALTER DELETE statement with subquery may be nondeterministic, see allow_nondeterministic_mutations setting. (BAD_ARGUMENTS) Migration completes. Affected merge requests are reinserted and get the correct created_by_duo value from the merge request views.

How to set up and validate locally

The test database uses non-replicated tables, so the spec cannot reproduce the original error. The spec runs the migration, triggers merge_requests_enriched_mv, and checks:

  • created_by_duo is set only for live merge requests with a live created link
  • deleted merge requests are not brought back
  • rows are reinserted with a higher version instead of being changed in place
  • idempotency
  • one insert per batch
  1. Run the migration spec: 
    bundle exec rspec spec/db/click_house/post_migrate/20260910104402_backfill_created_by_duo_on_merge_requests_spec.rb
    Result locally: 5 examples, 0 failures. With the previous ALTER TABLE ... UPDATE migration, the version and batching examples fail.
Not covered by the spec
  • On an upgrade, this migration runs before 20260914090004_create_materialized_view_merge_requests_enriched_mv, so the reinserted rows go through the earlier merge_requests_mv, which computes created_by_duo directly in the view. The test schema always contains the later refreshable enrichment, so this path is not tested.
  • Replicated ClickHouse clusters.

MR acceptance checklist

Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Resolves #630852 (closed)

Edited by Norman Debald

Merge request reports