Skip to content

fix(azure): bound concurrency in the multi-subscription recommendations fan-out #58

Open

Description

@cristim

Summary

MultiSubscriptionRecommendationsClient.GetRecommendations (providers/azure/recommendations_multi_subscription.go) fans out to every selected subscription with an unbounded errgroup — one goroutine per subscription, no SetLimit. Each per-subscription client then issues its own service calls (6 services), so a caller with N subscriptions can have roughly N x 6 ARM calls outstanding at once. For a 200-subscription tenant that is ~1200 simultaneous requests.

Latent today: the fan-out is not reachable in production (see LeanerCloud/cloud-commitments-cli#1520's disclosure), and when it is, the scheduler path attaches a CUDLY_MAX_PARALLELISM semaphore to the context, which the per-subscription clients acquire around their outbound calls.

The gap

The bound lives in the caller's context, not in the fan-out layer. A CLI invocation or any library consumer that constructs MultiSubscriptionRecommendationsClient without installing that semaphore gets no bound at all. The fan-out's own doc comment states the semaphore is acquired inside each per-subscription client "so no additional semaphore is needed at this layer" — true only for callers that supply one.

Consequences of the unbounded case: ARM throttling (429s) that the fan-out then reports as per-subscription failures, wasted retries, and a burst profile that can trip subscription-level rate limits for unrelated workloads in the same tenant.

Suggested direction

Give the fan-out a bound that does not depend on the caller remembering to install one:

  • g.SetLimit(n) on the errgroup, with n derived from the same configuration CUDLY_MAX_PARALLELISM feeds (do not hardcode a magic number), or
  • have NewMultiSubscriptionRecommendationsClient install a default semaphore on the context when the caller supplied none, so the caller-provided value still wins when present.

Either way, a test should assert the observed peak concurrency stays within the bound for a large subscription list.

Provenance

Found during the adversarial review of LeanerCloud/cloud-commitments-cli#1520. Deliberately not fixed there — the fan-out is unreachable today, and picking the bound is a configuration decision rather than a defect in the merge/error logic that PR is about.

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions