fix(pool): count registered offline runners within boot time - #5487
Open
shinharaguchi wants to merge 2 commits into
Open
shinharaguchi wants to merge 2 commits into
shinharaguchi wants to merge 2 commits into
Conversation
With JIT config the runner is registered in GitHub before the agent on the instance connects, so it can be reported as offline while it is still booting. The pool counted a runner as booting only when it was not registered yet, so such a runner was excluded without checking its boot time, and a frequently running pool could launch extra instances. Count a registered runner that is offline and not busy while it is within its boot time. Offline runners that are busy, have no busy value, or whose boot time has expired are still excluded. Related github-aws-runners#3799
Move the check for registered offline runners that are not busy into its own branch instead of excluding them from the "not idle" branch with a negated flag. Behaviour is unchanged. Drop the test case for a runner status without a busy value, since RunnerStatus.busy is required and the case could only be built with a type cast.
shinharaguchi
changed the title
Contributor
|
@shinharaguchi we need your commits to be signed, so it can be merged |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A registered runner that is offline and not busy is now counted by the pool while it is within
RUNNER_BOOT_TIME_IN_MINUTES. Today the pool skips the boot time check for any registered runner that is not online and idle, so a frequently running pool can launch extra instances while the first runner is still starting.Root cause
With JIT config, the runner is registered in GitHub right after the EC2 instance is launched, before its agent connects. The example response of the JIT configuration endpoint and the observation in #3799 show such a runner as
{ busy: false, status: "offline" }.countAvailableEc2PoolRunnerstreats a runner as booting only when it is not in the GitHub status map. A registered offline runner falls into the "not idle in GitHub and NOT counted" branch instead.Change
Registered offline runners that are not busy get their own branch: counted while within the boot time, not counted after it. Online idle runners, busy runners and unregistered runners keep their existing handling. The check is based on runner status, so it also applies to non-JIT runners reported as offline within their boot time.
Logs
Pool size one, running every minute (instance IDs anonymised):
Test Plan
counts registered offline runners that are still booting. It fails onmainand passes with this change.does not count registered busy or offline runnersinto a busy case and an expired offline case, and added an offline busy case.yarn format,yarn lint,yarn testandyarn buildinlambdas/. All passed.Related Issues
Related #3799
Related #3809