Add per-phase graph concurrency limits - #9752
Open
Rick Winter (RickWinter) wants to merge 3 commits into
Open
Conversation
Rick Winter (RickWinter)
requested review from
JeffreyCA,
Marina He (hemarina),
Richard Park (richardpark-msft),
tg-msft and
Victor Vazquez (vhvb1989)
as code owners
August 27, 2026 04:52
|
Azure Pipelines: Successfully started running 1 pipeline(s). 20 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds per-phase concurrency limits to the graph scheduler and wires them into up, deploy, and provision.
Changes:
- Adds fair concurrency-group scheduling beneath a global ceiling.
- Adds centralized environment-variable precedence and validation.
- Documents and tests phase limits, fairness, callbacks, and failure handling.
Show a summary per file
| File | Description |
|---|---|
docs/specs/exegraph/spec.md |
Updates scheduler specification. |
cli/azd/pkg/exegraph/step.go |
Adds step concurrency groups. |
cli/azd/pkg/exegraph/scheduler.go |
Implements grouped admission scheduling. |
cli/azd/pkg/exegraph/scheduler_test.go |
Adds scheduler regression tests. |
cli/azd/internal/cmd/up_graph.go |
Configures up concurrency. |
cli/azd/internal/cmd/service_graph.go |
Groups package and deployment steps. |
cli/azd/internal/cmd/service_graph_test.go |
Verifies service group assignments. |
cli/azd/internal/cmd/provision_graph.go |
Groups provision steps. |
cli/azd/internal/cmd/deploy.go |
Configures deploy concurrency. |
cli/azd/internal/cmd/deploy_test.go |
Removes superseded resolver tests. |
cli/azd/internal/cmd/concurrency.go |
Centralizes concurrency resolution. |
cli/azd/internal/cmd/concurrency_test.go |
Tests precedence and parsing. |
cli/azd/docs/environment-variables.md |
Documents concurrency variables. |
cli/azd/docs/concurrency-model.md |
Documents scheduler and phase semantics. |
Review details
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 14/14 changed files
- Comments generated: 1
- Review effort level: Balanced
Contributor
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
Victor Vazquez (vhvb1989)
left a comment
Victor Vazquez (vhvb1989)
left a comment
Member
There was a problem hiding this comment.
A few concurrency configuration and observability questions to consider.
Contributor
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: None
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
 |
cli/azd/pkg/exegraph/scheduler.go — This stable sort still leaves tied steps nondeterministic because dependents is populated by… View resolved comment |
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
cli/azd/pkg/exegraph/scheduler.go:443
- This reports every never-admitted fail-fast step as
StepSkippedError, but the existing provision callback interprets every such error as “dependency failed” (provision_graph.go:497-500). An independent layer waiting behind the concurrency ceiling will therefore show a false dependency-failure reason. Preserve the terminal notification while distinguishing fail-fast cancellation from dependency propagation, and update consumers to report the actual cause.
skipErr := &StepSkippedError{StepName: name}
safeNotifyDone(opts, name, skipErr)
docs/specs/exegraph/spec.md:287
scheduler_test.gocurrently defines 50 top-level tests, so this count is stale.
This issue also appears on line 296 of the same file.
| `pkg/exegraph/scheduler_test.go` | 49 | Execution semantics, cancellation, skip propagation, global and group concurrency bounds, group fairness, deterministic ordering, panic recovery, goroutine cleanup, timing, per-step timeout |
docs/specs/exegraph/spec.md:296
- With 15 graph tests and 50 scheduler tests, the documented engine total is 65 rather than 64.
**64 exegraph engine tests** (15 graph + 49 scheduler). Additional integration tests across
Victor Vazquez (vhvb1989)
approved these changes
Sep 1, 2026
Victor Vazquez (vhvb1989)
left a comment
Victor Vazquez (vhvb1989)
left a comment
Member
There was a problem hiding this comment.
LGTM, thank you
Marina He (hemarina)
approved these changes
Sep 1, 2026
Marina He (hemarina)
left a comment
Marina He (hemarina)
left a comment
Contributor
There was a problem hiding this comment.
Please preserve the .NET publish-isolation wiring from #9775 when resolving the current conflicts with main in deploy.go and up_graph.go.
The resolved implementation should:
- Resolve the new global and group concurrency settings once.
- Pass
concurrency.maxtoserviceGraphOptions.maxConcurrency. - Retain
packagePublishBuildGateKeyandbuildGateKey. - Set both
RunOptions.MaxConcurrencyandRunOptions.GroupConcurrency.
The concurrency groups do not replace the .NET isolation gate. Package and publish use different groups, so they can overlap whenever the global ceiling is greater than one, even when each group has a limit of one. Dropping packagePublishBuildGateKey would prevent runIsolatedDotNetBuild from using an isolated ArtifactsPath and its serialized fallback, potentially reintroducing collisions in shared .NET obj directories.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7915
Summary
AZD_CONCURRENCY_MAXandAZD_PACKAGE_CONCURRENCY, while preserving command-level fallback behavior forazd up,azd deploy, andazd provision.Validation
mage preflight: gofmt, go fix, copyright, lint, cspell, cspell-misc, build, and unit tests passed.Test_CLI_Up_Down_ContainerFuncApppassed. That test requires the local Docker service, which is stopped.Telemetry Change Checklist
New Fields
fields/fields.godocs/specs/metrics-audit/telemetry-schema.mdNew Events
Privacy
CustomerContentemitted in telemetryTesting
exegraph.runspanDownstream
Documentation