feat: event-driven runner count cache to reduce DescribeInstances - #5281
Open
yalafe wants to merge 8 commits into
Open
feat: event-driven runner count cache to reduce DescribeInstances#5281yalafe wants to merge 8 commits into
yalafe wants to merge 8 commits into
Conversation
…o upstream main Base for reworking the count-cache feature on current upstream (plugin layout). Not yet rebased/genericized. cache.ts here is github-aws-runners#4983's version and overwrites main's existing cache.ts - to reconcile next.
…r-count caches into runner-count-cache.ts - Restore upstream githubCache in cache.ts (was clobbered by github-aws-runners#4983) - Move ec2RunnerCountCache + dynamoDbRunnerCountCache to runner-count-cache.ts - Drop duplicated githubCache/GhRunners/Octokit import - 21/21 count-cache tests pass
…nsactWriteItems) - Guard each +1/-1 with a per-instance marker (COUNTED/TERMINATED) in an atomic TransactWriteItems, so at-least-once/out-of-order EventBridge delivery cannot double-count. Replaces the 'count only running' workaround; counts on first active event without missing pending->terminated. - Marker guard prevents negative drift by construction (a -1 only applies when the matching +1 was recorded) -> separate write-floor unnecessary; read clamp remains. - Add lambda.test.ts (11 tests) covering dup/out-of-order/underflow/filter/rethrow; github-aws-runners#4983 shipped no test for this lambda.
…e target - retry_policy (1h max age, 10 attempts) + dead_letter_config on the target - SQS DLQ (14d retention, SSE) + scoped queue policy for events.amazonaws.com - CloudWatch alarm on DLQ depth > 0 so silent delivery loss is observable Closes the retry-exhaustion silent-loss gap github-aws-runners#4983 had no handling for.
- withRunnerCountCache wraps any ScaleUpComputeProvider (contract-level, provider- agnostic): tiered getCurrentRunners in-memory -> DDB counter (if fresh) -> delegate to the provider's own count on miss/stale; reset in-memory after create. - Applied at provider composition in scale-up.ts; scale-up stays provider-agnostic. - Transparent pass-through when RUNNER_COUNT_CACHE_TABLE_NAME is unset (opt-in). - 5 decorator tests + 113 existing scale-up tests pass; tsc clean.
- modules/runners/scale-up.tf: RUNNER_COUNT_CACHE_TABLE_NAME + STALE_THRESHOLD_MS env
vars on the scale-up lambda, and a count-gated dynamodb:GetItem policy on the table
- modules/runners/variables.tf + variables.runner-count-cache.tf: runner_count_cache vars
- main.tf: instantiate module.runner_count_cache (enable-gated) and pass
{table_name, stale_threshold_ms} into the runners module
Feature stays fully opt-in (runner_count_cache.enable=false by default; decorator is
pass-through when the table env var is empty).
- yarn install registers the workspace package (functions/*) in the lockfile so 'yarn dist' builds the ncc bundle + zip - fix lambda.tf zip path to the function root (matches the dist script output, consistent with the other functions) Build verified: runner-count-cache.zip produced; 11 lambda tests pass.
yalafe
changed the title
Contributor
|
I liked the idea, but this should be added in compute provider ec2. We want to keep the controle plane neutral provider, so we can add new compute provider. |
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
Reduces the
DescribeInstancesload in the scale-up path (#4710) with an opt-in, event-driven runner count cache. Builds on the approach in #4983 by s1v4-d@, reworked onto currentmain(plugin layout) with a few additions. Opt-in viarunner_count_cache = { enable = true }; default is unchanged behaviour.Approach :
Why: scale-up currently counts active + pending runners via
DescribeInstanceson every decision, which gets slow and rate-limited at scale. We built a fix, then found it lines up with #4710 (npalm@) and #4983 (s1v4-d@) - so bringing ithere rather than maintaining it separately.
What: replace that per-decision call with a DynamoDB counter kept up to date by an EventBridge EC2 state-change Lambda, a short in-memory TTL cache in scale-up, and a
DescribeInstancesfallback when the counter is cold/stale - combining both mitigations from #4710.What this adds on top of #4983:
INSTANCE#<id>, COUNTED/TERMINATED) written with the counter update in oneTransactWriteItems;+1once,-1only when counted, can't go negative. Lets it count on the first active event (pendingorrunning) - matching the original "active and pending" count and avoiding boot-window over-provisioning.getCurrentRunnerscontract (no EC2 specifics). The counter Lambda is the only EC2-aware piece.Consistency trade-off (upfront): the counter is eventually consistent (updated a few seconds after each state change via the event path), so it can briefly under-report just-launched runners - the "can scale beyond max" drawback noted in #4710. Bounded by in-process accounting within a scale-up invocation and the staleness fallback to
DescribeInstances. The marker leaves room for a synchronous+1at launch as a follow-up if needed.Test Plan
running/terminatedare no-ops (idempotency), out-of-orderterminateddoesn't underflow, env-filter/tag guards, and non-cancellation errors rethrow for retry.runner count cache classes (in-memory + DynamoDB) - 21 tests.
terraform validateclean;terraform planshows only the expected new resources whenrunner_count_cache.enable = true, and no changes when disabled (opt-in).DescribeInstanceskept off the scale-up hot path and the DLQ staying empty. Note: single-repo test setup, so this is functional evidence rather than a 20K/day-scale benchmark.Related Issues
Addresses #4710. Builds on the approach in #4983 . Read path is written to align with the storage/plugin-layout refactor in #5277.