Switch Gerrit submit type of integration repos to merge if necessary
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	JanZerebecki
	Mar 26 2016, 9:06 PM

Description

I'd like to switch the Gerrit submit type of the integration repositories to merge if necessary instead of fast forward only. The CI does the right thing while testing which reduces the chance that an error is caused by a conflict free merge. While there is still a slim chance that the effect is not what a human intended it is IMHO not worth the hassle. (See also T111426 .)

Related Objects

Mentioned In: T210442: Zuul merger should reject merge when Gerrit repository has Allow Content Merge set to false
T111426: Zuul can't merge two patches at once on integration/config.git because of fast-forward only Gerrit strategy
Mentioned Here: T49056: Jenkins: Set up postmerge job to auto-deploy jenkins-job-builder configuration
T111426: Zuul can't merge two patches at once on integration/config.git because of fast-forward only Gerrit strategy

Event Timeline

JanZerebecki created this task.Mar 26 2016, 9:06 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 26 2016, 9:06 PM

For integration/config, I prefer being reminded that I need to rebase before I deploy with jjb to make sure I'm picking up all the latest changes and not overwriting anything.

Paladox added a subscriber: Paladox.Mar 28 2016, 5:13 PM

git fetch && if [ "$(git log --oneline HEAD..origin)" ]; then echo "HEAD is outdated!"; else DEPLOY fi

Would that work for you?

T49056 would be even nicer.

I agree with @Legoktm. While unintended side-effects from unconflicted merges are unlikely. There is no question that deploying something will override even unconflicted parts since we do that independently on our own workstation.

However, we don't have a lock on git submissions between "compile locally and push to Jenkins" and "merge in Gerrit". Meaning, even now this may lead to pushing job updates that concurrent updates from someone else. Although at least you'll find out when you do end up merging so you know to re-push to Jenkins (and not blindly rebase to merge anyway).

This workflow would work with merges and would also solve the race condition mentioned: merge patch, fetch, deploy to jenkins, check if origin/master changed. If yes redo, if no then no concurrent change happened. Would you be fine with that?

Krinkle removed a subscriber: Krinkle.Apr 9 2016, 9:33 PM

operations/mediawiki-config has the strategy "Rebase if necessary". I noticed that on https://gerrit.wikimedia.org/r/#/c/349413/6 which got automatically rebased / submitted on successfull gate-and-submit.

