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

Build Cop: duplicate issues for same test failure? #283

Closed
shollyman opened this issue Feb 14, 2020 · 5 comments · Fixed by #291 or #297
Closed

Build Cop: duplicate issues for same test failure? #283

shollyman opened this issue Feb 14, 2020 · 5 comments · Fixed by #291 or #297
Assignees
Labels
bot: flakybot priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@tbpg tbpg self-assigned this Feb 14, 2020
@tbpg tbpg added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Feb 14, 2020
@tbpg
Copy link
Contributor

tbpg commented Feb 14, 2020

My first thought was that this was a race condition when parallel runs finish at the same time. But, it looks like some of those issues were filed days apart.

GoogleCloudPlatform/python-docs-samples#2892 and GoogleCloudPlatform/python-docs-samples#2891 look like a race condition.

I'm not sure why the other issues aren't being found (or, only found sometimes).

@tbpg
Copy link
Contributor

tbpg commented Feb 16, 2020

The problem is pagination. There are > 32 Build Cop issues in python-docs-samples: https://github.com/GoogleCloudPlatform/python-docs-samples/issues?utf8=%E2%9C%93&q=is%3Aissue+label%3A%22buildcop%3Aissue%22

But, we currently only get the first 32 issues. So, it's just luck with whether or not the already-existing issue is returned/found.

const issues = (
await context.github.issues.listForRepo({
owner,
repo,
per_page: 32,
labels: ISSUE_LABEL,
state: 'all', // Include open and closed issues.
})
).data;

Pagination docs are here: https://octokit.github.io/rest.js/#pagination

I'll send a PR.

@tbpg
Copy link
Contributor

tbpg commented Feb 16, 2020

Part of the fix needs to include closing duplicate issues, maybe preferring to keep an issue someone other than the bot has commented on.

@tbpg
Copy link
Contributor

tbpg commented Feb 18, 2020

For some reason, we're getting many, many duplicate comments on issues (sorry!)

Example: GoogleCloudPlatform/python-docs-samples#2849

Looking.

@tbpg
Copy link
Contributor

tbpg commented Feb 18, 2020

All of the duplicate comments are because there are many, many sponge_log.xml files that get sent to the bot.

The bot then has a different instance for each one of the logs. They are all running at the same time, they all try to close the duplicate issues, then they all comment at the same time.

I can't think of a way to prevent this race condition within the bot itself. So, one idea is to add some sort of external locking mechanism.

We could limit concurrency, but the same Cloud Function is used for every repo, so that would just slow things down.

@tbpg tbpg closed this as completed in #297 Feb 19, 2020
tbpg added a commit that referenced this issue Feb 19, 2020
Otherwise, if there are 50 invocations of the bot at the same time, they
will all try to close the duplicate issues. This isn't perfect, but will
be way better than 50.

Fixes #283
yoshi-automation added a commit that referenced this issue Apr 1, 2020
d70933a
commit d70933a
Author: Tyler Bui-Palsulich <[email protected]>
Date:   Tue Feb 18 09:21:32 2020 -0500

    feat(Build Cop): handle issue pagination and duplicates (#291)

    Fixes #283.
yoshi-automation added a commit that referenced this issue Apr 1, 2020
9e953ad
commit 9e953ad
Author: Tyler Bui-Palsulich <[email protected]>
Date:   Wed Feb 19 13:09:53 2020 -0500

    fix(Build Cop): only close duplicates when the test was run (#297)

    Otherwise, if there are 50 invocations of the bot at the same time, they
    will all try to close the duplicate issues. This isn't perfect, but will
    be way better than 50.

    Fixes #283
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: flakybot priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
2 participants