chdsbd / kodiak

🔮 A bot to automatically update and merge GitHub PRs
https://kodiakhq.com
GNU Affero General Public License v3.0
1.03k stars 65 forks source link

Unnecessary branch updates #761

Open sindrig opened 2 years ago

sindrig commented 2 years ago

Recently we've discovered that kodiak is updating all branches that have automerge set, regardless of their place in the queue. The status that's set on the commits is "⛴ merging PR (updating branch)". This causes quite a lot of extra CI jobs, especially when we've got long queues, leading to way longer merge times.

Here are some random 3 pull requests: https://github.com/island-is/island.is/pull/5749 https://github.com/island-is/island.is/pull/5742 https://github.com/island-is/island.is/pull/5721

We can provide more examples or information as needed.

chdsbd commented 2 years ago

Hey @sindrig, thanks for the report.

This is weird behavior. I'm not sure why for example this PR is marked as missing reviews: https://github.com/island-is/island.is/pull/5717 It seems like it was merging but then Kodiak said it was missing reviews

Also, looking in the logs, it seems like there were two Kodiak repo workers running at the same time merging 5750 and 5742

https://github.com/chdsbd/kodiak/blob/0c6cda5169e2f311ed654877b11a15feba65ad05/bot/kodiak/queue.py#L430:11

chdsbd commented 2 years ago

I'm going to try eliminating those incorrect "missing required reviews" status messages in #762

sindrig commented 2 years ago

Thanks for the quick response @chdsbd.

I did suspect some kind of race condition or error due to parallelism but didn't have anything to go by. When you say two Kodiak repo workers running, you mean there were two runners running in your systems, right? Is there anything we could have spotted?

I'll try to keep an eye on pull requests once developers start pushing changes tomorrow and update here if we see this happening still.

chdsbd commented 2 years ago

Yeah, I meant two internal workers in Kodiak (actually asyncio tasks) processing the same repo.

Kodiak starts repo worker tasks with this code: https://github.com/chdsbd/kodiak/blob/0c6cda5169e2f311ed654877b11a15feba65ad05/bot/kodiak/queue.py#L519-L525

I'm honestly not sure yet how it's possible to create two duplicate tasks. 😬

sindrig commented 2 years ago

Pretty sure this is still happening. See this one for example:

https://github.com/island-is/island.is/pull/5733

https://github.com/island-is/island.is/pull/5733/commits/ecefbbfe0c4beefc92ed2c0c56bfdb2319815cdd https://github.com/island-is/island.is/pull/5733/commits/a7c236b1e701ed51d28dfb86b714dc49cfa3ea87 https://github.com/island-is/island.is/pull/5733/commits/d647687473325a4e4e37d7649ad48113c02265e5

chdsbd commented 2 years ago

Looking in the logs, it seems like the issue of having two workers at the same time isn't happening anymore, but, it seems like I introduced a small bug via https://github.com/chdsbd/kodiak/commit/e60988dfc472a30d5f1d745aac81df6cd673f73b, where if changes are requested, the bot doesn't update the branch correctly.

I think this can cause some unnecessary updates.

chdsbd commented 2 years ago

I've deployed #765 which should help with some of the unnecessary updates when changes are requested