Repository navigation
fix(aws/recs): data race on shared rateLimiter in fetchRIPageWithRetry #1263
Description
Discovered while reviewing #1218 (PERF-02 coverage semaphore). go test -race ./providers/aws/recommendations/... fails on origin/main in TestGetAllRecommendations*: GetAllRecommendations runs 6 concurrent goroutines that each call fetchRIPageWithRetry, which accesses the shared c.rateLimiter (incl. Reset()) concurrently. Per feedback_rate_limiter_per_call.md, a rate limiter must not be shared across concurrent sweeps.
This is pre-existing on main (not introduced by #1218; #1218 adds no new race). Fix: instantiate a fresh rate limiter per concurrent sweep, or guard the shared one with a mutex. Verify with go test -race.
Activity
cristim commentedon Jun 19, 2026
PR #865 (fix/271-wave18) resolves this. The shared
rateLimiter *RateLimiterfield was replaced with anewRateLimiter func() *RateLimiterfactory; everyfetch*WithRetry/fetchCoveragePage/fetchUtilizationPagecall now invokesrl := c.newRateLimiter()at entry, giving each concurrent sweep its own independent instance.go test -race ./providers/aws/recommendations/...passes (331 tests, 0 data race reports). The mock'scallCount/riCallsmutations are also guarded by async.Mutex.cristim commentedon Jul 27, 2026
Verification sweep against
main(101f099fb) on 2026-07-27 finds this already resolved.rateLimiter.newOperation() now gives each fetch call an independent retry counter.
Evidence: commit 63c7a90 (PR #842).
Recommending close.
cristim commentedon Sep 2, 2026
Verified resolved at 3c0f8ac: each fetch-with-retry path creates its own limiter through rateLimiter.newOperation(), so concurrent service sweeps no longer share retry state or call Reset() on a shared instance; go test -race on providers/aws/recommendations passes at this commit. Evidence: providers/aws/recommendations/client.go:212 and providers/aws/recommendations/ratelimiter.go:63 (#865). Residual axes checked: coverage, utilization, on-demand series, savings-plan and SP-coverage fetchers all use newOperation(); no remaining direct Wait/Reset on the shared c.rateLimiter. Closing as completed; reopen if the behaviour recurs.