Repository navigation
fix(aws): EC2 per-pool expiry ignores size flexibility and keys non-RDS services without the service #450
Description
Summary
Two known limitations of the EC2 per-pool expiry adjustment in providers/aws/recommendations/expiry.go. They predate go#449, which documents them in its godoc.
- Size flexibility. Pools key on the exact instance type. A replacement of a different size in the same family is not credited, and an expiring RI of one size is not netted against coverage of another size. Size-flexible regional Linux/default-tenancy RIs are therefore over-deducted, which makes the tool re-buy. The RDS family path (
family_nu.go) handles this with normalized units. - Key collision. Non-RDS services share the
commitmentPoolKeyshape (expiry.go:~153). If two services used an identical ResourceType string in the same region, their expiring counts would combine.
Expected
- Credit and net EC2 size-flexible RIs in normalized units within the family, as the RDS family path does.
- Include the service in the pool key, or prove it already is unique per service.
Test
- A large RI expiring, replaced by two medium RIs, produces no re-buy.
- Two services with the same ResourceType string in one region keep their expiring counts separate.
Activity
cristim commentedon Oct 16, 2026
Claiming: working on this.
cristim commentedon Oct 16, 2026
Plan (
go-450-ec2-expiry-sizeflex-key.md) findings on main:Part 1, the key collision, is not reachable today. The non-RDS services that share the 2-part
region:typekey have disjoint ResourceType shapes:m5.largecache.m5.largem5.large.searchdc2.*/ra3.*db.r6g.largeRDS uses its own 4-part
rdsPoolKey. Adding a service dimension would changepoolKey,lookupPoolKeyandcommitmentPoolKeytogether, and with them the exportedPoolCoverageMapshape that the ladder and the CLI read. So I'm doing a guard test over the real per-service shapes plus a comment oncommitmentPoolKey, with no key change.Part 2, EC2 size flexibility, needs maintainer decisions before implementation, because it changes recommended purchase quantities. Proposal:
d = max(0, E_nu - R_nu). That keeps fix(aws): credit replacement RIs bought inside the coverage lookback in the EC2 expiry deduction #449's rule of possibly under-buying but never over-buying.dback to the per-size recs.Decisions needed:
100*d/Tcoverage points (uniform), or shoulddbe weighted toward the expiring sizes?CoverageLookbackDays == 0, i.e. on expiring-vs-expiring netting alone?Part 2 is about 400 lines and may split into two PRs. It changes no API, so the CLI caller picks it up on upgrade. I'm leaving part 2 open pending these answers.
test(aws): guard that non-RDS expiry pool keys stay disjoint across s…
test(aws): guard that non-RDS expiry pool keys stay disjoint across s…
cristim commentedon Nov 12, 2026
Fixed on
mainin 984f206, 5a3302b and f1ffe43 (head f1ffe43).Root cause
commitmentPoolKeykeyed EC2 by exact instance type, so an expiring large RI was not netted against a replacement of another size in the same family, and the rec re-bought.Change (
providers/aws/recommendations/expiry.go)error; this is an API change, recorded in CHANGELOG Notices.Tests
TestPoolExpiryNetsSizeFlexWithinFamily: large replaced by two mediums, no re-buy.TestPoolExpiryWeightsExpiringSizeByNormalizationFactor: lookback 0 weighting.TestPoolExpiryKeysNonRDSByService: no cross-service netting.TestPoolExpiryUnknownDimensionFailsLoud: error path, recs untouched.TestPoolExpiryAcceptsProducerServiceAliases,TestPoolExpiryAcceptsComputeRecommendationAlias.The first three fail on the previous code. Mutation probes on the weight, the service key and both error paths each fail a test.
Known limits, not in this change