Conversation
82f9870 to
9035425
Compare
The write-pacing guard only started delaying writes once a single invocation's own batch reached the store's maxWritesPerSecond threshold, and sized the delay off that same per-invocation number. Parameter Store's write-rate limit is account-wide: several pools' scale-up/pool lambdas can each stay under the threshold individually while their combined writes exceed the account limit, and a batch below the threshold got no pacing at all. The guard now always paces once a write limit is configured, and divides the per-write delay by a new ssm_parameter_store_max_concurrent_invocations variable so the configured account-wide write budget is shared across the expected number of concurrent invocations instead of assumed available to each one independently. The write limit itself was also hardcoded to SSM's standard-tier default of 40/sec. It's now configurable via a new ssm_parameter_store_max_writes_per_second variable, so accounts that enabled SSM's higher-throughput tier (up to several thousand writes/second) can raise the pacing ceiling to match instead of being paced against a limit that no longer applies to them. Both variables are wired through for the scale-up and pool lambdas; defaults (1 concurrent invocation, 40 writes/sec) match prior behavior. Documented in docs/rate-limits-and-tuning.md alongside the existing SSM/batch_size guidance.
9035425 to
4ec6bcb
Compare
…rate-limit # Conflicts: # lambdas/functions/control-plane/src/scale-runners/github-runner.test.ts
|
@atsikham also implement the changes in https://github.com/github-aws-runners/terraform-aws-github-runner/tree/main/modules/orchestration-providers/webhook and https://github.com/github-aws-runners/terraform-aws-github-runner/tree/main/modules/multi-runner multi-runner is the module to deploy multi runners |
|
@edersonbrilhante digging into the wiring: in modules/runners the two new pacing settings are one flat value shared by scale-up and pool lambda. But multi-runner/orchestration-providers/webhook split lambda config per type with no shared bucket. Duplicate the field under both scale.up and pool, or is there a preferred place for settings shared across lambdas in a runner group? |
Fixes #5410
Summary
The code that slows down writes to Parameter Store (so they don't hit AWS's rate limit) had two problems:
maxWritesPerSecondnumber, and it based the delay only on that one invocation's batch size. But Parameter Store's write limit applies to the whole AWS account, not to one invocation: several pools can each run their own scale-up/pool Lambda at the same time, each one under its own threshold, while their writes added together go over the account's limit. And a batch smaller than the threshold got no slowdown at all.Changes
ssm_parameter_store_max_concurrent_invocations, sets how many scale-up/pool Lambda invocations can realistically be running at the same time across the whole deployment. The delay between writes is divided by that number, so the account-wide write budget is shared across all of them instead of each invocation assuming it has the whole budget to itself.ssm_parameter_store_max_writes_per_second, makes the actual write-rate limit configurable instead of it being fixed at 40. If an account turns on SSM's higher-throughput setting, this can be raised to match the real limit.1invocation,40writes/sec) match how the code behaved before.docs/rate-limits-and-tuning.mdis updated: the Parameter Store section now explains both new variables, and points out that turning on AWS's higher-throughput setting by itself does nothing —ssm_parameter_store_max_writes_per_secondalso needs raising, or the code keeps slowing writes down to the old default. Also fixed a claim in thebatch_sizetable that was no longer true (the slowdown no longer depends on batch size), and updated the advice for large deployments.One thing left out on purpose: this only connects the new variables in
modules/runners(the main scale-up + pool setup). The webhook-orchestration and multi-runner-v2 setups were not touched. This could be extended if wanted.Test plan
yarn test—github-runner.test.tschecks the delay math inaddDelay(including the small-batch case that used to get no delay, and the sharing-across-invocations behavior), plus an end-to-end test using fake timers throughcreateStartRunnerConfig.runner-config-store.test.tsandstorage-providers.test.tscheck that the write-rate limit can be set, falls back to 40 if the value is missing or invalid, and is picked up from the environment correctly.yarn lint/yarn formattsc --noEmitterraform fmt -check/terraform validate(tflint could not be installed in this environment — no network access to its Homebrew source — so the full set of pre-commit checks was not run locally)