[main] Source code updates from dnceng/internal/dotnet-build-insights - #6606
Conversation
…-insights / b0cad7a build 327400 Diff: https://dev.azure.com/dnceng/internal/_git/dotnet-build-insights/compare/67ee5b086154174d904cfea186c3c1f109795e0d..b0cad7a45e20bf64b1c410c9165c519c90470a1c From: https://dev.azure.com/dnceng/internal/_git/dotnet-build-insights/commit/67ee5b086154174d904cfea186c3c1f109795e0d To: https://dev.azure.com/dnceng/internal/_git/dotnet-build-insights/commit/b0cad7a45e20bf64b1c410c9165c519c90470a1c [[ commit created by automation ]]
No dependency updates to commit [[ commit created by automation ]]
|
Note VMR PRs included in this codeflow update: 💡 You may consult the FAQ for more information or tag @dotnet/prodconsvcs for assistance. |
No dependency updates to commit [[ commit created by automation ]]
There was a problem hiding this comment.
Pull request overview
This PR is a codeflow update that reworks PCS work-item processing control from a single “state” key into a desired/observed protocol, adding an in-process admission gate and a replica-side controller to safely stop/start queue processing during deployments and operator actions.
Changes:
- Introduces
IWorkItemProcessorStateStore(desired + observed Redis keys),WorkItemAdmissionGate, andWorkItemProcessorStateControllerto manage replica-side queue admission and state acknowledgements. - Updates the deployment CLI flow to coordinate replica state across revisions via
ReplicaStateCoordinatorbefore traffic transfer and cleanup. - Updates PCS API initialization and the
/statuscontroller to use the new state protocol; adjusts/expands tests accordingly.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/Tools.Cli.Common/Options/DeploymentOptions.cs | Updates CLI DI registrations to use the new replica provider + state store + coordinator. |
| tools/Tools.Cli.Common/Operations/ReplicaStateCoordinator.cs | Adds control-plane polling logic for desired/observed state convergence per revision replica set. |
| tools/Tools.Cli.Common/Operations/DeploymentOperation.cs | Reworks deployment sequencing to stop old replicas, start candidate replicas, transfer traffic, and cleanup state keys. |
| src/Maestro/Maestro.WorkItems/WorkItemProcessorState.cs | Replaces string-based state machine with a simplified WorkItemProcessorState enum. |
| src/Maestro/Maestro.WorkItems/WorkItemProcessorStateStore.cs | Adds desired/observed Redis key storage for queue processing state. |
| src/Maestro/Maestro.WorkItems/ReplicaWorkItemProcessorStateStore.cs | Adds replica-scoped interface to enforce “replica reads desired, writes observed”. |
| src/Maestro/Maestro.WorkItems/WorkItemAdmissionGate.cs | Adds an in-process admission gate + draining semantics for safe stopping. |
| src/Maestro/Maestro.WorkItems/WorkItemProcessorStateController.cs | Adds a hosted service to reconcile desired state into admission open/close and observed acknowledgements. |
| src/Maestro/Maestro.WorkItems/WorkItemScopeManager.cs | Switches work-item scope creation to be governed by the admission gate (with cancellation). |
| src/Maestro/Maestro.WorkItems/WorkItemProcessorReplicaProvider.cs | Adds replica enumeration abstraction for local vs ACA environments. |
| src/Maestro/Maestro.WorkItems/WorkItemConsumer.cs | Updates consumer loop to pass cancellation into scope admission and stop cleanly on cancellation. |
| src/Maestro/Maestro.WorkItems/WorkItemConfiguration.cs | Registers new state/admission/controller services and improves local queue initialization logging/perf. |
| src/ProductConstructionService/ProductConstructionService.Api/Controllers/StatusController.cs | Moves Start/Stop/Status to replicaProvider + stateStore using desired/observed keys. |
| src/ProductConstructionService/ProductConstructionService.Api/InitializationHealthCheck.cs | Uses WorkItemProcessorStateController.IsInitializationPending instead of querying old state. |
| src/ProductConstructionService/ProductConstructionService.Api/InitializationBackgroundService.cs | Signals initialization completion via the state controller. |
| src/ProductConstructionService/ProductConstructionService.Api/Configuration/WorkItemProcessorStateInitialization.cs | In dev only, seeds desired state to Working for the local replica. |
| test/Maestro/Maestro.WorkItems.Tests/WorkItemScopeTests.cs | Updates tests to use admission gate; adds assertions around admission release and closed-gate behavior. |
| test/Maestro/Maestro.WorkItems.Tests/WorkItemProcessorStateStoreTests.cs | Adds coverage for desired/observed keys, parsing, and delete behavior. |
| test/Maestro/Maestro.WorkItems.Tests/WorkItemProcessorStateControllerTests.cs | Adds coverage for controller start/stop sequencing, draining, and initialization gating. |
| test/Maestro/Maestro.WorkItems.Tests/WorkItemAdmissionGateTests.cs | Adds coverage for open/close/drain semantics. |
| test/Maestro/Maestro.WorkItems.Tests/FakeRedisCache.cs | Expands fake Redis to support multi-key store; adds a fake cache factory. |
| test/Maestro/Maestro.WorkItems.Tests/WorkItemsProcessorScopeManagerTests.cs | Removes legacy tests tied to the old state machine implementation. |
| src/Maestro/Maestro.WorkItems/WorkItemProcessorStateCache.cs | Removes legacy single-key state cache implementation. |
| src/Maestro/Maestro.WorkItems/ReplicaWorkItemProcessorStateCache.cs | Removes legacy replica cache factory implementation. |
| src/Maestro/Maestro.WorkItems/LocalReplicaWorkItemProcessorStateCacheFactory.cs | Removes legacy local replica cache factory. |
| global.json | Adds additional runtime roll-forward entries (10.0.10 alongside 10.0.11). |
| eng/Version.Details.xml | Updates codeflow source SHA/BarId for dotnet-build-insights. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
Suppressed comments (7)
tools/Tools.Cli.Common/Operations/ReplicaStateCoordinator.cs:61
- The first rollout of this protocol cannot stop an existing replica running the previous implementation: that code reads and writes the single
<replicaName>key and transitions throughStopping, while this code writesqueue-processing-state:<replica>:desiredand waits on a new observed key. Such replicas can never acknowledge this command, so deployment times out and compensates instead of upgrading. Add a migration path that can stop legacy replicas, or stage a backward-compatible replica implementation before switching the coordinator.
await _stateStore.SetDesiredStateAsync(replicaName, desiredState, cancellationToken);
tools/Tools.Cli.Common/Operations/DeploymentOperation.cs:199
- This newly added async helper must use the
Asyncsuffix per the repository's C# coding guidelines; rename it toCompensateAsyncand update its callers.
private async Task Compensate(string? oldRevisionName, string? candidateRevisionName)
tools/Tools.Cli.Common/Operations/DeploymentOperation.cs:229
- This newly added async helper must use the
Asyncsuffix per the repository's C# coding guidelines; rename it toWaitForRevisionToBecomeHealthyAsyncand update its caller.
private async Task<bool> WaitForRevisionToBecomeHealthy(string revisionName, bool wasReusedInactiveCandidate)
tools/Tools.Cli.Common/Operations/DeploymentOperation.cs:346
- This newly added async helper must use the
Asyncsuffix per the repository's C# coding guidelines; rename it toStopDeactivateAndCleanupRevisionAsyncand update its callers.
private async Task<bool> StopDeactivateAndCleanupRevision(string revisionName)
tools/Tools.Cli.Common/Operations/DeploymentOperation.cs:466
- This newly added async helper must use the
Asyncsuffix per the repository's C# coding guidelines; rename it toActivateRevisionAsyncand update its callers.
private async Task ActivateRevision(string revisionName)
tools/Tools.Cli.Common/Operations/DeploymentOperation.cs:160
- This newly added async helper must use the
Asyncsuffix per the repository's C# coding guidelines; rename it toTransferTrafficAsyncand update its caller.
This issue also appears in the following locations of the same file:
- line 199
- line 229
- line 346
- line 466
private async Task<bool> TransferTraffic(string candidateRevisionName, string? oldRevisionName, string label)
src/Maestro/Maestro.WorkItems/WorkItemProcessorStateStore.cs:110
- These writes no longer set the 60-day expiration used by the removed state cache. Replica names disappear during scale-in, and keys written by the status endpoints are not tracked by
ReplicaStateCoordinator.DeleteStateAsync, so desired/observed keys for retired replicas can accumulate indefinitely (and remain available as stale state if a name is reused). Restore a bounded retention policy or add authoritative cleanup for departed replicas.
await _redisCacheFactory.Create(key).SetAsync(state.ToString());
Note
This is a codeflow update. It may contain both source code changes from
the VMR
as well as dependency updates. Learn more here.
This pull request brings the following source code changes
From https://dev.azure.com/dnceng/internal/_git/dotnet-build-insights
Associated changes in source repos
Diff the source with this PR branch