Queue BBM to clean up dual sharding keys in notes table

Chen Zhangrequested to merge
duo/feature/569520-cleanup-dual-sharding-keys-in-notes into master

What does this MR do and why?

Queue a batched background migration (BBM) that nulls namespace_id on notes rows where both namespace_id and project_id are populated, and fix the write paths that still dual-key, so a follow-up MR can tighten the multi-column NOT NULL constraint from >= 1 to = 1.

Scope note: the num_nonnulls(...) = 1 NOT VALID constraint was removed from this MR (commit a45ec885) and deferred to #601451. Enforcing = 1 while ~2.29B dual-key rows still exist would break ResolvableNote#resolve!/unresolve! (which use update_all, so Postgres re-checks the NOT VALID constraint on every UPDATE of an already-violating row → PG::CheckViolation) during the multi-week BBM window. The constraint MR is opened only after this BBM drains dual-key rows on GitLab.com.

What

  • Adds Gitlab::BackgroundMigration::CleanupDualShardingKeysInNotes, a BBM that runs UPDATE notes SET namespace_id = NULL WHERE namespace_id IS NOT NULL AND project_id IS NOT NULL in sub-batches.
  • Adds the post-deployment migration to enqueue the BBM, plus the BBM YAML doc and schema_migrations entry.
  • Adds a wiki user-mention trigger fallback migration and a synchronized temporary index.
  • Repairs Note#ensure_namespace_id / ensure_organization_id / save_markdown so creates and updates set exactly one sharding key and clear stray keys (pre-existing dual-key rows are repaired when next saved) — this makes the follow-up = 1 constraint safe once it lands.
  • Fixes the project/epic import paths so imported notes are single-keyed.
  • Adds RSpec coverage for the BBM class and the queue migration.

This MR does not add the = 1 constraint — see the scope note above.

Reviewer's guide (file groups)

