Skip to content

operations: Skip precursory update of target ref in UserMergeToRef

UserMergeToRef performs three steps when computing a merge:

1. The target reference is force-overwritten with whatever the
   first-parent-ref is currently pointing to.
2. The merge is computed.
3. The target reference gets updated with the resulting merge
   commit.

This sequence is problematic with transactions: while it is an expected failure mode that the second step fails, we have already updated a reference and thus cast a vote before returning an error. Because we will always create a replication job if there's been at least one vote and the primary returned an error, we'll thus do so and cause excessive replication to occur even though it shouldn't be needed.

It's questionable whether the first step is by design or whether it serves as a means against races. It certainly feels unexpected to me that an RPC which fails with PreconditionFailed because of conflicts still modifies references, and this behaviour is not documented either.

This commit goes with the assumption that the purpose of the current code is to protect against races because computing the merge can take a rather long time, and we don't want to overwrite any reference which has changed while we were computing the merge. This can be implemented in a much more graceful manner though by first reading the target reference, computing the merge and then doing a git-update-ref(1) with the revision we've just read as expected old value. This avoids doing the precursory update of the reference and thus also casting the vote, and as a result we now don't trigger replication anymore.

Given that we're operating based on assumptions here, this change is implemented behind a feature flag. This flag only acts as an escape hatch though and is thus default-enabled.

Merge request reports