Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add experimental batch support #84

Merged
merged 19 commits into from
Jun 6, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 68 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ optional arguments:
[env var: MARGE_BRANCH_REGEXP] (default: .*)
--debug Debug logging (includes all HTTP requests etc).
[env var: MARGE_DEBUG] (default: False)
--batch Enable processing MRs in batches.
[env var: MARGE_BATCH] (default: False)
```
Here is a config file example
```yaml
Expand All @@ -117,6 +119,7 @@ auth-token-file: token.FILE
branch-regexp: .*
ci-timeout: 15min
embargo: Friday 1pm - Monday 9am
batch: false
git-timeout: 120s
gitlab-url: "https://gitlab.example.com"
impersonate-approvers: true
Expand Down Expand Up @@ -178,7 +181,7 @@ docker run \
--gitlab-url='http://your.gitlab.instance.com'
```

For completeness sake, here's how we run marge-bot at smarkets ourselves:
For completeness sake, here's how we run marge-bot at Smarkets ourselves:
```bash
docker run \
-e MARGE_AUTH_TOKEN="$(cat marge-bot.token)" \
Expand Down Expand Up @@ -206,7 +209,7 @@ Once running, the bot will continuously monitor all projects that have its user
will pick up any changes in membership at runtime.


## Suggested worfklow
## Suggested workflow
1. Alice creates a new merge request and assigns Bob and Charlie as reviewers

2. Both review the code and after all issues they raise are resolved by Alice,
Expand All @@ -229,7 +232,7 @@ two [standardized git commit trailers](https://www.kernel.org/doc/html/v4.11/pro
the name of the `marge-bot` user in GitLab).

If you pass `--add-reviewers` and the list of approvers is non-empty and you
have enough approvers to meet the required approver count, Marge will add a the
have enough approvers to meet the required approver count, Marge will add the
following header to each commit message and each reviewer as it rebases the
target branch into your PR branch:

Expand Down Expand Up @@ -281,6 +284,66 @@ More than one embargo period can be specified, separated by commas. Any merge
request assigned to her during an embargo period, will be merged in only once all
embargoes are over.


## Batching Merge Requests

The flag --batch enables testing and merging merge requests in batches. This can
significantly speed up the rate at which marge-bot processes jobs - not just
because merge requests can be tested together, but because marge-bot will ensure
the whole set of merge requests is mergeable first. This includes, for example,
checking if a merge request is marked as WIP, or does not have enough approvals.
Essentially, users get faster feedback if there is an issue.

This is currently an experimental feature.

### How it works

If marge-bot finds multiple merge requests to deal with, she attempts to create
a batch job. She filters the merge requests such that they have all have a
common target branch, and eliminates those that have not yet passed CI (a
heuristic to help guarantee the batch will pass CI later).

Once the merge requests have been gathered, a batch branch is created using the
commits from each merge request in sequence. Any merge request that cannot be
merged to this branch (e.g. due to a rebase conflict) is filtered out. A new
merge request is then created for this branch, and tested in CI.

If CI passes, the original merge requests will be merged one by one.

If the batch job fails for any reason, we fall back to merging the first merge
request, before attempting a new batch job.

### Limitations

* Batch mode is incompatible with the tested-by trailer, as trailers are only
added to the original merge requests. This means the tested-by trailer would
be added to each merge request's last commit, as opposed to the last commit of
the last merge request in the batch, which is the only that should be
considered tested.
* As trailers are added to the original merge requests only, their branches
would need to be pushed to in order to reflect this change. This would trigger
CI in each of the branches again that would have to be passed before merging,
which effectively defeats the point of batching. To workaround this, the
current implementation merges to the target branch through git, instead of the
GitLab API. GitLab will detect the merge request as having been merged, and
update the merge request status accordingly, regardless of whether it has
passed CI. This does still mean the triggered CI jobs will be running even
though the merge requests are merged. marge-bot will attempt to cancel these
pipelines, although this doesn't work too effectively if external CI is used.
* There is what can be considered to be a flaw in this implementation that could
potentially result in a non-green master; consider the following situation:

1. A batch merge request is created, and passes CI.
2. Several merge requests are then merged to master, but one could fail
(perhaps due to someone pushing directly to master in between).
3. At this point, marge-bot will abort the batch job, resulting in a subset of
the batch merge requests having been merged.

We've guaranteed that individually, each of these merge requests pass CI, and
together with some extra merge requests they also pass CI, but this does not
guarantee that the subset will. However, this would only happen in a rather
convoluted situation that can be considered to be very rare.

## Restricting the list of projects marge-bot considers

By default marge-bot will work on all projects that she is a member of.
Expand All @@ -298,7 +361,7 @@ look-ahead and will match any string not starting with `project/with_reviewers`.
## Restricting the list of branches marge-bot considers

It is also possible to restrict the branches marge-bot watches for incoming
merge requests. By default, marge-bot will process MRs targetted for any branch.
merge requests. By default, marge-bot will process MRs targeted for any branch.
You may specify a regexp that target branches must match with `--branch-regexp`.

This could be useful, if for instance, you wanted to set a regular freeze
Expand All @@ -324,7 +387,7 @@ git config --global alias.revert-mr '!f() { REVS=$(git mr-revs "$1"); URL="$(git
```

E.g. `git revert-mr 123`. This will create a single commit reverting all commits
that are part of MR 123 with a a commit message that looks like this:
that are part of MR 123 with a commit message that looks like this:

```
Revert <http://gitlab.example.com/mygropup/myproject/merge_requests/123>
Expand Down
17 changes: 13 additions & 4 deletions marge/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,20 +99,25 @@ def regexp(str_regex):
metavar='INTERVAL[,..]',
help='Time(s) during which no merging is to take place, e.g. "Friday 1pm - Monday 9am".\n',
)
merge_group = parser.add_mutually_exclusive_group(required=False)
merge_group.add_argument(
experimental_group = parser.add_mutually_exclusive_group(required=False)
experimental_group.add_argument(
'--use-merge-strategy',
action='store_true',
help=(
'Use git merge instead of git rebase (EXPERIMENTAL)\n'
'Enable if you use a workflow based on merge-commits and not linear history.\n'
),
)
merge_group.add_argument(
experimental_group.add_argument(
'--add-tested',
action='store_true',
help='Add "Tested: marge-bot <$MR_URL>" for the final commit on branch after it passed CI.\n',
)
experimental_group.add_argument(
'--batch',
action='store_true',
help='Enable processing MRs in batches (EXPERIMENTAL)\n',
)
parser.add_argument(
'--add-part-of',
action='store_true',
Expand Down Expand Up @@ -223,6 +228,9 @@ def main(args=None):
)
options.ci_timeout = timedelta(minutes=options.max_ci_time_in_minutes)

if options.batch:
logging.warning('Experimental batch mode enabled')

config = bot.BotConfig(
user=user,
ssh_key_file=ssh_key_file,
Expand All @@ -238,7 +246,8 @@ def main(args=None):
embargo=options.embargo,
ci_timeout=options.ci_timeout,
use_merge_strategy=options.use_merge_strategy,
)
),
batch=options.batch,
)

marge_bot = bot.Bot(api=api, config=config)
Expand Down
Loading