[PERF] Use standalone Helix monitor for slow performance jobs - #133726
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: Successfully started running 6 pipeline(s). 10 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @dotnet/runtime-infrastructure |
There was a problem hiding this comment.
🔵 Needs a closer look
Wire monitor parameters through the performance templates before approval.
Pull request overview
Adds standalone Helix monitoring to the perf-slow pipeline for asynchronous submission and completion tracking.
Changes:
- Configures scoped credentials and a 360-minute monitor cap.
- Preserves existing job selection, triggers, and schedules.
- Allows legitimate zero-submission runs.
File summaries
| File | Review |
|---|---|
eng/pipelines/performance/perf-slow.yml |
Adds monitor configuration, but monitor parameters are not wired through the performance templates, so jobs remain synchronous and completion gating is ineffective. |
Review details
Suppressed comments (1)
eng/pipelines/performance/perf-slow.yml:64
- The monitor job is not connected to the submitter configuration. At the pinned
dotnet/performancerevision,runtime-slow-perf-jobs.ymlforwardsjobParametersintoruntime-perf-job, butrun-performance-job.ymlinvokessend-to-helix-step.ymlwith onlyosGroupandprojectFile; that step'sUseHelixMonitordefaults tofalseandWaitForWorkItemCompletiondefaults totrue. Therefore these slow jobs remain synchronous, so agents stay occupied and the standalone monitor cannot provide the stated asynchronous submission/completion gating. Wire a monitor parameter through the performance templates (includingUseHelixMonitorand its wait behavior) and pass it from this pipeline before merging.
- ${{ if eq(variables['enableHelixJobMonitor'], true) }}:
- template: /eng/common/core-templates/job/helix-job-monitor.yml
parameters:
${{ if and(eq(variables['System.TeamProject'], 'internal'), ne(variables['Build.Reason'], 'PullRequest')) }}:
helixAccessToken: $(HelixApiAccessToken)
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Private PullRequest workloads may be undiscoverable by the monitor, allowing failures to leave the pipeline green.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5c9e5ce3-0d1b-440d-9c69-78918c5f6dc5
There was a problem hiding this comment.
🔵 Needs a closer look
Credential and template behavior lacks live Azure validation for internal and public paths.
Review details
Suppressed comments (1)
eng/pipelines/performance/perf-slow.yml:62
- This credential path is the one that previously caused false-green runs when the standalone monitor could not discover private Helix jobs. The final revision has no Azure template preview or live internal-PR/public validation, and local YAML/expression checks cannot verify stage variable-group inheritance or runtime secret expansion. Please validate the compiled plan and exercise both the internal and public paths before relying on this change.
helixAccessToken: ${{ iif(eq(variables['System.TeamProject'], 'internal'), '$(HelixApiAccessToken)', '') }}
- Files reviewed: 1/1 changed files
- Comments generated: 0 new
- Review effort level: Lite
Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
There was a problem hiding this comment.
🟡 Changes recommended
Moderate issues remain with token handling, submitter parameter propagation, and the monitor timeout.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
eng/pipelines/performance/perf-slow.yml:40
- This value does not actually reach the slow submitters. At the pinned
dotnet/performancerevision,runtime-slow-perf-jobs.ymlforwardsjobParameters, butruntime-perf-job.ymlinvokesrun-performance-job.yml, whosesend-to-helix-step.ymlcall omitsUseHelixMonitor; that parameter defaults tofalseandWaitForWorkItemCompletionremainstrue. The submitter jobs therefore still wait for Helix and occupy agents, so this standalone monitor cannot provide the claimed asynchronous submission/completion gating. Wire those parameters through the performance templates (or update the pinned template) before enabling this flag.
value: true
- Files reviewed: 1/1 changed files
- Comments generated: 2
- Review effort level: Lite
## Summary Add `release/*.0` to the CI branch filter in `eng/pipelines/performance/perf-slow.yml` so current and future mainline release branches trigger slow performance runs alongside `main`. Scheduled runs remain main-only. Path filters, batching, job configuration, `perf.yml`, and `perf-build.yml` are unchanged. Follow-up to #133634 and relates to #133726 (comment). ## Validation - Parsed the updated YAML and checked 12 branch-matching cases locally. - Confirmed the upstream PR diff contains only the one-line CI branch filter addition. - `git diff --check` passed. - No runtime build or tests were run for this pipeline-only change. The Azure pipeline has not been executed for this change. > [!NOTE] > This change and PR description were generated with GitHub Copilot. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…#133726) ## Summary Adopt the standalone Helix Job Monitor for `perf-slow`, following dotnet#132807, the stage-scoped credential fix in dotnet#133633, and the internal-PR credential alignment proposed in dotnet#133885. - Always enable asynchronous submission and include the standalone monitor job with an explicit parameter block, including on public and no-selection runs. Existing benchmark selectors are unchanged; no public benchmark workloads are added. - Import `DotNet-HelixApi-Access` at stage scope for all internal-project runs, including PRs, since the sibling monitor does not inherit submitter job variable groups. Always include `helixAccessToken`, using a compile-time `iif` expression to select `$(HelixApiAccessToken)` only for the internal project and an empty string for anonymous access elsewhere. Public runs, including public PRs, receive no private group or token. - Keep a six-hour monitor job cap (`360` minutes; the tool's maximum wait is `355` minutes). Existing submitter and workitem timeouts remain unchanged, so this is not a whole-pipeline wall-clock deadline. - Allow legitimate zero-submission runs and preserve the existing private/scheduled job selection, sanity flag, triggers, and schedules. Only `eng/pipelines/performance/perf-slow.yml` changes. ## Validation Internal pipeline **1012**, [sanity run 3072457](https://dev.azure.com/dnceng/internal/_build/results?buildId=3072457), ran with `onlySanityCheck=true`, `runScheduledJobs=true`, and `runPrivateJobs=true`. - Tested runtime commit: `12fb88f1065a13388d7488fd28e65e18006a08f2`. - Pinned performance resource: `c735123e8b45db1c8d0e30207bfe24a5fca2621c`. - All four build jobs and five Azure submitter jobs succeeded. Sends completed in 29-45 seconds, before their Helix workloads finished. - At 22:23:48 UTC, monitor log **239** recorded nine Azure jobs completed, none running or waiting, while one Helix job/workitem was still running. The monitor was the only remaining Azure job. - After the final workitem finished at 22:24:06 UTC, the monitor drained all results and exited with code 1. Its final summary was **5 jobs submitted, 0 resubmitted, 5 processed; 5 workitems, 5 failed; 5 results uploaded**. The pipeline correctly finished **failed**, not false-green. This demonstrates private job discovery, asynchronous agent release, completion gating, and failure propagation. **It is not a fully passing benchmark run.** ### Failure caveats Four Linux workitems failed with `NETSDK1045`: generated benchmark projects target .NET 12.0 while the SDK reports support through .NET 11.0. The same error was verified in [main's slow-performance run 3072225](https://dev.azure.com/dnceng/internal/_build/results?buildId=3072225), including Linux Helix job `59558d46-2044-4bbe-9b59-cc369574171a`, `arm64.micro.net11.0.Partition0`, console line 848. The Windows workitem failed differently: `WinError 225` blocked `dotnet --info` for the downloaded SDK as a virus or potentially unwanted software. This is **unclassified**, not a confirmed false positive or confirmed pre-existing failure. Evidence: Helix job `071489c5-3f06-4671-b957-0a25abcd5d65`, `arm64.micro.net11.0.Partition0`, console line 436. Build Analysis supplied no known-issue match. ### Final follow-up coverage Subsequent revisions decouple private credentials from monitor eligibility. Commit `1b1a9defdfb3d4a1e1e0b47ba5edaeeb0794ce0e` now includes the flag, job, and all three parameters unconditionally. These changes were **not included in the live run** at `12fb88f1065a13388d7488fd28e65e18006a08f2`. Commit `7b7d4d1bb9319a3f377126a1139e8f60b095db11` also aligns both credential guards with the internal-project-only predicate in dotnet#133885 (reference head `0250fbf77bfe09a37256a550a2a3de35414444e9`, open when inspected). It enables credentials for internal PRs while leaving public PRs anonymous. This revision has no new live-run coverage. The final version passed local unique-key YAML parsing and 192 project/reason/private/scheduled/sanity combinations, including 68 public or no-selection cases with a monitor but no benchmark submissions. Checks cover unconditional monitor inclusion, credential import and token selection for internal runs including PRs, an explicit empty token for all other projects, the 360/355-minute limits, preserved workload selection/triggers/schedules, the monitor-template contract, and `git diff --check`. An Azure preview of the earlier unconditional-monitor revision was attempted but denied because `EditBuild` permission is required. No permissions were changed or bypassed. The local expression checks are not an Azure template compiler or a live internal-PR/public-access test, and no new manual CI run or local product build was performed for this pipeline-only revision. > [!NOTE] > This change and PR description were prepared with GitHub Copilot. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com> Copilot-Session: 5c9e5ce3-0d1b-440d-9c69-78918c5f6dc5
## Summary Add `release/*.0` to the CI branch filter in `eng/pipelines/performance/perf-slow.yml` so current and future mainline release branches trigger slow performance runs alongside `main`. Scheduled runs remain main-only. Path filters, batching, job configuration, `perf.yml`, and `perf-build.yml` are unchanged. Follow-up to dotnet#133634 and relates to dotnet#133726 (comment). ## Validation - Parsed the updated YAML and checked 12 branch-matching cases locally. - Confirmed the upstream PR diff contains only the one-line CI branch filter addition. - `git diff --check` passed. - No runtime build or tests were run for this pipeline-only change. The Azure pipeline has not been executed for this change. > [!NOTE] > This change and PR description were generated with GitHub Copilot. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
## Description Backport the Helix job monitor integration from main/release/11.0, including the groundwork from #129690 and #132150 and the re-enablement in #131969. Includes the subsequent fixes for empty stages (#132019), conditional monitor inclusion (#132882, #132884), parameter forwarding (#133002), performance monitoring (#132807, #133480), and internal credentials (#133633, #133688, #133885). - Use `Microsoft.DotNet.Helix.JobMonitor` version `10.0.0-beta.26461.103`, matching release/10.0's existing Arcade/VMR build. - Preserve the existing SDK, shared Arcade templates, queues, and release/10.0 job layouts. - Follow upstream enablement, except scheduled libraries outerloop runs retain release/10.0's existing warning-only reporting policy. - Leave SuperPMI's post-Helix processing unchanged. The separate perf-slow enablement in #133726 is not included. ## Customer Impact CI infrastructure only; no shipped runtime changes. Moves Helix waiting and test-result reporting into the standalone monitor for the enabled pipelines. ## Regression Not a product regression fix; backports existing CI infrastructure and its follow-up fixes. ## Testing - Validated YAML/JSON/XML configuration and preservation of unrelated settings. - Checked 43 entry pipelines, 149 forwarding sites, and 516 public/internal, PR/scheduled/manual, and normal/staging combinations, plus disabled-mode behavior. - Exercised MSBuild child-property forwarding and the pinned SDK's waiting/reporter properties, including environment-based opt-in. - Restored the monitor and verified its CLI compatibility under .NET 10. No product build or live Azure DevOps pipeline execution was performed locally. ## Risk Changes CI scheduling and result reporting, not product behavior. Uses the monitor package matching the branch's existing toolset and retains the release/10.0-specific scheduled outerloop failure policy. End-to-end Azure DevOps validation is still required. > [!NOTE] > This PR description and backport were prepared with GitHub Copilot. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: aae9b201-32a8-45eb-ae1a-db80a0ab4156
Summary
Adopt the standalone Helix Job Monitor for
perf-slow, following #132807, the stage-scoped credential fix in #133633, and the internal-PR credential alignment proposed in #133885.DotNet-HelixApi-Accessat stage scope for all internal-project runs, including PRs, since the sibling monitor does not inherit submitter job variable groups. Always includehelixAccessToken, using a compile-timeiifexpression to select$(HelixApiAccessToken)only for the internal project and an empty string for anonymous access elsewhere. Public runs, including public PRs, receive no private group or token.360minutes; the tool's maximum wait is355minutes). Existing submitter and workitem timeouts remain unchanged, so this is not a whole-pipeline wall-clock deadline.Only
eng/pipelines/performance/perf-slow.ymlchanges.Validation
Internal pipeline 1012, sanity run 3072457, ran with
onlySanityCheck=true,runScheduledJobs=true, andrunPrivateJobs=true.12fb88f1065a13388d7488fd28e65e18006a08f2.c735123e8b45db1c8d0e30207bfe24a5fca2621c.This demonstrates private job discovery, asynchronous agent release, completion gating, and failure propagation. It is not a fully passing benchmark run.
Failure caveats
Four Linux workitems failed with
NETSDK1045: generated benchmark projects target .NET 12.0 while the SDK reports support through .NET 11.0. The same error was verified in main's slow-performance run 3072225, including Linux Helix job59558d46-2044-4bbe-9b59-cc369574171a,arm64.micro.net11.0.Partition0, console line 848.The Windows workitem failed differently:
WinError 225blockeddotnet --infofor the downloaded SDK as a virus or potentially unwanted software. This is unclassified, not a confirmed false positive or confirmed pre-existing failure. Evidence: Helix job071489c5-3f06-4671-b957-0a25abcd5d65,arm64.micro.net11.0.Partition0, console line 436. Build Analysis supplied no known-issue match.Final follow-up coverage
Subsequent revisions decouple private credentials from monitor eligibility. Commit
1b1a9defdfb3d4a1e1e0b47ba5edaeeb0794ce0enow includes the flag, job, and all three parameters unconditionally. These changes were not included in the live run at12fb88f1065a13388d7488fd28e65e18006a08f2.Commit
7b7d4d1bb9319a3f377126a1139e8f60b095db11also aligns both credential guards with the internal-project-only predicate in #133885 (reference head0250fbf77bfe09a37256a550a2a3de35414444e9, open when inspected). It enables credentials for internal PRs while leaving public PRs anonymous. This revision has no new live-run coverage.The final version passed local unique-key YAML parsing and 192 project/reason/private/scheduled/sanity combinations, including 68 public or no-selection cases with a monitor but no benchmark submissions. Checks cover unconditional monitor inclusion, credential import and token selection for internal runs including PRs, an explicit empty token for all other projects, the 360/355-minute limits, preserved workload selection/triggers/schedules, the monitor-template contract, and
git diff --check.An Azure preview of the earlier unconditional-monitor revision was attempted but denied because
EditBuildpermission is required. No permissions were changed or bypassed. The local expression checks are not an Azure template compiler or a live internal-PR/public-access test, and no new manual CI run or local product build was performed for this pipeline-only revision.Note
This change and PR description were prepared with GitHub Copilot.