search

smarkets / marge-bot

A merge-bot for GitLab
BSD 3-Clause "New" or "Revised" License
700 stars 136 forks source link

Recognise 'Branch cannot be merged' error and retry later #292

Open nirizr opened 3 years ago

nirizr commented 3 years ago

I've seen the 'Branch cannot be merged' several times due to a race condition between margebot and pipelines been rerun.

If this occurs, IMHO margebot shouldn't necessarily give up immediately but instead retry again which is achieved by keeping marge bot the assignee.

WDYT?

snim2 commented 3 years ago

Maybe! Another option might be to call this function which implements a simple linear backoff: https://github.com/smarkets/marge-bot/pull/265/files -- BTW do you know why this error occurs?

nirizr commented 3 years ago

Thanks for your reply!

I was actually working on an outdated version (just recently rebased on latest version) when I originally made this commit internally a while ago. The commit you pointed out might be making this PR redundant.

When I investigated it in the past, it was happening because while marge was trying to merge, another MR was merged in manually by a user, which required the MR marge was merging to be rebased / updated (per our gitlab configuration) before it could be merged.

If you expect your change to fix this issue I can report back if I ever get this issue again (now with v0.9.5). Please lmk and I'll revert this change internally and close this PR.

snim2 commented 3 years ago

That's interesting @nirizr - I'm not sure whether my PR would fix this or not. My changes assume that the MR is good to merge, but that the GitLab API endpoint isn't yet returning the correct result. In your situation, you need Marge to realise that the MR now cannot be merged, and then rebase again.

I'd be interested to find out whether you still experience this bug with version 0.9.5, but my suspicion is that you will.

nirizr commented 3 years ago

If you don't believe your fix applies here. What could be the risk of merging it in? Would you prefer waiting until I can confirm its still helpful?

I haven't seen that behavior with v0.9.5 but we're more frequently using Marge now (compared to manual merging we used to do a lot when I just brought Marge in).

snim2 commented 3 years ago

Hi @nirizr I'm not a maintainer of this project, so I can't help with merging your change. I think it would be useful to confirm that the bug still occurs in v0.9.5, but that's just my opinion!

nirizr commented 3 years ago

Yea, I was asking for your opinion :)

snim2 commented 3 years ago

Ha! Well, personally I'm always cautious about merging, but maybe that's just me ;)

qqshfox commented 3 years ago

Hi, wondering if this is still needed for v0.9.5+?

nirizr commented 3 years ago

Hi @qqshfox, thanks for taking the time!

I believe I've encountered the error still with latest marge version.

Keep in mind, though, that company I work for was recently bought so we no longer use Marge bot.

qqshfox commented 3 years ago

Interesting. We've been using v0.9.5 for 2+ months at Smarkets, and not seen this again.

I'll leave this open in case anyone encounters this issue with the latest version again.

nirizr commented 3 years ago

We encountered it because users were also merging (and pushing to master, btw) manually. Do you do that at Smarkets? Either way waiting for confirmation sounds good.

LMK if there's anything else I can maybe help with.

qqshfox commented 3 years ago

No. We don't allow pushing to master.

But wait, is that the expected behavior to give up merging when there are new commits in master?

nirizr commented 3 years ago

Our gitlab configuration was such that only fast forward merges were allowed (so users would often rebase on top of master just before merging, to run our pipeline on the most up to date code) into master.

From memory, what would happen was this:

  1. A merge was permitted (because pipelines passed and merge was a FF merge).
  2. Marge bot would pull all MRs in the project and cycle through until it reaches MR.
  3. User would than manually merge (i.e click the "merge MR" button on gitlab) or push some changes to master (because it was allowed in certain cases).
  4. Marge bot would start the merging sequence / flow.
  5. Merge bot would issue the merge MR API request to gitlab.
  6. Gitlab would fail because MR is no longer permitted (it now requires a rebase to be FFed).