Repository navigation
Conversation
Serial JIT minting in createJitConfig costs about 1.3 s per runner, so a 190-runner pool fill takes roughly 275 s and steady-state pool ticks slip past the 60 s schedule. Mint JIT configs with a bounded worker pool instead: default concurrency 8, override with JIT_CONFIG_CONCURRENCY (positive integer; anything else falls back to 8) or the new `jitConfigConcurrency` option on StartRunnerConfigOptions. Each worker's write-throttle sleep is scaled by the concurrency so the aggregate write rate still honours the config store's maxWritesPerSecond. Failures are collected per runner as before; one failed mint never aborts the batch.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unvalidated concurrency values can silently skip all runner configuration writes while reporting success, and high concurrency can exceed SSM’s write-rate limit.
3 open issues
What changed in this PR
This PR speeds up runner-pool fills by replacing serial JIT configuration minting with a bounded worker pool in the control plane.
Changes:
- Adds configurable minting concurrency, defaulting to 8, through an environment variable or per-call option.
- Scales per-worker storage delays and retains per-runner failure collection.
- Adds tests for concurrency bounds, failure handling, and configuration fallback.
| File | Description |
|---|---|
lambdas/functions/control-plane/src/scale-runners/github-runner.ts |
Adds concurrency configuration and parallel JIT minting with scaled delays. |
lambdas/functions/control-plane/src/scale-runners/github-runner.test.ts |
Tests bounded concurrency, partial failures, and resolver fallback behavior. |
Review effort: Balanced
Comment on lines
+238
to
+239
| const jitConfigConcurrency = | ||
| options.jitConfigConcurrency ?? resolveJitConfigConcurrency(process.env.JIT_CONFIG_CONCURRENCY); |
Comment on lines
+40
to
+41
| const n = Number.parseInt(value ?? '', 10); | ||
| return Number.isInteger(n) && n > 0 ? n : fallback; |
Comment on lines
+313
to
+314
| // Each worker sleeps `concurrency` times longer so the aggregate write rate stays within maxWritesPerSecond. | ||
| const workerDelayMilliseconds = delayMilliseconds * concurrency; |
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
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
In our production deployment, filling a large pool is dominated by JIT config minting.
createJitConfigmints one config per runner serially, and each mint (GitHub API call plus config-store write) takes about 1.3 s, so a 190-runner fill takes roughly 275 s. Steady-state pool ticks on a 60 s schedule also slip past the schedule whenever a top-up is more than a few dozen runners, because CreateFleet returns quickly and the rest of the invocation is spent minting.This PR mints JIT configs with a bounded worker pool instead of a
forloop:JIT_CONFIG_CONCURRENCYenvironment variable (positive integer; anything else falls back to 8) or the newjitConfigConcurrencyoption onStartRunnerConfigOptions. Setting it to1restores the previous serial behaviour.maxWritesPerSecondis still honoured: each worker's throttle sleep is multiplied by the concurrency, so the aggregate write rate across workers stays at the store's limit. The existing "storage write rate limit" tests pass unchanged and act as the guard.With the default of 8, the same 190-runner fill should take about 36 s (the scaled throttle sleep adds a little once a batch exceeds 40 runners). No Terraform variable is added in this PR to keep it small; if maintainers would rather expose this as a module variable than an environment variable, I am happy to add one here or in a follow-up.
Test Plan
lambdas/functions/control-plane/src/scale-runners/github-runner.test.ts, undercreateJitConfig concurrency:mints with bounded concurrencycollects failures without stopping the batchfalls back to the default concurrency(forresolveJitConfigConcurrency)calls create start runner config of 40 instances (storage write rate limit) to test time delay, including the GHES and data-residency variants) pass unchanged, confirming the aggregate write rate is still capped.yarn install --immutable && yarn format && yarn lint && yarn test && yarn buildinlambdas/on this branch (rebased onmain).Related Issues
maxWritesPerSecond; it does not change how that limit is derived, so it should compose with whatever that issue lands.