MergeRequests::KeepAroundRefsWorker enqueues duplicate jobs per merge and has no deduplication, the concurrent WriteRefs race in Gitaly Cluster

Summary

MergeRequests::KeepAroundRefsWorker gets enqueued from an after_commit callback on MergeRequest that fires on every transaction once merge_commit_sha is present, not just when it gets set:

after_commit :enqueue_keep_around_commit, unless: :importing?

def enqueue_keep_around_commit
  return unless merge_commit_sha.present?

  MergeRequests::KeepAroundRefsWorker.perform_async(
    [project.id],
    [merge_commit_sha],
    self.class.name
  )
end

The merge flow commits the MR record several times (setting merge_commit_sha, clearing in_progress_merge_commit_sha, state transition), so a single merge enqueues several jobs with identical arguments. The worker declares idempotent! and retry: 20 but no deduplicate, so nothing collapses them. On our instance one merge reliably produces 4 jobs for the same SHA within ~600ms (same correlation_id, caller MergeWorker).

Each job issues a Gitaly WriteRef for the same refs/keep-around/<sha>. On plain Gitaly the duplicates are just wasted RPCs. On Gitaly Cluster they race each other through Praefect's transaction voting: the loser gets its transaction canceled, git update-ref is killed while holding the ref lockfile, and the orphaned lock freezes replication on the secondaries until someone deletes it by hand. Full Gitaly-side analysis with log evidence in gitaly#7331. We had three frozen replicas on one morning from this, on repositories with completely normal merge activity.

Why this got loud in 19.2

The async path came in with !223665 (merged) behind async_keep_around_refs_for_merge_request_diffs. !243873 (merged) (19.2) removed the flag, so it is now unconditional and there is no way to opt out. Our numbers around the 19.1.2 to 19.2.1 upgrade: zero KeepAroundRefsWorker jobs before, about 3700 on the first day after, 38 killed update-ref processes and 880 failed replication attempts on the first morning.

Environment

  • GitLab EE Omnibus 19.2.1, Gitaly Cluster (Praefect, 3 storages)
  • Worker source quoted from current master, behavior observed on 19.2.1

Proposal

Either or both of:

  • deduplicate :until_executed on the worker. The duplicates all come from the same call site with identical arguments, this would collapse them.
  • Enqueue only when the SHA actually changed, e.g. if: -> { saved_change_to_merge_commit_sha? } instead of firing on every commit of an already-merged MR.

That fixes the trigger. The underlying Gitaly issue (canceled transaction orphans the lockfile) is tracked in gitaly#7331 and would still deserve a fix of its own, concurrent ref writes can always happen.

Edited by 🤖 GitLab Bot 🤖