This MR makes notes single-sharding-keyed on the write path and cleans up existing dual-key rows via a BBM. The write-path changes (groups 1 & 2) fix every place that still dual-keys, so that once the BBM has drained the backlog a follow-up MR (#601451) can safely add the num_nonnulls(...) = 1 constraint. The constraint is intentionally not in this MR — see the scope note at the top.

The behaviour-neutral spec fixtures (note_metadata, editable, snippet fixture, vulnerability-states, the serializer shared example) were split out and merged separately in !246000 (merged), so they no longer appear here.

1. Note model single-keying (write-path normalization)

  • app/models/note.rb, ee/app/models/ee/note.rb — ensure_namespace_id / ensure_organization_id / save_markdown set exactly one sharding key (project_id XOR namespace_id XOR organization_id) and clear stray keys, on create and update (so pre-existing dual-key rows are repaired when next saved).
  • Specs: spec/models/note_spec.rb, ee/spec/models/note_spec.rb, spec/models/concerns/mentionable_spec.rb, spec/services/organizations/transfer/concerns/organization_updater_spec.rb, spec/support/shared_examples/models/concerns/notes/model_with_associated_note_shared_examples.rb, spec/support/helpers/notes_sharding_key_helpers.rb, spec/spec_helper.rb.

2. Import path fixes

  • lib/gitlab/import_export/{project/relation_factory,base/relation_object_saver,group/relation_tree_restorer}.rb, ee/app/services/work_items/legacy_epics/imports/create_from_imported_epic_service.rb — project/epic import produce single-keyed notes (drop the exported namespace_id for project notes; attach the noteable before validating).

3. BBM + migrations (the cleanup)

  • lib/gitlab/background_migration/cleanup_dual_sharding_keys_in_notes.rb + db/post_migrate/20260724100002_queue_cleanup_dual_sharding_keys_in_notes.rb — the cleanup BBM and its queue migration.
  • db/post_migrate/20260724100000_add_project_fallback_to_wiki_user_mention_trigger.rb, 20260724100001_sync_tmp_index_... — wiki user-mention trigger fallback and the synchronized temp index (the temp index makes the BBM's batch-bound lookups cheap).
  • db/{structure.sql,schema_migrations/*}, db/docs/*.yml, migration specs, spec/db/schema_spec.rb, and the BBM/backfill specs.

The num_nonnulls(...) = 1 constraint is deferred to #601451 and is not part of this MR.

Why

Part of the GitLab database sharding strategy (see issue #569520). The notes table uses project_id as the authoritative sharding key for project-scoped notes. A previous BBM (MR !191155 (merged), milestone 18.4) backfilled namespace_id on rows that had only project_id. That left ~2.29B rows with both keys populated. The existing NOT VALID multi-column constraint (>= 1 of project_id, namespace_id, organization_id) was added in migration 20251017182313. To tighten it to = 1 in a follow-up MR, we must first clear the redundant namespace_id from dual-populated rows.

Phase 1 findings

  • ~2.94B total rows in notes; ~2.29B (~78%) have both namespace_id and project_id populated.
  • Spot-check on id BETWEEN 1 AND 10_000_000: 4,008,252 dual-populated rows; only 4 had a namespace_id that did not match projects.project_namespace_id (~1 per million mismatch rate).
  • Because project_id is authoritative and the constraint flip only requires exactly one sharding key, the few mismatched namespace_id values are discarded by the cleanup anyway. No per-row reconciliation against projects is needed. A straight SET NULL is correct and safe.
  • A JOIN-based reconciliation would cost ~7.4 min and ~19 GB read per 10M id window — intentionally avoided.

Migration sizing

Parameter Value
BATCH_SIZE 25,000
SUB_BATCH_SIZE 250
DELAY_INTERVAL 2 minutes

SUB_BATCH_SIZE was sized down from 1,000 to 250 based on EXPLAIN ANALYZE evidence (see below) to keep per-statement latency under the BBM ~100ms guideline on a table with 16 indexes.

Wall-clock duration will be weeks to months given the ~2.29B rows to process. DBRE (@gitlab-com/gl-infra/database-reliability) should review the rollout plan and WAL impact before enabling on GitLab.com.

Out of scope / follow-ups

  • The num_nonnulls(...) = 1 NOT VALID constraint — deferred to #601451. Added in a separate MR once this BBM has drained the dual-key rows on GitLab.com, to avoid PG::CheckViolation on update_all write paths (e.g. ResolvableNote#resolve!) during the multi-week backfill window.
  • Constraint validation (VALIDATE CONSTRAINT + dropping the old >= 1 constraint) — follows the = 1 constraint MR above, also tracked in #601451.
  • Cascading tables (system_note_metadata, award_emoji): separate follow-up issues.

Queries and query plans

All plans captured on a gitlab-production-main clone in postgres.ai (session #52028). The sample id range 2_900_000_000..2_900_101_000 sits in a densely populated, recent region of notes (MAX(id) ≈ 2.94B, ~99% dual-populated per the Rows Removed by Filter counts).

Sub-batch UPDATE (SUB_BATCH_SIZE = 250)

UPDATE notes
SET namespace_id = NULL
WHERE notes.id BETWEEN <batch_min> AND <batch_max>
  AND (namespace_id IS NOT NULL AND project_id IS NOT NULL)
  AND notes.id >= <sub_batch_min>
  AND notes.id < <sub_batch_max>;

Steady-state cost is well within the BBM ~100ms per-statement target. All 16 index RowExclusiveLocks are acquired via fastpath.

Sub-batch boundary lookup (each_sub_batch OFFSET probe)

SELECT notes.id
FROM notes
WHERE notes.id BETWEEN <batch_min> AND <batch_max>
  AND (namespace_id IS NOT NULL AND project_id IS NOT NULL)
  AND notes.id >= <sub_batch_min>
ORDER BY notes.id ASC
LIMIT 1
OFFSET 250;

Why SUB_BATCH_SIZE = 250 and not 1000

For reviewer context, we tested SUB_BATCH_SIZE = 1000 first:

That is ~4× over the ~100ms BBM guideline on a hot table. Per-statement cost scaled near-linearly with sub-batch size, so dropping to 250 brings the steady-state UPDATE comfortably under the guideline without changing total migration work (only the size and duration of each lock window).

Migration testing pipeline (against the previous SUB_BATCH_SIZE=1000 configuration): https://ops.gitlab.net/gitlab-com/database-team/gitlab-com-database-testing/-/pipelines/5806774. A fresh testing run will be triggered once this MR is pushed with the updated constant.

References

Related to #569520

Screenshots or screen recordings

N/A — database migration only.

How to set up and validate locally

  1. Run the post-deployment migration: bundle exec rails db:migrate:post_deploy
  2. Verify the BBM is queued: Gitlab::Database::BackgroundMigration::BatchedMigration.find_by(job_class_name: 'CleanupDualShardingKeysInNotes')
  3. Run the BBM spec: bundle exec rspec spec/lib/gitlab/background_migration/cleanup_dual_sharding_keys_in_notes_spec.rb
  4. Run the queue migration spec: bundle exec rspec spec/migrations/20260724100002_queue_cleanup_dual_sharding_keys_in_notes_spec.rb

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.

Edited by Chen Zhang

Merge request reports