Make EphemeralRunnerController MaxConcurrentReconciles configurable #3832
+78
−7
Conversation
This file contains 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
mumoshu
requested review from
toast-gear,
rentziass and
a team
as code owners
mumoshu
commented
Nov 30, 2024
| // This updates the option value only if the environment variable is set. | ||
| // If the option is already set (via a command-line flag), the value from the environment variable takes precedence. | ||
| func (o *Options) LoadEnv() error { | ||
| v, err := o.getEnvInt("RUNNER_MAX_CONCURRENT_RECONCILES") |
There was a problem hiding this comment.
I added this env just to make it easy to give it a shot without modifying the chart.
Instead, we can modify the manager args template there
There was a problem hiding this comment.
@mumoshu - I recommend we go with modifying the chart and adding this as an extra element under flags:
mumoshu
commented
Nov 30, 2024
| // rather than having to correlate those in multiple places. | ||
| func OptionsWithDefault() Options { | ||
| return Options{ | ||
| RunnerMaxConcuncurrentReconciles: 2, |
There was a problem hiding this comment.
Making 2 the new default so that users can benefit from the update without any config changes.
Formerly it was treated 1 as not specified, and having 2 shouldn't be harmful as you should be able to limit CPU req/lim at the K8s level anyway.
Link-
approved these changes
Dec 11, 2024
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Improve the runner pod startup times by using more goroutines(almost "more cpus", assuming you are NOT K8s API or network bounded) for ephemeralrunner reconciliation.
Today,
cotroller-runtimeand hence ARC reconciles EphemeralRunnerPod one by one, which should be limiting the runner pod creation throughput to almostunit time / latency of the GHA jit config API.If you are not bounded by CPU, K8s, network, or GHA-API yet, setting N MaxConcurrentReconciles should improve the throughput by N.
This is the PR for the second commit mentioned in #3276 (comment).