On 06/03/2018 19:44, Marko Obrovac wrote:
Hello,
Today during the scheduled deploy window of moving JobQueue jobs from the Redis-based infrastructure to the new EventBus-based one, a combination of PEBKAC and Gerrit trying to be smart caused an accidental merge of the mw core's master branch into wmf.23. Luckily, the mistake was spotted before the wmf.23 branch got updated on tin, so the blast radius was limited to delaying the Euro-SWAT window by 30 minutes~[1]. Big thanks to Antoine and Giuseppe for their help in discovering and resolving the issues.
You can read the full incident report here~[2]. The TL;DR is that currently MW core repository is configured in such a way so as to allow Gerrit to do implicit (and silent!) merges, even for cherry-picked change-sets, which is something we might want to change in the near future.
Cheers, Marko
[1] Thanks, James F, for patiently waiting on the resolution! [2] https://wikitech.wikimedia.org/wiki/Incident_documentation/20180306-MasterMe...
Marko Obrovac, PhD Senior Services Engineer Wikimedia Foundation
Hello,
That Gerrit feature has been disabled by default for all repositories by setting receive.rejectImplicitMerges = true in All-Projects.
https://gerrit.wikimedia.org/r/#/c/416704/
Implicit merging of a branch into another might be helpful sometime, but most probably all typical use cases would be due to a mistake and could lead to a catastrophe. I blame Gerrit default behavior on that one :]
I would like to thanks Marko and Petr to have followed the deployment guide: * always check current workspace and remote before rebasing: git remote update git log HEAD..HEAD@{u} * halt immediately and ring the bell in case of doubt. Surely have ~90 unwanted commits was a red alarm.
Thank you!
On Wed, Mar 7, 2018 at 2:25 AM Antoine Musso hashar@free.fr wrote:
Implicit merging of a branch into another might be helpful sometime, but most probably all typical use cases would be due to a mistake and could lead to a catastrophe.
Indeed. Surprising this hasn't hit us hard before. To further on "this could be helpful sometime"...if a particular repo or group of repos needed this back on, we can always override it there.
I blame Gerrit default behavior on that one :]
In their defense, the way they view the review/merge model is that you target the lowest necessary supported branch for fixes and new features land in master. Lower branches are routinely merged upwards. The default behavior here totally makes sense in that context and clearly less so in a cherry-pick driven model we use. But yay for an easy fix :)
-Chad
Hi!
In their defense, the way they view the review/merge model is that you target the lowest necessary supported branch for fixes and new features land in master. Lower branches are routinely merged upwards.
My experience has been this usually happens in older projects where most changes is either maintenance work or changes that need to be maintained across multiple branches. PHP does bugfixes this way, fix is put in the least supported branch and then branches are upmerged up to master. So upper branch is assumed to contain all lower branches plus new features.