Skip to content

gha: add guardrails timeouts on all jobs#48629

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:gha_timeout
Oct 10, 2024
Merged

gha: add guardrails timeouts on all jobs#48629
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:gha_timeout

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Oct 10, 2024

We had a few "runaway jobs" recently, where the job got stuck, and kept running for 6 hours (in one case even 24 hours, probably due some github outage). Some of those jobs could not be terminated.

While running these actions on public repositories doesn't cost us, it's still not desirable to have jobs running for that long (as they can still hold up the queue).

This patch adds a blanket "2 hours" time-limit to all jobs that didn't have a limit set. We should look at tweaking those limits to actually expected duration, but having a default at least is a start.

Also changed the position of some existing timeouts so that we have a consistent order in which it's set; making it easier to spot locations where no limit is defined.

We had a few "runaway jobs" recently, where the job got stuck, and kept
running for 6 hours (in one case even 24 hours, probably due some github
outage). Some of those jobs could not be terminated.

While running these actions on public repositories doesn't cost us, it's
still not desirable to have jobs running for that long (as they can still
hold up the queue).

This patch adds a blanket "2 hours" time-limit to all jobs that didn't
have a limit set. We should look at tweaking those limits to actually
expected duration, but having a default at least is a start.

Also changed the position of some existing timeouts so that we have a
consistent order in which it's set; making it easier to spot locations
where no limit is defined.

Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah added status/2-code-review kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. area/testing process/cherry-pick/27.x labels Oct 10, 2024
@thaJeztah thaJeztah added this to the 28.0.0 milestone Oct 10, 2024
@thaJeztah thaJeztah self-assigned this Oct 10, 2024
@thaJeztah thaJeztah requested a review from crazy-max October 10, 2024 11:45
@thaJeztah
Copy link
Copy Markdown
Member Author

Wondering if there's a linter for this (similar for the "should have a top-level default permissions"); do you know, @crazy-max ?

permissions:
contents: read


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional double blank line? 👀

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh! very intentionally not intentional.

Oh! I know what happened; I first tried if it would accept a top-level timeout-minutes (similar to permissions), because that would make the most logical place to put a default.

But unfortunately it doesn't, so then I picked "after every runs-on as a conventions" (closest to "top level").

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/testing kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. process/cherry-picked status/2-code-review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants