Skip to content

perf(journaling): benchmark Azure storage providers - #11261

Merged
ReubenBond merged 8 commits into
dotnet:mainfrom
ReubenBond:rb-perf-journaling-azure-provider-benchmark
Sep 16, 2026
Merged

ReubenBond merged 8 commits into
dotnet:mainfrom
ReubenBond:rb-perf-journaling-azure-provider-benchmark

Conversation

@ReubenBond

@ReubenBond ReubenBond commented Sep 15, 2026 •

Copy link
Copy Markdown
Member

Problem

Azure journaling needs repeatable provider-level measurements for durable append, checkpoint replacement, recovery, and catalog discovery, alongside the existing end-to-end durable-job workload. Account tier, seeded history, listing population, concurrency, and measurement boundaries need to be explicit so comparisons remain meaningful.

Solution

  • Add Journaling.Azure to the existing benchmark project: bounded fixed-work workers, independent single-writer journals, deterministic payloads, verified recovery and exact catalog membership, per-operation latency distributions, throughput, and JSON/CSV reports. Setup, warmup, verification, cleanup, and export have separate measurement boundaries.
  • Compare Azurite Blob/Table, standard Blob, premium BlockBlobStorage accounts, and Azure Table. Standard and premium execute the same append-blob WAL/block-blob checkpoint provider. Real Azure requires explicit opt-in and environment-based Entra authentication; Blob account kind/SKU is data-plane verified and Table tier is labeled unverified. Account validation uses the Blob SDK's typed AccountKind and SkuName values, with regression coverage for supported tiers and rejected mismatches. The standalone runner records the existing account's SKU; operators match redundancy when comparing storage tiers.
  • Capture the provider's exact DI meter for catalog pages, candidate items, delivered entries, and explicit retries. The listener is active only during fixed-work collection windows and detaches when workers finish. Operation latency, outcomes, throughput, and payload bytes come from the runner's own samples. Host-owned SDK diagnostics, Aspire integrations, and Azure service metrics provide request-level timing, counts, and cost accounting.
  • Use report schema version 2 for the catalog-only metric rows: instrument, unit, provider, retry reason, observation count, and integer sum. Historical reports retain their original schema and measured build. Regression tests exercise actual Blob/Table provider catalog emission through fake service pages, including empty pages and local filtering, alongside exact-meter/window isolation, listener enabled states, and export precision.
  • Record loaded benchmark, Journaling, and Azure Storage assembly source revisions and module version IDs in JSON Build, CSV build_json, and BDN global-setup logs. Sanitized revision metadata and compiler-generated module IDs identify the binaries being measured independently of the current checkout.
  • Add a bounded BDN adapter with one invocation per iteration, up to three measured iterations, one launch, fresh baseline resources, and synchronous setup/cleanup bridges. A critical validator checks the final resolved job limits before resource setup. BDN leaves provider metric collection inactive, keeping discarded metric callbacks out of its timings. Preserve existing codec and grain-storage dispatch.
  • Wire playground backend selection, preserve Azure as the premium alias, provide matched standard/premium LRS presets, keep clustering on an appropriate Table service, and use catalog-compatible names in GUID-scoped run resources. Preserve scoped Blob/Table role defaults in published manifests, including separate premium journal and clustering accounts. Add public UseAzureTableDurableJobs overloads through the existing journaled-shard registration, with regenerated API surface and package metadata covering both journal backends.

Resource ownership requires positively acknowledged creation. A failed create, including an ambiguous SDK-retry 409, retains creation-unconfirmed / manual-check-required and never authorizes deletion. Worker failures and cancellation retain partial results and nonzero exit status. Cleanup unwinds attempted lifecycle stages after failed or cancelled startup before owned-resource deletion and service disposal.

Review scope

Rebased directly onto main at 9222be9a19196335e129e211718619f47bf73adc. The catalog, discovery, and finalized catalog-metrics prerequisites have landed through #11258, #11257, and #11259. This PR now contains only the benchmark/playground work and its integration with the merged metric contract.

The original four benchmark commits replay without patch changes. Focused follow-ups align metric capture and reports with the finalized catalog counters, add measured-build provenance, scope listener activation, and enforce setup/execution safety. The payload and recovery workloads remain unchanged.

Review the benchmark changes against main.

The benchmark fixtures use fixed ASCII identifier/separator positions, an explicit catalog/ prefix, and isolated playground namespaces. HNS edge ordering and arbitrary identifier alphabets have dedicated provider correctness scenarios.

Usage, workload/report units, cost considerations, and cleanup procedures are documented in test/Benchmarks/Journaling/Azure/README.md and playground/DurableJobsJournaling/README.md. New benchmark artifacts include their loaded assembly build identities and configuration. Historical results retain their original provenance; the rebase and review updates do not represent a new performance measurement.

Microsoft Reviewers: Open in CodeFlow

Copilot AI lite review requested due to automatic review settings September 15, 2026 18:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@github-actions

github-actions Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

Code coverage

Metric Pull request
Lines 82.03% (111,932 / 136,459)
Branches 71.20% (32,166 / 45,180)

Report-only conclusion: current-main baseline stale.

The newest successful coverage run tested 9222be9, not current main b92f08a.

Coverage combines every CI test matrix job, including providers, CodeGen, .NET 8/10, Linux, Windows, and macOS, using canonical physical source and branch identities.

The comparison remains report-only while normal line and branch variance is calibrated.

Coverage details

Copilot AI review requested due to automatic review settings September 15, 2026 18:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved critical compatibility and compilation findings, plus moderate correctness and benchmark issues, block approval.

Get a fresh assessment by requesting another Copilot review.

Review tier: Lite
Findings: 6 High severity · 1 Medium severity

Open (7)

Comment thread src/AWS/Orleans.Journaling.S3/S3JournalStorageOptions.cs
Comment thread src/AWS/Orleans.Journaling.S3/S3JournalStorageProvider.cs
Comment thread src/Azure/Orleans.Journaling.AzureStorage/AzureBlobJournalStorageProvider.cs Outdated
Comment thread src/Redis/Orleans.Journaling.Redis/RedisJournalStorage.cs Outdated
Comment thread test/Benchmarks/Journaling/Azure/AzureJournalScenario.cs
Copilot AI review requested due to automatic review settings September 15, 2026 19:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Unresolved benchmark resource-bound issues remain, and the broad provider changes require final human review.

Review tier: Lite
Findings: None

Resolved since last review (7)

Copilot AI review requested due to automatic review settings September 15, 2026 22:03
@ReubenBond
ReubenBond force-pushed the rb-perf-journaling-azure-provider-benchmark branch from f773d38 to e030252 Compare September 15, 2026 22:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Three moderate findings remain unresolved around report provenance, cleanup-failure preservation, and deterministic metric ordering.

Get a fresh assessment by requesting another Copilot review.

Review tier: Lite
Findings: 1 Medium severity

Open (1)

Comment thread test/Benchmarks/Journaling/Azure/ProviderMetrics.cs
Copilot AI review requested due to automatic review settings September 16, 2026 03:55
@ReubenBond
ReubenBond force-pushed the rb-perf-journaling-azure-provider-benchmark branch from e030252 to 9ae0117 Compare September 16, 2026 03:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Unresolved critical correctness/compilation findings and a moderate input-validation defect remain.

Get a fresh assessment by requesting another Copilot review.

Review tier: Lite
Findings: 2 High severity · 1 Low severity

Open (3)
Resolved since last review (1)

Comment thread test/Benchmarks/Journaling/Azure/AzureJournalScenario.cs
Comment thread test/Benchmarks/Journaling/Azure/AzureJournalScenario.cs
Comment thread test/Benchmarks/Journaling/Azure/AzureJournalReport.cs
Copilot AI review requested due to automatic review settings September 16, 2026 05:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The BDN path still incurs metric-listener overhead without producing metrics; package metadata also needs updating.

Review tier: Lite
Findings: None

Resolved since last review (3)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Disable the provider metric listener during BDN timings

test/​Benchmarks/​Journaling/​Azure/​AzureJournalScenario.cs:123

ProviderMetrics starts its MeterListener in the constructor, which enables the catalog/retry counters even while _recording is false. The BDN adapter constructs this scenario but never calls StartMetrics, so every catalog page/item/entry or retry during the timed Execute still invokes Record (including key construction and the lock) and then discards the value. This adds benchmark-only listener overhead while producing no BDN metrics; construct the scenario with metric collection disabled for BDN, or otherwise keep these instruments disabled for the BDN measurement path.

Copilot AI review requested due to automatic review settings September 16, 2026 06:30
@ReubenBond

Copy link
Copy Markdown
Member Author

Addressed the remaining items in the latest review summary in 1aae3e8.

  • ProviderMetrics now creates and starts its MeterListener only when Start() opens a collection window. Stop()/Dispose() detach it. BDN never starts collection, so its catalog/retry counters remain disabled by this collector and produce no discarded callbacks or aggregation overhead. Fixed-work runs continue to collect metrics only while measured workers run.
  • Updated the NuGet package description to cover Azure Blob and Azure Table journal storage, and verified the produced .nuspec.

MetricsEnableCountersOnlyDuringExplicitCollectionWindows reproduced the issue before the fix and now verifies pre-start, active, stopped, restarted, late-published, and disposed instrument states. MetricsCaptureActualAzureCatalogPagesCandidatesAndEntries also checks disabled/enabled states around real Blob/Table provider catalog enumeration using fake service pages. All 62 focused benchmark tests pass on each of net8.0 and net10.0; the normal Release pack passes. No Azure workloads were run, and historical results are unchanged.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Three unresolved findings remain around role cleanup, BDN iteration bounds, and partial lifecycle cleanup.

Review tier: Lite
Findings: None

Copilot AI review requested due to automatic review settings September 16, 2026 15:14
@ReubenBond

Copy link
Copy Markdown
Member Author

Addressed all three concerns from the review summary in 2d5006d.

  • Role cleanup: publish mode previously removed all default assignments. It now replaces them with the services each storage account needs: Blob + Table Data Contributor for the shared standard Blob/clustering account, Table-only for Table journals, and separate Blob-only/Table-only assignments for premium journal/clustering accounts. This uses Aspire resource defaults, preserving generic manifest publishing and its intended-principal inputs without requiring a new compute deployment environment. Offline manifests for all six backend spellings contain the expected role modules and exact role IDs; unnecessary Queue roles and unsupported premium endpoints are excluded. No Azure permissions were changed.
  • BDN bounds: added a critical validator over the final resolved benchmark jobs and an explicit one-launch default. Validation requires 1-3 measured iterations, 0-1 warmup iterations, one invocation, unroll factor 1, and one launch before resource setup. Tests exercise default/Dry/Short/Medium/Long presets, CLI overrides, and rejection of unsafe or adaptive final jobs. The existing per-iteration invocation guard remains.
  • Partial lifecycle cleanup: cleanup now calls OnStop on an assigned lifecycle even when OnStart failed or was cancelled. Orleans already tracks the highest attempted stage and unwinds in reverse order. Cleanup detaches the lifecycle once, then continues to owned-resource cleanup and DI disposal. Tests verify failed and cancelled startup stop the failed stage and earlier stages, skip unreached stages, dispose once, and preserve the startup failure.

All 80 focused tests passed on each of net8.0 and net10.0. The AppHost Release build and six offline publish-manifest checks passed. No real Azure or emulator workloads were run, and historical benchmark artifacts remain unchanged.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The broad benchmark, storage-provider, API, and provisioning changes warrant final human review.

Review tier: Lite
Findings: None

@ReubenBond
ReubenBond merged commit 7193f06 into dotnet:main Sep 16, 2026
73 checks passed
@ReubenBond
ReubenBond deleted the rb-perf-journaling-azure-provider-benchmark branch September 16, 2026 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants