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

Reduce number of auto-rebases when merging #2204

Closed
Jym77 opened this issue Jan 4, 2019 · 9 comments
Closed

Reduce number of auto-rebases when merging #2204

Jym77 opened this issue Jan 4, 2019 · 9 comments

Comments

@Jym77
Copy link

Jym77 commented Jan 4, 2019

So, one of my main issues with dependabot so far is when several automergeable updates on the same repo happen to conflict with each other, thus actually preventing automerging… I'm not sure whether I'm doing something wrong in the way I use dependabot and I'm not sure either how to solve this… So I'm open for suggestions :-D


The problem I have:

dependabot is set on weekly bumps. So, many PRs are started on Monday morning. This is fine. Of these, there are also several that are automerge according to my rules. Even better.
However, it happens quite often that 2 (or more) automerge updates result in conflicting commits. I assume that this is due to git merge strategy looking for context and if these deps are on adjacent lines in the requirement file, then context is broken and automerge fails.

This result in "automerge" updates that do not actually automerge. Sure, after rebasing, they will automerge but in the meantime they stay idle.

If the rebasing is done automatically, then each successful merge can potentially trigger rebasing of all other PRs thus creating up to a quadratic number of rebasing. This is just killing the CI tool and/or gets too expensive if you pay per build (in our case, we have a max concurrent builds number on our plan and auto rebasing on ~15 repos every Monday will just completely kill the CI tool for the whole day…)

If the rebasing is done manually (our current strategy to avoid CI bottleneck issue), this requires a linear amount of @dependabot rebase which is annoying and becomes even worse because you need to wait for the CI to finish between each (so that dependabot can automerge), making it a "do something every 15min" process which is just plain bad with regular development tasks.

Of course, all that gets worse and worse the more repos with dependabot you have.

What can be done about it?

And I'm not sure how to solve that and get dependabot into a manageable amount of work for us. Ideas are welcome. I have a few ideas myself, but maybe they're not that good…
Of course, this will not remove manual intervention on non-automerge updates, but we do know that these require human eyes, so it's OK.

1/ Group automerge updates
The topic of grouping updates has been discussed a lot. Grouping all automerge updates would solve my problem since there will only be one of these and thus no mutual conflict. Of course, grouping updates has its own problems (mostly how to handle cases where the bundle of updates is breaking something and how much bissecting should dependabot do to find the breaking one). And in case of troubles where dependabot tries to make smaller groups to merge as much as possible automatically, we get back to mutual conflict issues…

2/ Wait between PRs
There is some discussion about waiting before issuing/merging PR (to check that the upstream update is actually OK-ish). This would be a bit different. The idea here is that instead of spawning all PRs at exactly the same time, dependabot could start them with a 10min interval or something. If the interval is reasonably larger than the normal CI build time (hence, need to be configurable per repo), that would give enough time for the previous automerge PR to build and merge and thus avoid conflicts and rebases.
We could also imagine that when bumping deps, first automerge PRs are created with some wait between them and next all non-automerge PRs are created at the same time and wait for human intervention.

3/ Different update schedules
An easy way to avoid simultaneous bumps is to set a live update schedule. However, this is generating quite some amount of spam for libs that update very often (aws-sdk is my main culprit here). Plus weekly/monthly bumps have a nice feeling of grouping "dependabot time" together and knowing that this is something I'll do at that point rather than getting interupted by new bumps all the time.
Things are a bit different for automerge updates which do not require human intervention (unless they break something). So, it could be nice to have live updates for automerge updates and weekly updates for the rest. As far as I can tell, this is not currently doable. Not even with the config file, and not even on a per-dep basis (ie "live update of pytest, weekly of the rest" rather than "live update of anything you can automerge", there could still be updates of pytest I do not want to automerge (eg, major updates) but a per-dep schedule would already help in most cases).


So, I don't know if other are experiencing the same issue and what is your way to handle that. And I'm not sure either which way to deal with it would be the best… However, given the number of repos we are currently handling in a rather small team, most of them only in maintenance mode, we do spend a substantial amount of time on dependabot and we're starting to find that rather annoying…

@feelepxyz
Copy link
Contributor

@Jym77 thanks for putting this together! Just been discussing this with @greysteil and thought of another way of doing this.

4/ Sequential automatic rebasing
We could add a setting to limit number of active rebases, so you could set this to one to get sequential rebasing. The strategy would be to start with the automerge-able ones that where passing before, jumping over the ones that fail.

This should considerably reduce the number of CI runs if you have multiple open PRs.

Would this work for you?

@Jym77
Copy link
Author

Jym77 commented Jan 4, 2019

Yes, this is essentially the same idea as my 2/ but being a bit more clever ("one PR at a time" rather than "wait hopefully long enough before next one").

I feel there might be some gain in handling differently automerge bumps and others. Which probably requires a more granular update_schedule than the current one (which is at package_manager level only, afaict). So that might be too much changes…

@feelepxyz
Copy link
Contributor

@Jym77 yes nice one! Think we'll start with just doing sequential auto rebases before potentially doing more to reduce the initial batch of PRs being created. Should be somewhat alleviated by running the batch at night.

There's definitely stuff we could do to reduce the initial number of PRs created. Like you suggested, one way would be to create them sequentially waiting for for CI to pass/fail and starting with automerge-able ones.

@feelepxyz
Copy link
Contributor

Feedback from dependabot/feedback#367

We often see an issue where merging a branch, causes all pending Dependabot PRs to automatically get rebased. For us, this causes swarms of running jobs in CircleCI. It would be nice if those rebases were sent sequentially.

For example, given 5 pending dependabot PRs, the current implementation results in 5+4+3+2+1 = 15 builds. If they are sent sequentially (and auto-merge is turned on) then it would be 5+5 = 10 builds. For 10 pending PRs, it would be 55 builds vs 20.

For this reason, we currently have auto-rebase turned off and would like to turn it back on.

@feelepxyz
Copy link
Contributor

Feedback from dependabot/feedback#368

Add option to only auto-rebase approved PRs - should help prevent swarms of CircleCI builds.

@feelepxyz feelepxyz changed the title Handling of mutually conflicting automerged updates Reduce number of auto-rebases when merging Mar 7, 2019
@WaldoJeffers
Copy link

Add option to only auto-rebase approved PRs - should help prevent swarms of CircleCI builds.

I think this is a quite clever idea :)

@stale
Copy link

stale bot commented Oct 23, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs within seven days. Thank you for your contributions.

@simlu
Copy link
Contributor

simlu commented Oct 23, 2019

Still very much applicable. Hopefully we'll get multiple dependency updates in single pr soon!

@infin8x infin8x transferred this issue from dependabot/feedback Jun 29, 2020
@infin8x
Copy link
Contributor

infin8x commented Jul 2, 2020

Duplicate of #1190

@infin8x infin8x marked this as a duplicate of #1190 Jul 2, 2020
@infin8x infin8x closed this as completed Jul 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants