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

Unnecessary branch updates #761

Open
sindrig opened this issue Nov 24, 2021 · 7 comments
Open

Unnecessary branch updates #761

sindrig opened this issue Nov 24, 2021 · 7 comments
Labels
bug Something isn't working

Comments

@sindrig
Copy link

sindrig commented Nov 24, 2021

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.

  • Reading through the source code and recent changes I'm not seeing any obvious candidates that might be causing this.
  • We don't know when this started, could have been some time ago.
  • The only configs we're changing from the defaults are merge.message.title and merge.priority_merge_label.

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

We can provide more examples or information as needed.

@sindrig sindrig added the bug Something isn't working label Nov 24, 2021
@chdsbd
Copy link
Owner

chdsbd commented Nov 24, 2021

Hey @sindrig, thanks for the report.

This is weird behavior. I'm not sure why for example this PR is marked as missing reviews: island-is/island.is#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
Copy link
Owner

chdsbd commented Nov 24, 2021

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

@sindrig
Copy link
Author

sindrig commented Nov 24, 2021

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
Copy link
Owner

chdsbd commented Nov 24, 2021

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

Kodiak starts repo worker tasks with this code:

kodiak/bot/kodiak/queue.py

Lines 519 to 525 in 0c6cda5

def start_repo_worker(self, *, queue_name: str) -> None:
self._start_worker(
queue_name,
"repo",
repo_queue_consumer(queue_name=queue_name, connection=self.connection),
)

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

kodiakhq bot pushed a commit that referenced this issue Nov 25, 2021
We can use reviewDecision to hopefully provide a more consistent view of reviews for a pull request.

My theory for some of the issues in #761 is that the GitHub API is being inconsistent when called via the v3 API vs the GraphQL API. Using only reviewDecision will help us avoid depending on the v3 API.

I've deployed this change
@sindrig
Copy link
Author

sindrig commented Nov 25, 2021

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

island-is/island.is#5733

island-is/island.is@ecefbbf
island-is/island.is@a7c236b
island-is/island.is@d647687

@chdsbd
Copy link
Owner

chdsbd commented Nov 25, 2021

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 e60988d, where if changes are requested, the bot doesn't update the branch correctly.

I think this can cause some unnecessary updates.

kodiakhq bot pushed a commit that referenced this issue Nov 25, 2021
A regression was introduced with #762 that caused the bot to attempt to update pull requests when changes were requested. This change fixes that issue.

related #761
@chdsbd
Copy link
Owner

chdsbd commented Nov 25, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants