Rewrite the Helix Job Monitor for scalable result processing - #17331
Rewrite the Helix Job Monitor for scalable result processing#17331mmitche wants to merge 13 commits into
Conversation
Batch Azure DevOps result requests by the service's 1,000 top-level result limit and increase work-item upload parallelism to eight. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77f2acbe-a455-4c48-9dee-58322d289862
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 77f2acbe-a455-4c48-9dee-58322d289862
Replace the per-job upload task graph with a channel-based work-item pipeline, stream test result parsing, preserve Azure DevOps batching semantics, and document the monitor architecture and durability model. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a3a2b2f-e044-4835-8a3c-0c99a6a29484
Track work items first observed terminal in the final poll, uploads newly eligible at the whole-job boundary, and remaining final-poll versus earlier backlog. Also expose upload parallelism through the shared Helix Job Monitor pipeline template. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a3a2b2f-e044-4835-8a3c-0c99a6a29484
Cache synchronous test-run creation failures in the per-job single-flight task and add regression coverage for creation, download, partial upload, and failed-test metadata behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a3a2b2f-e044-4835-8a3c-0c99a6a29484
Recognize Helix result files with a trailing .txt transport suffix, report explicit per-job file and result counts, and emit aggregate status on an independent five-minute timer throughout final drain. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a3a2b2f-e044-4835-8a3c-0c99a6a29484
Report request counts, retries, payload volume, latency, rate-limit behavior, pipeline-active throughput, and aggregate/max stage timings without per-request information logging. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a3a2b2f-e044-4835-8a3c-0c99a6a29484
Use the submitter-assigned logical Helix job name in work-stream identity so independent jobs from one AzDO submitter and queue cannot overwrite same-named work-item outcomes. Preserve the identifier through monitor resubmissions and document the revised reconciliation semantics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a3a2b2f-e044-4835-8a3c-0c99a6a29484
Prefer System.PhaseName when reconciling work-item outcomes because runtime stamps System.JobName=__default across independent matrix jobs. Preserve phase identity through resubmission, move semantic documentation into the new Design tree, and set the runtime-validated upload parallelism default to 48. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a3a2b2f-e044-4835-8a3c-0c99a6a29484
Specify that AzDO job identity and queue are insufficient when one job submits multiple Helix jobs to the same queue. Define the logical job discriminator, resubmission preservation, safe fallback, and submitter uniqueness requirement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a3a2b2f-e044-4835-8a3c-0c99a6a29484
There was a problem hiding this comment.
Pull request overview
This PR rewrites the Helix Job Monitor to handle very large Helix workloads by decoupling polling/status from result processing and introducing a globally bounded, channel-based upload pipeline with shared Azure DevOps throttling and streaming XML parsing, while preserving the existing retry/replay and pass/fail semantics.
Changes:
- Replace the per-job fire-and-forget upload queue with a globally bounded pipeline (job expansion → work-item processing → finalization) plus shared rate-limit gating and performance metrics.
- Move/reshape Azure DevOps test-result publishing into the Job Monitor (streaming XML reader, aggregation, batching/splitting) and extend result-file discovery to handle Helix’s
.xml.txt/.trx.txttransport suffix. - Add design documentation and update templates/tests to reflect new semantics, defaults (48 upload workers), and logging.
Reviewed changes
Copilot reviewed 40 out of 51 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Microsoft.DotNet.Helix/Sdk/Readme.md | Links to new Job Monitor design docs and updates behavioral notes for the new pipeline/streaming model. |
| src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/ScenarioHelpers/ScenarioHelpers.cs | Extends HelixJobInfo test helper to include logical job name and submitter phase name. |
| src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests.csproj | Removes reference to the deleted AzureDevOpsTestPublisher project. |
| src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/LocalTestResultsReaderTests.cs | Adds coverage for .txt-suffixed result artifacts and updates existing test to use .xml.txt. |
| src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/JobMonitorRunnerTests.cs | Updates and expands end-to-end runner tests for pipeline drain/status cadence, bounded concurrency, replay semantics, and new logging. |
| src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/HelixServiceTests.cs | Updates Helix download tests for .xml.txt handling and new per-work-item download API. |
| src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/Fakes/FakeHelixService.cs | Adapts fake Helix service to per-work-item downloads and adds list-work-items call counting for polling optimizations. |
| src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/Fakes/FakeAzureDevOpsService.cs | Adapts fake AzDO service to per-work-item uploads and adds concurrent-upload accounting used by new pipeline tests. |
| src/Microsoft.DotNet.Helix/Sdk.Tests/Microsoft.DotNet.Helix.Sdk.Tests/AzureDevOpsResultPublisherTests.cs | Adds/updates publisher tests for batching/splitting, throttling delay selection, lazy enumeration behavior, and new default parallelism. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResultUploadQueue.cs | Removes old fire-and-forget upload queue implementation. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResultUploadPipeline.cs | Introduces the new bounded, multi-stage upload pipeline with per-job single-flight test-run creation and finalization. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResults/TestResultUploadSummary.cs | Adds upload summary model now hosted in Job Monitor after publisher project removal. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResults/TestNameFormatter.cs | Adds formatting logic for AzDO display titles when fully qualified names are enabled. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResults/ResultAggregator.cs | Adds aggregation logic (reruns/data-driven/flaky/FQN grouping) now hosted under Job Monitor. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResults/Model/TestResultAttachmentMode.cs | Adds attachment-mode model under new Job Monitor-hosted publisher surface. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResults/Model/TestResultAttachment.cs | Adds attachment model under new Job Monitor-hosted publisher surface. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResults/Model/TestResult.cs | Adds normalized test-result model including stable FullyQualifiedName identity. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResults/Model/TerminalError.cs | Adds internal exception type for terminal publisher failures. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResults/Model/PackedTestReport.cs | Adds model for packed report payload representation. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResults/Model/AzureDevOpsReportingParameters.cs | Adds consolidated AzDO reporting parameter record used by publisher. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResults/Model/AzureDevOpsReportingError.cs | Adds publisher exception type for AzDO reporting failures. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResults/LocalTestResultsReader.cs | Adds streaming XML reader that supports xUnit/JUnit/TRX and warns on unsupported roots; strips a single trailing .txt. |
| src/Microsoft.DotNet.Helix/JobMonitor/TestResults/AzureDevOpsResultPublisher.cs | Reworks publisher to stream/partition requests (1,000 top-level results), split deep hierarchies, and share throttling/metrics. |
| src/Microsoft.DotNet.Helix/JobMonitor/StatusReporter.cs | Separates status reporting from Helix calls and adds bounded pipeline/metrics reporting. |
| src/Microsoft.DotNet.Helix/JobMonitor/Services/HelixService.cs | Changes downloads to per-work-item and adds metrics for Helix and blob-download operations; stamps logical job metadata. |
| src/Microsoft.DotNet.Helix/JobMonitor/Services/AzureDevOpsService.cs | Switches upload API to per-work-item, adds shared rate-limit gating + metrics recording, and reuses a shared HttpClient. |
| src/Microsoft.DotNet.Helix/JobMonitor/Services/AzureDevOpsRateLimitGate.cs | Adds a shared deferral gate to coordinate AzDO throttling across concurrent workers. |
| src/Microsoft.DotNet.Helix/JobMonitor/Parallelism/ParallelAsync.cs | Adds helper for bounded parallel snapshot reads (e.g., list-work-items fanout). |
| src/Microsoft.DotNet.Helix/JobMonitor/Parallelism/ActionQueue.cs | Adds reusable channel-based worker queue with bounded/unbounded modes and queue metrics. |
| src/Microsoft.DotNet.Helix/JobMonitor/MonitorState.cs | Updates stream identity keying (phase/queue/logical job), adds single-result observation helper, and adjusts lineage root logic. |
| src/Microsoft.DotNet.Helix/JobMonitor/Models/HelixJobInfo.cs | Adds logical job name + submitter phase name properties and persists them into Helix job properties. |
| src/Microsoft.DotNet.Helix/JobMonitor/Microsoft.DotNet.Helix.JobMonitor.csproj | Enables implicit usings/nullable annotations and removes AzureDevOpsTestPublisher project reference; keeps InternalsVisibleTo for tests. |
| src/Microsoft.DotNet.Helix/JobMonitor/JobMonitorRunner.cs | Refactors runner to use cached work-item snapshots, fixed-cadence status task, new pipeline drain/final metrics, and new production dependency wiring. |
| src/Microsoft.DotNet.Helix/JobMonitor/JobMonitorOptions.cs | Changes default result upload parallelism to 48 and updates CLI defaults accordingly. |
| src/Microsoft.DotNet.Helix/JobMonitor/JobMonitorMetrics.cs | Adds aggregate request/throughput/latency counters and snapshots used for final status reporting. |
| src/Microsoft.DotNet.Helix/JobMonitor/Interfaces/IHelixService.cs | Updates Helix service contract to per-work-item result downloads. |
| src/Microsoft.DotNet.Helix/JobMonitor/Interfaces/IAzureDevOpsService.cs | Updates AzDO service contract to per-work-item uploads. |
| src/Microsoft.DotNet.Helix/JobMonitor/Design/SemanticBehavior.md | Updates/renames spec to reflect new stream key identity and bounded pipeline semantics. |
| src/Microsoft.DotNet.Helix/JobMonitor/Design/README.md | Adds top-level index for the new design documentation set. |
| src/Microsoft.DotNet.Helix/JobMonitor/Design/Components/UploadPipeline.md | Documents the new multi-stage upload pipeline behavior and invariants. |
| src/Microsoft.DotNet.Helix/JobMonitor/Design/Components/TestResults.md | Documents streaming parsing, .txt suffix handling, aggregation, and AzDO limits/batching. |
| src/Microsoft.DotNet.Helix/JobMonitor/Design/Components/StateAndStatus.md | Documents state ownership and timer-driven status reporting. |
| src/Microsoft.DotNet.Helix/JobMonitor/Design/Components/Shutdown.md | Documents drain, cancellation behavior, and crash/replay durability boundary. |
| src/Microsoft.DotNet.Helix/JobMonitor/Design/Components/Polling.md | Documents polling snapshot reuse and bounded work-item refresh behavior. |
| src/Microsoft.DotNet.Helix/JobMonitor/Design/Components/PerformanceMetrics.md | Documents aggregate metrics captured without per-request logging noise. |
| src/Microsoft.DotNet.Helix/JobMonitor/Design/Architecture.md | Documents ownership boundaries, shared parallelism utilities, data flow, and durability boundary. |
| src/Microsoft.DotNet.Helix/AzureDevOpsTestPublisher/Microsoft.DotNet.Helix.AzureDevOpsTestPublisher.csproj | Removes the standalone publisher project (Job Monitor is now the sole host/consumer). |
| src/Microsoft.DotNet.Helix/AzureDevOpsTestPublisher/LocalTestResultsReader.cs | Removes the old DOM-based reader in favor of the new streaming reader in Job Monitor. |
| eng/common/core-templates/job/helix-job-monitor.yml | Adds pipeline-template parameter to flow upload parallelism into the monitor invocation. |
| Arcade.slnx | Removes the deleted AzureDevOpsTestPublisher project from the solution. |
| .gitignore | Un-ignores the new JobMonitor/TestResults directory (previously matched by MSTest TestResult*/ ignore). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| _options.UseFullyQualifiedTestName, | ||
| _options.TestResultAttachmentMode); | ||
| using var publisher = new AzureDevOpsResultPublisher( | ||
| var publisher = new AzureDevOpsResultPublisher( |
| IReadOnlyList<HelixJobInfo> jobsForFirstPoll = await ExecuteRetryPassAsync(cancellationToken); | ||
| return await RunPollLoopAsync(jobsForFirstPoll, cancellationToken); | ||
| } | ||
| catch (OperationCanceledException) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 51 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Microsoft.DotNet.Helix/JobMonitor/Interfaces/IHelixService.cs:36
- The XML doc comment for
DownloadTestResultsAsyncstill describes returning results for multiple work items and says work items may be omitted. The API now returns a singleWorkItemTestResults, so callers will always get an instance and should interpretTestResultFilesbeing empty as “no recognized results”. Updating the comment avoids misleading consumers and future maintainers.
/// Work items without recognizable test result files may be omitted from the result.
/// Individual file download failures should not prevent other result files from being downloaded.
/// </summary>
Task<WorkItemTestResults> DownloadTestResultsAsync(
string jobName,
Use Azure DevOps submitter job attempts to distinguish selective retries from full-stage reruns, preserve lineage metadata, and add scenario coverage for retry races and stage isolation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a3a2b2f-e044-4835-8a3c-0c99a6a29484
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 51 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/Microsoft.DotNet.Helix/JobMonitor/Interfaces/IHelixService.cs:34
- The XML doc for DownloadTestResultsAsync still describes returning metadata for multiple work items and says work items without recognizable result files may be omitted. The method now downloads results for a single work item and always returns a WorkItemTestResults (possibly with an empty TestResultFiles list), so the contract comment is misleading for implementers/callers.
/// Downloads test result files for a completed Helix job's work items
/// and returns metadata about each work item's results.
/// Work items without recognizable test result files may be omitted from the result.
/// Individual file download failures should not prevent other result files from being downloaded.
/// </summary>
Specify stream identities and timing-sensitive retry behavior, make phase-to-timeline matching explicit, and reject ambiguous fallback job identities. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a3a2b2f-e044-4835-8a3c-0c99a6a29484
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 51 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/Microsoft.DotNet.Helix/JobMonitor/Parallelism/ActionQueue.cs:88
EnqueueAsyncincrements_acceptedafter awaitingWriteAsync. Since workers can start processing as soon as the write completes,_startedcan be incremented before_accepted, again allowing transient negative queued counts. Increment_acceptedbefore writing and roll it back if the write fails/cancels.
public async ValueTask EnqueueAsync(T item, CancellationToken cancellationToken)
{
await _channel.Writer.WriteAsync(item, cancellationToken);
Interlocked.Increment(ref _accepted);
}
src/Microsoft.DotNet.Helix/JobMonitor/Parallelism/ActionQueue.cs:82
QueueSnapshot.Queuedis computed asAccepted - Started, butTryEnqueueincrements_acceptedafter the item is visible to readers. A worker can read and increment_startedbefore_acceptedis updated, briefly producing negative queued counts in snapshots/logging.
This issue also appears on line 84 of the same file.
public bool TryEnqueue(T item)
{
if (!_channel.Writer.TryWrite(item))
{
return false;
}
Interlocked.Increment(ref _accepted);
return true;
}
Keep previous-attempt results uploadable while excluding streams superseded by newer submitter attempts from current status and pass/fail reconciliation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 3a3a2b2f-e044-4835-8a3c-0c99a6a29484
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 51 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/Microsoft.DotNet.Helix/JobMonitor/Services/AzureDevOpsService.cs:620
- HonorRateLimitAsync both defers the shared rate-limit gate and then awaits it immediately. Because SendForStringAsync already runs inside an ExponentialRetry loop, this can cause rate-limit delay to be paid twice (once here, and again via the retry loop), increasing latency and reducing throughput. Deferring the gate without awaiting it here lets the retry loop’s delay “consume” part/all of the guidance and the next request’s pre-send WaitAsync will only wait any remaining time (effectively max(backoff, guidance) rather than sum).
if (delayToApply > TimeSpan.Zero)
{
_rateLimitGate.Defer(delayToApply);
_logger.LogDebug(
"Azure DevOps rate limit back-off. Delaying next request by {DelaySeconds:0.###}s (request: {RequestUri}).",
delayToApply.TotalSeconds,
requestUri);
await _rateLimitGate.WaitAsync(cancellationToken);
}
src/Microsoft.DotNet.Helix/JobMonitor/Services/HelixService.cs:89
- The Helix client call to ListFilesAsync isn’t passed the method CancellationToken, so cancellation won’t stop this request (it will use the default CancellationToken.None). This can delay shutdown and interfere with the monitor’s timeout/cancel semantics.
JobResultsUri resultsUri = await RetryAsync(() => _helixApi.Job.ResultsAsync(jobName), cancellationToken);
IImmutableList<UploadedFile> availableFiles = await RetryAsync(
() => _helixApi.WorkItem.ListFilesAsync(workItemName, jobName, false),
cancellationToken);
Summary
Completely rewrite the Helix Job Monitor around a globally bounded, non-blocking result-processing pipeline while preserving its existing retry, replay, stage-attempt, upload, and pass/fail semantics.
The rewrite is designed for hundreds of Helix jobs, thousands of work items, and millions of test results:
Semantic behavior
Result processing
.txttransport suffix, including*.testResults.xml.txtand.trx.txt.Parallelism and performance
Representative full runtime runs:
At 48 workers, server-directed deferrals dropped from 1,459 to 1,080 and shared-gate waits dropped from 2,429 to 2,061 while keeping final drain below 2%.
Logging and diagnostics
Design documentation
The new design set lives under
src/Microsoft.DotNet.Helix/JobMonitor/Design:SemanticBehavior.mdArchitecture.mdComponents/Polling.mdComponents/UploadPipeline.mdComponents/TestResults.mdComponents/StateAndStatus.mdComponents/PerformanceMetrics.mdComponents/Shutdown.mdValidation
Microsoft.DotNet.Helix.Sdk.Teststests pass..xml.txtdiscovery, and independent same-queue/phase work-item accounting.The PR is intentionally draft while the final package validation run completes.