Problem
go test -race ./providers/aws/recommendations/... reports concurrent
writes to (*RateLimiter).Reset after PR #269 parallelised the AWS
service-loop:
WARNING: DATA RACE
Write at 0x... by goroutine N:
recommendations.(*RateLimiter).Reset()
providers/aws/recommendations/ratelimiter.go:132
recommendations.(*Client).GetRecommendations()
providers/aws/recommendations/client.go:83
recommendations.(*Client).GetRecommendationsForService()
providers/aws/recommendations/client.go:158
recommendations.(*Client).GetAllRecommendations.func{1..5}()
providers/aws/recommendations/client.go:215
Root cause: Client.rateLimiter is a single *RateLimiter instance
held on the Client struct (client.go:28). Both Reset() and the
internal state mutated by Wait / ShouldRetry / GetRetryCount are
shared across the 5 per-service goroutines launched in
GetAllRecommendations. None of those methods take a lock.
Why it's not been visible
- The project's default
go test invocation doesn't pass -race.
- The
pre-commit hook runs go test without -race.
- All standard tests pass (the race produces interleaved-state behaviour
that doesn't typically cause assertion failures, just throttling
imbalance and inflated retry counts in production).
Acceptance criteria
References
Problem
go test -race ./providers/aws/recommendations/...reports concurrentwrites to
(*RateLimiter).Resetafter PR #269 parallelised the AWSservice-loop:
Root cause:
Client.rateLimiteris a single*RateLimiterinstanceheld on the Client struct (
client.go:28). BothReset()and theinternal state mutated by
Wait/ShouldRetry/GetRetryCountareshared across the 5 per-service goroutines launched in
GetAllRecommendations. None of those methods take a lock.Why it's not been visible
go testinvocation doesn't pass-race.pre-commithook runsgo testwithout-race.that doesn't typically cause assertion failures, just throttling
imbalance and inflated retry counts in production).
Acceptance criteria
go test -race ./providers/aws/recommendations/...passes fromproviders/aws/.sync.Mutex(preserves the shared-state-across-services design); orRateLimiter(no shared state) — preferred since AWS Cost Explorerrate limits are per-API-call, not aggregate, so the shared limiter
isn't actually doing useful coordination.
-raceto thepre-commitgo testinvocation (or to aCI-only matrix entry) so future regressions are caught.
References
defac6c2a) — the parallelisedGetAllRecommendationsexposes pre-existing single-threaded code toconcurrent callers.
-raceto validateunrelated test changes in
pkg/concurrency.