Commit Part Numbers and MozReview

January 27, 2015 at 08:17 PM | categories: Mozilla, code review | View Comments

It is common for commit messages in Firefox to contains strings like Part 1, Part 2, etc. See this push for bug 784841 for an extreme multi-part example.

When code review is conducted in Bugzilla, these identifiers are necessary because Bugzilla orders attachments/patches in the order they were updated or their patch title (I'm not actually sure!). If part numbers were omitted, it could be very confusing trying to figure out which order patches should be applied in.

However, when code review is conducted in MozReview, there is no need for explicit part numbers to convey ordering because the ordering of commits is implicitly defined by the repository history that you pushed to MozReview!

I argue that if you are using MozReview, you should stop writing Part N in your commit messages, as it provides little to no benefit.

I, for one, welcome this new world order: I've previously wasted a lot of time rewriting commit messages to reflect new part ordering after doing history rewriting. With MozReview, that overhead is gone and I barely pay a penalty for rewriting history, something that often produces a more reviewable series of commits and makes reviewing and landing a complex patch series significantly easier.

Read and Post Comments

Automatic Python Static Analysis on MozReview

January 24, 2015 at 11:30 PM | categories: Python, Mozilla, code review | View Comments

A bunch of us were in Toronto last week hacking on MozReview.

One of the cool things we did was deploy a bot for performing Python static analysis. If you submit some .py files to MozReview, the bot should leave a review. If it finds violations (it uses flake8 internally), it will open an issue for each violation. It also leaves a comment that should hopefully give enough detail on how to fix the problem.

While we haven't done much in the way of performance optimizations, the bot typically submits results less than 10 seconds after the review is posted! So, a human should never be reviewing Python that the bot hasn't seen. This means you can stop thinking about style nits and start thinking about what the code does.

This bot should be considered an alpha feature. The code for the bot isn't even checked in yet. We're running the bot against production to get a feel for how it behaves. If things don't go well, we'll turn it off until the problems are fixed.

We'd like to eventually deploy C++, JavaScript, etc bots. Python won out because it was the easiest to integrate (it has sane and efficient tooling that is compatible with Mozilla's code bases - most existing JavaScript tools won't work with Gecko-flavored JavaScript, sadly).

I'd also like to eventually make it easier to locally run the same static analysis we run in MozReview. Addressing problems locally before pushing is a no-brainer since it avoids needless context switching from other people and is thus better for productivity. This will come in time.

Report issues in #mozreview or in the Developer Services :: MozReview Bugzilla component.

Read and Post Comments

End to End Testing with Docker

January 24, 2015 at 11:10 PM | categories: Docker, Mozilla, testing | View Comments

I've written an extensive testing framework for Mozilla's version control tools. Despite it being a little rough around the edges, I'm a bit proud of it.

When you run tests for MozReview, Mozilla's heavily modified Review Board code review tool, the following things happen:

  • A MySQL server is started in a Docker container.
  • A Bugzilla server (running the same code as bugzilla.mozilla.org) is started on an Apache httpd server with mod_perl inside a Docker container.
  • A RabbitMQ server mimicking pulse.mozilla.org is started in a Docker container.
  • A Review Board Django development server is started.
  • A Mercurial HTTP server is started

In the future, we'll likely also need to add support for various other services to support MozReview and other components of version control tools:

  • The Autoland HTTP service will be started in a Docker container, along with any other requirements it may have.
  • An IRC server will be started in a Docker container.
  • Zookeeper and Kafka will be started on multiple Docker containers

The entire setup is pretty cool. You have actual services running on your local machine. Mike Conley and Steven MacLeod even did some pair coding of MozReview while on a plane last week. I think it's pretty cool this is even possible.

There is very little mocking in the tests. If we need an external service, we try to spin up an instance inside a local container. This way, we can't have unexpected test successes or failures due to bugs in mocking. We have very high confidence that if something works against local containers, it will work in production.

I currently have each test file owning its own set of Docker containers and processes. This way, we get full test isolation and can run tests concurrently without race conditions. This drastically reduces overall test execution time and makes individual tests easier to reason about.

As cool as the test setup is, there's a bunch I wish were better.

Spinning up and shutting down all those containers and processes takes a lot of time. We're currently sitting around 8s startup time and 2s shutdown time. 10s overhead per test is unacceptable. When I make a one line change, I want the tests to be instantenous. 10s is too long for me to sit idly by. Unfortunately, I've already gone to great pains to make test overhead as short as possible. Fig wasn't good enough for me for various reasons. I've reimplemented my own orchestration directly on top of the docker-py package to achieve some significant performance wins. Using concurrent.futures to perform operations against multiple containers concurrently was a big win. Bootstrapping containers (running their first-run entrypoint scripts and committing the result to be used later by tests) was a bigger win (first run of Bugzilla is 20-25 seconds).

I'm at the point of optimizing startup where the longest pole is the initialization of the services inside Docker containers themselves. MySQL takes a few seconds to start accepting connections. Apache + Bugzilla has a semi-involved initialization process. RabbitMQ takes about 4 seconds to initialize. There are some cascading dependencies in there, so the majority of startup time is waiting for processes to finish their startup routine.

Another concern with running all these containers is memory usage. When you start running 6+ instances of MySQL + Apache, RabbitMQ, + ..., it becomes really easy to exhaust system memory, incur swapping, and have performance fall off a cliff. I've spent a non-trivial amount of time figuring out the minimal amount of memory I can make services consume while still not sacrificing too much performance.

It is quite an experience having the problem of trying to minimize resource usage and startup time for various applications. Searching the internet will happily give you recommended settings for applications. You can find out how to make a service start in 10s instead of 60s or consume 100 MB of RSS instead of 1 GB. But what the internet won't tell you is how to make the service start in 2s instead of 3s or consume as little memory as possible. I reckon I'm past the point of diminishing returns where most people don't care about any further performance wins. But, because of how I'm using containers for end-to-end testing and I have a surplus of short-lived containers, it is clearly I problem I need to solve.

I might be able to squeeze out a few more seconds of reduction by further optimizing startup and shutdown. But, I doubt I'll reduce things below 5s. If you ask me, that's still not good enough. I want no more than 2s overhead per test. And I don't think I'm going to get that unless I start utilizing containers across multiple tests. And I really don't want to do that because it sacrifices test purity. Engineering is full of trade-offs.

Another takeaway from implementing this test harness is that the pre-built Docker images available from the Docker Registry almost always become useless. I eventually make a customization that can't be shoehorned into the readily-available image and I find myself having to reinvent the wheel. I'm not a fan of the download and run a binary model, especially given Docker's less-than-stellar history on the security and cryptography fronts (I'll trust Linux distributions to get package distribution right, but I'm not going to be trusting the Docker Registry quite yet), so it's not a huge loss. I'm at the point where I've lost faith in Docker Registry images and my default position is to implement my own builder. Containers are supposed to do one thing, so it usually isn't that difficult to roll my own images.

There's a lot to love about Docker and containerized test execution. But I feel like I'm foraging into new territory and solving problems like startup time minimization that I shouldn't really have to be solving. I think I can justify it given the increased accuracy from the tests and the increased confidence that brings. I just wish the cost weren't so high. Hopefully as others start leaning on containers and Docker more for test execution, people start figuring out how to make some of these problems disappear.

Read and Post Comments

Bugzilla and the Future of Firefox Development

January 16, 2015 at 10:50 AM | categories: Bugzilla, Mozilla, code review | View Comments

Bugzilla has played a major role in the Firefox development process for over 15 years. With upcoming changes to how code changes to Firefox are submitted and reviewed, I think it is time to revisit the central role of Bugzilla and bugs in the Firefox development process. I know this is a contentious thing to say. Please, gather your breath, and calmly read on as I explain why I believe this.

The current Firefox change process defaults to requiring a Bugzilla bug for everything. It is rare (and from my experience frowned upon) when a commit to Firefox doesn't reference a bug number. We've essentially made Bugzilla and a bug prerequisites for changing anything in the Firefox version control repository. For the remainder of this post, I'm going to say that we require a bug for any change, even though that statement isn't technically accurate. Also, when I say Bugzilla, I mean bugzilla.mozilla.org, not the generic project.

Before I go on, let's boil the Firefox change process down to basics.

At the heart of any change to the Firefox source repository is a diff. The diff (a representation of the differences between a set of files) is the smallest piece of data necessary to represent a change to the Firefox code. I argue that anything more than the vanilla diff is overhead and could contribute to process debt. Now, there is some essential overhead. Version control tools supplement diffs with metadata, such as the author, commit message, and date. Mozilla has also instituted a near-mandatory code review policy, where changes need to be signed off by a set of trusted individuals. I view both of these additions to the vanilla diff as essential for Firefox development and non-negotiable. Therefore, the bare minimum requirements for changing Firefox code are a diff plus metadata (a commit/patch) and (almost always) a review/sign-off. That's it. Notably absent from this list is a Bugzilla bug. I argue that a bug is not strictly required to change Firefox. Instead, we've instituted a near-universal policy that we should have bugs. We've chosen to add overhead and process debt - interaction with Bugzilla - to our Firefox change process.

Now, this choice to require all changes be associated with bugs has its merits. Bugs provide excellent anchor points for historical context and for additional information after the change has been committed and is forever set in stone in the repository (commits are immutable in Mercurial and Git and you can't easily attach metadata to the commit after the fact). Bugs are great to track relationships between different problems or units of work. Bugs can even be used to track progress towards a large feature. Bugzilla components also provide a decent mechanism to follow related activity. There's also a lot of tooling and familiar process standing on top of the Bugzilla platform. There's a lot to love here and I don't want diminish the importance of all these things.

When I look to the future, I see a world where the current, central role of Bugzilla and bugs as part of the Firefox change process begin to wane. I see a world where the benefits to maintaining our current Bugzilla-centric workflow start to erode and the cost of maintaining it becomes higher and harder to justify. You actually don't have to look too far into the future: that world is already here and I've already started to feel the pains of it.

A few days ago, I blogged about GitHub and its code first approach to change. That post was spun off from an early draft of this post (as were the posts about Firefox contribution debt and utilizing GitHub for Firefox development). I wanted to introduce the concept of code first because it is central to my justification for changing how we do things. In summary, code first capitalizes on the fact that any change to software involves code and therefore puts code front and center in the change process. (In hindsight, I probably should have used the term code centric, because that's how I want people to think about things.) So how does code first relate to Bugzilla and Firefox development?

Historically, code review has occurred in Bugzilla: upload a patch to Bugzilla, ask for review, and someone will look at it. And, since practically every change to Firefox requires review, you need a bug in Bugzilla to contain that review. Thus, one way to view a bug is as a vehicle for code review. Not every bug is just a code review, of course. But a good number of them are.

The only constant is change. And the way Mozilla conducts code review for changes to Firefox (and other projects) is changing. We now have MozReview, a code review tool that is not Bugzilla. If we start accepting GitHub pull requests, we may perform reviews exclusively on GitHub, another tool that is not Bugzilla.

(Before I go on, I want to quickly point out that MozReview is nowhere close to its final form. Parts of MozReview are pretty bad right now. The maintainers all know this and we have plans to fix it. We'll be in Toronto all of next week working on it. If you don't think you'll ever use it because parts are bad today, I ask you to withhold judgement for a few more months.)

In case you were wondering, the question on whether Bugzilla should always be used for code review for Firefox has been answered and that answer is no. People, including maintainers of Bugzilla, realized that better-than-Splinter/Bugzilla code review tools exist and that continuing to invest time to develop Bugzilla/Splinter into a best-in-class code review tool would be better spent integrating Bugzilla with an existing tool. This is why we now have a Review Board based code review tool - MozReview - integrated with Bugzilla. If you care about code quality and more powerful workflows, you should be rejoicing at this because the implementation of code review in Bugzilla does not maximize optimal outcomes.

The world we're moving to is one where code review occurs outside of Bugzilla. This raises an important question: if Bugzilla was being used primarily as a vehicle for code review, what benefit and/or role should Bugzilla play when code review is conducted outside of Bugzilla?

I posit that there are a class of bugs that won't need to exist going forward because bugs will provide little to no value. Put another way, I believe that a growing number of commits to the Firefox repository won't reference bugs.

Come with me on a journey to the future.

MozReview is purposefully being designed in a code and repository centric way. To initiate the formal process for considering a change to code, you push to a Mercurial (or Git!) repository. This could be directly to Mozilla's review repository. If I have my way, this could even be kicked off by submitting a pull request on GitHub or Bitbucket. No Bugzilla attachment uploading here: our systems talk in terms of repositories and commits. Again, this is by design: we don't want submitting code to Mozilla to be any harder than hg push or git push so as to not introduce process debt. If you have code, you'll be able to send it to us.

In the near future, MozReview will stop cross-posting detailed review updates to Bugzilla. Instead, we'll use Review Board's e-mail feature to send its flavor of emails. These will have rich HTML content (or plain text if you insist) and will provide a better experience than Bugzilla ever will. We'll adopt the model of tools like Phabricator and GitHub and only post summaries or links of activity, not full content, to bugs. You may be familiar with the concept as applied to the web: it's called hyperlinking.

Work is being invested into Autoland. Autoland is an automated landing queue that pushes/lands commits semi-automatically once they are ready (have review, pass automation, etc). Think of Autoland as a bot that does all the labor intensive and menial actions around pushing that you do now. I believe Autoland will eventually handle near 100% of pushes to the Firefox repository. And, if I have my way, Autoland will result in the abolishment of integration branches and merge commits in the Firefox repository. Good riddance.

MozReview and Autoland will be highly integrated. MozReview will be the primary user interface for interacting with Autoland. (Some of this should be in place by the end of the quarter.)

In this world, MozReview and its underlying version control repositories essentially become a database of all submitted, pending, and discarded commits to Firefox. The metaphorical primary keys of this database are not bug numbers: they are code/commits. (Code first!) Some of the flags stored in this database tell Autoland what it should do. And the MozReview user interface (and API) provide a mechanism into controlling those flags.

Landing a change in Firefox will be initiated by a simple action such as clicking a checkbox in MozReview. (That could even be the Grant Review checkbox.) Commits cleared for landing will be picked up by Autoland and eventually automatically pushed to the Firefox repository (assuming the build and test automation is happy, of course). Once Autoland takes control, humans are just passengers. We won't be bothered with menial tasks like updating the commit message to reflect a review was performed: this will happen automatically inside MozReview or Autoland. (Although, there's a chance we may adopt some PGP-based signing to more strongly convey review for some code changes in order to facilitate stronger auditing and trust guarantees. Stay tuned.) Likewise, if a commit becomes associated with a bug, we can add that metadata to the commit before it is landed, no human involvement necessary beyond specifying the link in the MozReview web UI (or API). Autoland/MozReview will close review requests and/or bugs automatically. (Are you excited about performing less work yet?)

When commits are added to MozReview, MozReview will read metadata from the repository they came from to automatically determine an appropriate reviewer. (We plan to leverage moz.build files for this in the case of Firefox.) This should eliminate a lot of process debt around choosing a reviewer. Similar metadata will also be used to determine what Bugzilla component a change is related to, static analysis rules to use to critique the phsyical structure of the change, and even automation jobs that should be executed given the set of files that changed. The use of this metadata will erode significant process debt around the change contribution workflow.

As commits are pushed into MozReview/Autoland, the systems will be intelligent about automatically tracking dependencies and facilitating complex development workflows that people run into on a daily basis.

If I create a commit on top of someone else's commit that hasn't been checked in yet, MozReview will detect the dependency between my changes and the parent ones. This is an advantage of being code first: by interfacing with repositories rather than patch files, you have an explicit dependency graph embedded in the repository commit DAG that can be used to aid machines in their activities.

It will also be possible to partially land a series of commits. If I get review on the first 5 of 10 commits but things stall on commit 6, I can ask Autoland to land the already-reviewed commits so they don't get bit rotted and so you have partial progress (psychological studies show that a partial reward for work results in greater happiness through a sense of accomplishment).

Since initiating actions in MozReview is light weight (just hg push), itch scratching is encouraged. I don't know about you, but in the course of working on the Firefox code base, I frequently find myself wanting to make small, 15-30s changes to fix something really minor. In today's world, the overhead for these small changes is often high. I need to upload a separate patch to Bugzilla. Sometimes I even need to create a new bug to hold that patch. If that patch depends on other work I did, I need to set up bug dependencies then worry about landing everything in the right order. All of a sudden, the overhead isn't worth it and my positive intentions go unacted on. Multiplied by hundreds of developers over many years, and you can imagine the effect on software quality. With MozReview, the overhead for itch scratching like this is minor. Just make a small commit, push, and the system will sort everything out. (These small commits are where I think a bugless process really shines.)

This future world revolves around code and commits and operations on them. While MozReview has review in its name, it's more than a review tool: it's a database and interface to code and its state.

In this code first world, Bugzilla performs an ancillary role. Bugzilla is still there. Bugs are still there. MozReview review requests and commits link to bugs. But it is the code, not bugs, that are king. If you want to do anything with code, you interact with the code tools. And Bugzilla is not one of them.

Another way of looking at this is that nearly everything involving code or commits becomes excised from Bugzilla. This would relegate Bugzilla to, well, an issue/bug tracker. And - ta da - that's something it excels at since that's what it was originally designed to do! MozReview will provide an adequate platform to discuss code (a platform that Bugzilla provides today since it hosts code review). So if not Bugzilla tools are handling everything related to code, do you really need a bug any more?

This is the future we're trying to build with MozReview and Autoland. And this is why I think bugs and Bugzilla will play a less central role in the development process of Firefox in the future.

Yes, there are many consequences and concerns about making this shift. You would be rational to be skeptical and doubt that this is the right thing to do. I have another post in the works that attempts to outline some common conerns and propose solutions to many of them. Before writing a long comment pointing out every way in which this will fail to work, I encourage you to wait for that post to be published. Stay tuned.

Read and Post Comments

Modern Mercurial Documentation for Mozillians

January 15, 2015 at 02:45 PM | categories: Mercurial, Mozilla | View Comments

Mozilla's Mercurial documentation has historically been pretty bad. The documentation on MDN (which I refuse to link to) is horribly disjointed and contains a lot of outdated recommendations. I've made attempts to burn some of it to the ground, but it is just too overwhelming.

I've been casually creating my own Mercurial documentation tailored for Mozillians. It's called Mercurial for Mozillians.

It started as a way to document extensions inside the version-control-tools repository. But, it has since evolved to cover other topics, like how to install Mercurial, how to develop using bookmarks, and how to interact with a unified Firefox repository. The documentation is nowhere near complete. But it already has some very useful content beyond what MDN offers.

I'm not crazy about the idea of having generic Mercurial documentation on a Mozilla domain (this should be part of the official Mercurial documentation). Nor am I crazy about moving content off MDN. I'm sure content will move to its appropriate location later. Until then, enjoy some curated Mercurial documentation!

If you would like to contribute to Mercurial for Mozillians, read the docs.

Read and Post Comments

Next Page ยป