Mentioned in SAL (#wikimedia-releng) [2017-05-30T22:08:05Z] <hasharAway> Changed integration/config.git submit type from "Fast forward only" to "Rebase if Necessary" T131008

hashar claimed this task.May 30 2017, 10:09 PM

hashar triaged this task as Medium priority.

hashar added a project: Release-Engineering-Team (Kanban).

hashar moved this task from Backlog to In-progress on the Release-Engineering-Team (Kanban) board.

I gave it a try on integration/config change https://gerrit.wikimedia.org/r/#/c/356431/1 which is not fast forward. The change parent is not the tip of the branch. I did a +2 and zuul-merger used Merging refs/changes/31/356431/1 with args ['-s', 'resolve', 'FETCH_HEAD'].

Using gerrit query 356431 --submit-records --current-patch-set shows as kind: REWORK which is Nontrivial content changes and the web interface shows it is in Merge Conflict.

The different kind of patches are listed on https://gerrit.wikimedia.org/r/Documentation/json.html#patchSet

kind
Kind of change uploaded.

REWORK
Nontrivial content changes.

TRIVIAL_REBASE
Conflict-free merge between the new parent and the prior patch set.

MERGE_FIRST_PARENT_UPDATE
Conflict-free change of first (left) parent of a merge commit.

NO_CODE_CHANGE
No code changed; same tree and same parent tree.

NO_CHANGE
No changes; same commit message, same tree and same parent tree.

Since Gerrit knows it is not going to submit the change (due to REWORK), Zuul should have the information and prevent the patch from being enqueued.

The 'error' on Gerrit side:

[2017-06-01 12:25:11,157] [SSH gerrit review --project integration/config --message "Gate pipeline build succeeded.

- integration-config-tox-jessie https://integration.wikimedia.org/ci/job/integration-config-tox-jessie/3813/console : SUCCESS in 1m 11s
- integration-zuul-layoutvalidation-gate https://integration.wikimedia.org/ci/job/integration-zuul-layoutvalidation-gate/3187/console : SUCCESS in 51s
" --verified 2 --submit 356431,1 (jenkins-bot)] ERROR com.google.gerrit.server.git.MergeOp : [356431-1496319911131-309f739b]

Error from integrateIntoHistory
com.google.gerrit.server.git.IntegrationException: Cannot rebase ea9dacb9ccf141f8debd16f68521714f8dfdd196: The change could not be rebased due to a conflict during merge.

        at com.google.gerrit.server.git.MergeOp.integrateIntoHistory(MergeOp.java:497)
        at com.google.gerrit.server.git.MergeOp.merge(MergeOp.java:433)
        at com.google.gerrit.server.change.Submit.apply(Submit.java:225)
        at com.google.gerrit.server.api.changes.RevisionApiImpl.submit(RevisionApiImpl.java:183)
        at com.google.gerrit.server.api.changes.RevisionApiImpl.submit(RevisionApiImpl.java:177)
        at com.google.gerrit.sshd.commands.ReviewCommand.reviewPatchSet(ReviewCommand.java:333)
        at com.google.gerrit.sshd.commands.ReviewCommand.run(ReviewCommand.java:241)
        at com.google.gerrit.sshd.SshCommand$1.run(SshCommand.java:35)
        at com.google.gerrit.sshd.BaseCommand$TaskThunk.run(BaseCommand.java:442)
        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.access$201(ScheduledThreadPoolExecutor.java:180)
        at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:293)
        at com.google.gerrit.server.git.WorkQueue$Task.run(WorkQueue.java:417)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
        at java.lang.Thread.run(Thread.java:748)
Caused by: com.google.gerrit.server.git.UpdateException: com.google.gerrit.server.git.IntegrationException: Cannot rebase ea9dacb9ccf141f8debd16f68521714f8dfdd196: The change could not be rebased due to a conflict during merge.
        at com.google.gerrit.server.git.BatchUpdate.executeUpdateRepo(BatchUpdate.java:675)
        at com.google.gerrit.server.git.BatchUpdate.execute(BatchUpdate.java:400)
        at com.google.gerrit.server.git.MergeOp.integrateIntoHistory(MergeOp.java:478)
        ... 16 more
Caused by: com.google.gerrit.server.git.IntegrationException: Cannot rebase ea9dacb9ccf141f8debd16f68521714f8dfdd196: The change could not be rebased due to a conflict during merge.
        at com.google.gerrit.server.git.strategy.RebaseIfNecessary$RebaseOneOp.updateRepoImpl(RebaseIfNecessary.java:147)
        at com.google.gerrit.server.git.strategy.SubmitStrategyOp.updateRepo(SubmitStrategyOp.java:116)
        at com.google.gerrit.server.git.BatchUpdate.executeUpdateRepo(BatchUpdate.java:657)
        ... 18 more
Caused by: com.google.gerrit.extensions.restapi.MergeConflictException: The change could not be rebased due to a conflict during merge.
        at com.google.gerrit.server.change.RebaseChangeOp.rebaseCommit(RebaseChangeOp.java:232)
        at com.google.gerrit.server.change.RebaseChangeOp.updateRepo(RebaseChangeOp.java:139)
        at com.google.gerrit.server.git.strategy.RebaseIfNecessary$RebaseOneOp.updateRepoImpl(RebaseIfNecessary.java:143)
        ... 20 more

From https://gerrit.wikimedia.org/r/Documentation/project-configuration.html#rebase_if_necessary :

Rebase If Necessary

If the change being submitted is a strict superset of the destination branch, then the branch is fast-forwarded to the change. If not, then the change is automatically rebased and then the branch is fast-forwarded to the change.

When Gerrit tries to do a merge, by default the merge will only succeed if there is no path conflict. A path conflict occurs when the same file has also been changed on the other side of the merge.

If Allow content merges is enabled, Gerrit will try to do a content merge when a path conflict occurs.

Mentioned in SAL (#wikimedia-releng) [2017-06-01T13:10:06Z] <hashar> Gerrit allow content merge for integration/config ( https://gerrit.wikimedia.org/r/#/admin/projects/integration/config ) - T131008

After setting Allow content merge:

the change is now in Ready to Submit and removing the Verified-2: Needs Verified Label. Ie the merge conflict is gone. It is still in kind REWORK.
the list of opened changes https://gerrit.wikimedia.org/r/#/q/project:integration/config+is:open is no more cluttered with most patches being in merge conflict.

Eventually the change got rebased/merged https://gerrit.wikimedia.org/r/#/c/356431/2

\O/

Mentioned in SAL (#wikimedia-releng) [2017-06-01T13:39:04Z] <hashar> Gerrit: change integration.git project to "Rebase if Necessary" with "Allow content merges" - T131008

all integration repos now have rebase / allow content merge. integration/config specially is fixed :-}

hashar merged a task: T111426: Zuul can't merge two patches at once on integration/config.git because of fast-forward only Gerrit strategy.Jun 1 2017, 1:49 PM

hashar added a subscriber: jayvdb.

And that also fix T111426: Zuul can't merge two patches at once on integration/config.git because of fast-forward only Gerrit strategy which is that when two changes received a CR+2, only the first would merge while the second was being rejected because it is no more fast forward.

hashar moved this task from In-progress to Done (within RelEng) on the Release-Engineering-Team (Kanban) board.Jun 27 2017, 9:05 PM

• Phabricator_maintenance edited projects, added RelEng-Archive-FY201718-Q1; removed Release-Engineering-Team (Kanban).Sep 26 2017, 11:47 PM

hashar mentioned this in T210442: Zuul merger should reject merge when Gerrit repository has Allow Content Merge set to false.Nov 26 2018, 10:39 PM

Switch Gerrit submit type of integration repos to merge if necessaryClosed, ResolvedPublicActions

Description

Related Objects

Event Timeline

Switch Gerrit submit type of integration repos to merge if necessary
Closed, ResolvedPublic
Actions