Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Successfully Merging the Work of 1000 Developers

engineering.shopify.com

Post reply on HN
Print view View original

51–60 of 108 posts

Re: Successfully Merging the Work of 1000 Developers

#51

Post by chrisseaton »

omeze wrote: post #43
DilumAluthge wrote: post #28

This is exactly the kind of workflow that Bors ( https://github.com/bors-ng/bors-ng ) automates. In addition to Bors, there are a number of apps and services that automate this kind of workflow. Here is an incomplete list: https://forum.bors.tech/t/other-apps-that-implement-the-nrsr... Edited to add: Graydon Hoare (creator of Rust) called this the Not Rocket Science Rule Of Software Engineering (NRSROSE): "automatica…

Bors when introduced at my workplace was summarily disabled after 2 separate incidents where it was improperly applying commit messages (which we patched internally) and improperly applying patches (at which point we disabled it and explored other solutions). Combined with the merge latency overhead, I would be extremely hesitant to advocate running it at scale.

> Bors when introduced at my workplace was summarily disabled after 2 separate incidents where it was improperly applying commit messages

Seems a bit extreme to discard something after finding two bugs. Could you have fixed them instead?

Re: Successfully Merging the Work of 1000 Developers

#52

Post by dkoston »

byroot wrote: post #34
dkoston wrote: post #27

Earlier quoted context omitted.

Interesting. It seems like you have a very flexible process of how to launch code which could contribute to issues with visibility and rollbacks. I’m curious as to why you had a queue instead of a develop branch before moving to CD? Was this to allow arbitrary commits to be launched to production rather than getting them batched by time?

The queue is simply an automated "develop" branch.

From what I gathered in the article, that’s the case now but before the queue required manual merges.

Re: Successfully Merging the Work of 1000 Developers

#53

Post by wvanbergen »

dkoston wrote: post #27
jacktli wrote: post #7

Earlier quoted context omitted.

Hi, Author here! Pull requests are our unit of work, and the queue was created to support all pull requests. We do have feature flags as a tool, but we let our developers make the judgment call on how their changes should be rolled out.

Interesting. It seems like you have a very flexible process of how to launch code which could contribute to issues with visibility and rollbacks. I’m curious as to why you had a queue instead of a develop branch before moving to CD? Was this to allow arbitrary commits to be launched to production rather than getting them batched by time?

A `develop` branch has several disadvantages.

You will want to make your `develop` branch the default branch in git and on GitHub, to make sure pull requests automatically are targeted properly (not doing this would be a major UX pain). However, that means that when you `git clone` a repository you are not guaranteed to get a working version.

The `develop` branch can still be broken, which is a problem that needs to be addressed. While you can revert breaking changes (or force-push it to a previous known good sha), and you can automate this process, the pull request is already marked as merged at this point. This means that developers have to open a new PR whenever that happens.

With the queue approach, pull requests remain open until we are sure they integrate properly. Also, we have the opportunity to use multiple branches to test different permutations of PRs, so we can still progress and merge some PRs even if the "happy path" that includes all PRs does not integrate properly.

Re: Successfully Merging the Work of 1000 Developers

#54

Post by wvanbergen »

evfanknitram wrote: post #48
jacktli wrote: post #7

Earlier quoted context omitted.

Hi, Author here! Pull requests are our unit of work, and the queue was created to support all pull requests. We do have feature flags as a tool, but we let our developers make the judgment call on how their changes should be rolled out.

Is anyone "signing off" on the deploys or is it fully automatic? I can't really imagine it being manual 40 times per day, but just wanted to hear. How do you handle the scenario that some developer pushes a send_me_all_the_credit_card_details() function to the code base which does something 'evil'? Do you rely on the reviewer "doing their works properly" to handle that? I'm not saying formal "signing off"-steps in pr…

We generally require 2 reviewers, and no sign-off on deploys. For PCI-compliant code things work a bit differently, but tries to follow this as closely as possible.

Re: Successfully Merging the Work of 1000 Developers

#55

Post by wvanbergen »

dkoston wrote: post #22
tekstar wrote: post #4

Earlier quoted context omitted.

Shopify always runs CI on branches before merging to master. Everything this article describes is in addition to that, in order to deal with the problems the article talks about at "merge to master" time, like 2 merged PRs failing or a stale PR that passed on branch but fails on master due changes. At this scale you need to be deploying constantly, otherwise deploys are hundreds of commits large and its impossible to…

Sorry if my comment was unclear. I consider the queue to be a “branch” as well. Many people use a “develop” branch instead of a queue in this instance. The queue appears designed to allow arbitrary selection rather than merging in order (though the new solution with CD seems generally in order) Totally agree that CD is required with this many commits. It’s commonplace on teams with many fewer developers. Was surprise…

Generally our metrics and exception reports are tagged with the sha and the deploy stage.

Re: Successfully Merging the Work of 1000 Developers

#56

Post by byroot »

dkoston wrote: post #52
byroot wrote: post #34

Earlier quoted context omitted.

The queue is simply an automated "develop" branch.

From what I gathered in the article, that’s the case now but before the queue required manual merges.

No, even with v1, the merge weren't manual. A bot would merge for you, but directly into master.

Now the bot merges into a temporary that is fast-forwarded as the new master if CI validates it.

Re: Successfully Merging the Work of 1000 Developers

#57

Post by byroot »

pojzon wrote: post #42
byroot wrote: post #37

Earlier quoted context omitted.

It was tested. It's simply that between the time you pushed on your branch, and the time you merged, many other commits made it to the master branch, potentially breaking your branch.

That pretty much forces tooling to try to rebase and run CI either way ? If it cannot rebase its up to the developer to fix his branch. Master should be always in a state of release at any moment. I just cannot imagine it was an unknown practice for some.

Except that with the amount of activity on the repo it's simply impossible.

Every new merge on master would require to rebase several hundred branches being worked on or awaiting reviewed. Multiply this with the hundreds of commits merged on master every day and you end up with way too much CI jobs to run.

Re: Successfully Merging the Work of 1000 Developers

#58

Post by dkoston »

wvanbergen wrote: post #53
dkoston wrote: post #27

Earlier quoted context omitted.

Interesting. It seems like you have a very flexible process of how to launch code which could contribute to issues with visibility and rollbacks. I’m curious as to why you had a queue instead of a develop branch before moving to CD? Was this to allow arbitrary commits to be launched to production rather than getting them batched by time?

A `develop` branch has several disadvantages. You will want to make your `develop` branch the default branch in git and on GitHub, to make sure pull requests automatically are targeted properly (not doing this would be a major UX pain). However, that means that when you `git clone` a repository you are not guaranteed to get a working version. The `develop` branch can still be broken, which is a problem that needs to…

Thanks, I was hoping for more of this in the blog post. Since tools are just an expression of process/policy, it’s more interesting to here about the process and why than it is about building “yet another CD tool”. Appreciate the thoughtful and thorough response.

The major pain point I agree with on develop is changing the defaults to merge to that rather than master. It’s a shame this is not easier to do in git/github.

I’m not sure I agree with “develop can still be broken” as an issue that supports a queue. Whether it’s a queue or develop, one should run CI on each change to validate that merging it to master will not cause issues. It’s possible for both to be broken via the same scenarios just as it’s possible for master to be broken. Since CI runs before the branch is merged to develop and upon merge, a failure would “stop the world” and prevent more code from being merged unless that code fixes the failure.

I guess I’m not fully understanding how a queue prevents this. Since you don’t have a full picture of the state of master until something is merged from the queue, how do the CI checks in the queue prevent things that branch-based CI checks wouldn’t prevent in a “develop” branch? With branches and develop, pull requests remain open until they can be assured they merge properly with develop as well.

For clarity, I’m not arguing that a develop branch is the way to go, I think CD is much better.

Maybe I’m missing something big here but using multiple branches is permissible in other setups also. You can cherry pick a bunch of commits to a branch and test permutations but only certain branches get deployed to staging and production based on rules.

I’m glad that Shopify has found tools and a process that works. Honestly, I’m just having trouble comparing and constraining this to the other tools that are out there. The article never speaks about other approaches and whether or not they were considered and why you decided to go with a queue. It’s not clear to me if this was a case of improving the existing queue system because it was already in place or whether or not the queue was specifically chosen again because it was better than other alternatives (and why).

Re: Successfully Merging the Work of 1000 Developers

#59

Post by dkoston »

byroot wrote: post #56
dkoston wrote: post #52

Earlier quoted context omitted.

From what I gathered in the article, that’s the case now but before the queue required manual merges.

No, even with v1, the merge weren't manual. A bot would merge for you, but directly into master. Now the bot merges into a temporary that is fast-forwarded as the new master if CI validates it.

Interesting.

Would you say this is more of a decision based around the constraints of using GitHub or more of the ideal process for Shopify’s needs?

I’m curious because the article doesn’t mention the core reasons that you chose to write your own CD tool versus the other options that exist. The workflow you describe seems readily available in most tools. Perhaps the throughput was causing other options to break?

Re: Successfully Merging the Work of 1000 Developers

#60

Post by dkoston »

wvanbergen wrote: post #55
dkoston wrote: post #22

Earlier quoted context omitted.

Sorry if my comment was unclear. I consider the queue to be a “branch” as well. Many people use a “develop” branch instead of a queue in this instance. The queue appears designed to allow arbitrary selection rather than merging in order (though the new solution with CD seems generally in order) Totally agree that CD is required with this many commits. It’s commonplace on teams with many fewer developers. Was surprise…

Generally our metrics and exception reports are tagged with the sha and the deploy stage.

Good to hear, that’ll make change management less of a chore.

I think the main thing that was missing for me is the rationale behind building this system rather than building a workflow in one of the existing CI/CD tools. Was there a throughout bottleneck in existing tools? Was there something custom about your workflow that wasn’t supported elsewhere? I may be wrong but the workflow you landed upon seems pretty common so I’m curious as to why the need to build and maintain a tool in house for this?

Post reply on HN