DUE TO SPAM, SIGN-UP IS DISABLED. Goto Selfserve wiki signup and request an account.
...
In fact, problems may occur even with no delay between testing and merging. Consider two PRs which have different and non-conflicting changes that would interact badly with each other. It is possible that these PRs have a successful test run, but when merged concurrently would break the base branch.
A separate class of problem exists where an author makes a small innocuous change after a PR has been tested and approved. Technically, any change should be fully retested, but there have been circumstances when this has been skipped (intentionally or otherwise).
A solution to both of problems One solution to this is to require PRs to be up-to-date before merging (a configuration option in GitHub). However, since our test suite runs for around 2 hours, this would lead to a never-ending cycle of testing and updating from the base branch.
...
Rather than chasing down problems in our mainline branches after the fact, or requiring up-to-date PRs in GitHub, this KIP proposes to leverage the Merge Queue feature of GitHub.
The merge queue is a queue of all PRs which have been requested to be merged. While the queue is not emptyeffectively gives us the "require up-to-date" feature without the need for constant rebuilding. It does so by placing all PRs into a queue rather than merging them directly. Once in the queue, a Continuous Integration (CI) job will run that applies the PR on top of the base branch and does some validation. Once completeruns our custom job. If successful, the PR is merged to the base branch and the next PR in the queue is built. If the custom job is not successful, the PR remains open and the author is automatically notified. This process continues as long as there are pending PRs to be merged.
The custom job acts as a gatekeeper for trunk. We will start with something similar to our existing "Compile and Check Java" workflow.
GitHub Details
When enabled, the merge queue replaces the "Squash and Merge" button that we use today. We can still do a squash merge, but the merge button becomes "Merge when ready". Clicking this button is tantamount to merging the PR to trunk. Similar to today, only committers are authorized to click the button, and only PRs which have been approved should be merged.
Figure 1: The GitHub UI for merging a Pull Request with the Merge Queue
Only a committer may submit a PR to the merge queue. It is functionally the same as directly merging to the base branch.
This controlled and sequential workflow will allow us to ensure that no change made to Kafka will break our mainline branches.
...
The "Only merge non-failing pull requests" option can help mitigate the risks of batching PRs into one merge group assuming a build concurrency of greater than 1.
Rollout Plan
The extend extent of validation we can perform in the merge queue is governed by the rate of PRs we expect to merge. Taking data from https://github.com/apache/kafka/graphs/commit-activity, we can see that over the last year we have merged around 15 commits per day. Our "Compile and Check Java" CI step is very consistently taking 11-12 minutes on trunk (with no caching). Our full test suite runs in around 2 hours on trunk (again, no caching). With the current rate of change in Kafka, we cannot reasonably run the full test suite for each PR in the merge queue. However, it would quite reasonable to perform compilation and static checks on each PR.
Since there are many unknowns with the merge queue, this KIP proposes that we enable it in a simple configuration with our "Compile and Check Java" step. This will give us the most immediate benefit of the merge queue which is protecting trunk from being broken. By disabling batching and concurrency initially, we can simplify the mental model of this new change management. This will give the community a chance to adapt to this new paradigm and really see how it works.
- Build Concurrency: 1
- Merge Limits: 1
- Workflow: Compile and Check Java
Following this rollout, we can experiment with the merge group batch size and concurrency. Eventually, we may opt to run some or all tests as part of the merge queue. That remains out of scope of this KIP.
Rejected Alternatives
If there are alternative ways of accomplishing the same thing, what were they? The purpose of this section is to motivate why the design is the way it is and not some other wayThere aren't really any alternatives to using the merge queue. The current scheme is working fairly well, but has the shortcomings mentioned above. Enabling "require up-to-date" for PRs would be a huge burden on contributors and our CI infra, so it is not really worth considering.
