fix(perf): Avoid server-side tag filtering in listEC2Runners DescribeInstances call - #5328
fix(perf): Avoid server-side tag filtering in listEC2Runners DescribeInstances call#5328wadherv wants to merge 2 commits into
Conversation
|
There is a plan to use dynamodb for caching the instance Metadata. My concern with your change is for aws regions with multiple installations, runner types and heavy usage. |
|
@edersonbrilhante Thanks for the feedback. I understand there are plans to introduce DynamoDB-based caching, although I'm not sure when that work will be available. |
|
Another observation from our production environment is that the current implementation does not scale linearly. As the number of runners grows, DescribeInstances latency increases and provisioning performance degrades accordingly. Smaller deployments are less impacted because the API responses are significantly faster at lower instance counts. |
|
Thanks for giving the environment context. There is this opened PR #5281 to use dynamodb as caching strategy And I am working in some drafts like this one to allow extract the ssm and ec2 tags filter approach to use storage plugin that would allow expands the scalabity issues(ssm and ec2 api limits) I am working in different PRs to allow multiple providers:
About "when", I hope soon. in my lab I already made work runner backed in microvm and and the orchestration of ec2 via scaleset. I paused because I am waiting for reviews in a stacked PR. I plan to resume this week. I am waiting this PR get merged #5312, So I can resume the work in the providers. I started a discussion in #5316, I would appreciate feedbacks |
|
@wadherv Update: Once the PRs in stacked PR #5367 are reviewed and merged. I will be able to request review in microvm and scaleset feature. To support microvm I needed to decouple the ssm from scale/up and pool, moving it to a storage provider with a contract interface. By doing it, now we have space to add dynamodb as alternative to cache/config/metadata. Hope soon we can fix this issue with aws api :) |
Description
The Scale-Up Lambda's runner-lookup call (
listEC2Runnersinlambdas/libs/compute-providers/aws/ec2/src/control-plane/runners.ts) dominated total invocation latency. X-Ray traces showed total invocation duration consistently ~35s, while the actual provisioning logic (credential lookups,CreateFleet) was sub-2s —DescribeInstancesalone accounted for ~93% of the invocation (~24–33s), reproducing consistently across 10+ sampled traces over a 24-hour window.Root-cause investigation ruled out the usual suspects:
tag:ghr:*filter (environment,Type,Owner,Application, optionallyorphan) pays the same tag-index lookup cost server-side; the current implementation stacks up to four.DescribeTags, Resource Groups Tagging API) — both performed the same or worse. This confirmed the fix isn't "use a different tag-lookup API," it's "avoid server-side tag filtering in this call entirely."This PR changes
listEC2Runnersto fetch byinstance-state-nameonly (a fast, native attribute) and filter by tag in application code afterward, instead of asking EC2 to filter by tags server-side:Since
Instance.Tagsis already returned on every instance in theDescribeInstancesresponse, no extra API calls are needed — the tag data is just filtered in-process instead of server-side. This is an internal implementation change only:listEC2Runners's signature and theEc2ListRunnerFiltersinput shape are unchanged, so no changes were needed in any of its callers (pool.ts,scale-up.ts,scale-down.ts).Measured locally with real data: state-only fetch took 6.4s for ~950 instances, with client-side tag filtering afterward costing under 2ms — a ~5x improvement over the ~24–33s baseline.
Test Plan
runners.test.tsupdated: filter-assertion tests now confirm onlyinstance-state-nameis sent toDescribeInstancesCommand, and new tests verify correct client-side inclusion/exclusion forenvironment,Type+Owner,orphan, andApplicationtag combinations (including instances missing theApplicationtag being correctly excluded).vitest runonrunners.test.ts: 71/71 passing.compute-providerspackage test suite: 10 files / 272 tests passing (no ripple effects inpool.ts,scale-up.ts,scale-down.tsor their tests).functions/control-plane/src/pool/pool.test.ts: 20/20 passing.tsc --noEmitandeslintclean on both changed files.docs//README.mdcontent describes this internal filtering behavior, so no doc updates needed.Related Issues
#5327