Cache Azure Blob container initialization to avoid per-upload CreateIfNotExistsAsync

[berndverst]

Summary

BlobPayloadStore.UploadAsync previously called CreateIfNotExistsAsync before every upload, adding an unnecessary storage transaction after the payload container was already known to exist.

This change caches a successful container-initialization generation for each BlobPayloadStore instance. Existing DI registrations create the payload store as a singleton, so the cached state is reused across uploads.

Implementation

• Steady-state fast path: a volatile read returns the cached generation and skips CreateIfNotExistsAsync.
• Concurrency-safe first use: when no generation is cached, callers enter a SemaphoreSlim asynchronously, double-check the cache, and only the lock holder calls CreateIfNotExistsAsync.
• Caller-owned cancellation: the caller token is used both while waiting for the semaphore and by the Azure Storage create request. Cancelling one caller does not cancel another; a waiting caller can retry after a cancelled initializer releases the gate.
• Retriable initialization failures: failed or cancelled initialization publishes no generation, so a later caller performs a fresh attempt without explicit fault eviction or a shared background task.
• Generation-safe deletion recovery: each successful initialization publishes a unique marker. If a write returns ContainerNotFound, the upload clears the cache only when it still contains the marker that upload used, then retries once. A stale failure cannot invalidate a newer initialization.

If Azure is still processing a container deletion, the retrying create can return 409 ContainerBeingDeleted; that error propagates until deletion settles, matching the behavior before initialization was cached.

No public API changes are introduced. Main's self-describing v2 payload tokens and cross-account read behavior remain unchanged.

Testing

• Focused unit coverage for concurrent first use, initialization failure retry, lock-holder cancellation, waiting-caller cancellation, settled deletion recovery, unrelated storage errors, and stale-generation invalidation.
• AzureBlobPayloads.Tests: 7/7 passing, including with DOTNET_PROCESSOR_COUNT=2.
• Existing v2 token coverage in Grpc.IntegrationTests: 6/6 passing.
• AzureBlobPayloads.csproj builds for netstandard2.0, net6.0, net8.0, and net10.0.
• Direct Azurite smoke verification: two uploads and downloads round-tripped successfully with exactly one container-create request in the request log.
• The full LargePayloadConsoleApp DTS scenario was not run because DURABLE_TASK_SCHEDULER_CONNECTION_STRING was not available in this environment.

This change remains independent of other performance work.

Fixes #771

[Copilot]

Pull request overview

This PR improves the Azure Blob payload externalization path by eliminating a per-upload CreateIfNotExistsAsync call, replacing it with a cached “single-flight” container initialization so repeated uploads avoid an extra storage transaction. It also adds a dedicated unit test project to validate initialization caching, cancellation isolation, failure retry, and cache reset behavior.

Changes:

• Cache container initialization in BlobPayloadStore to avoid calling CreateIfNotExistsAsync on every upload.
• Reset the cached initialization state when a write fails with ContainerNotFound to enable recovery after out-of-band container deletion.
• Add a new AzureBlobPayloads.Tests project with focused unit tests for concurrency, failure, cancellation, and error propagation scenarios.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/Extensions/AzureBlobPayloads/PayloadStore/BlobPayloadStore.cs	Adds cached/single-flight container initialization and cache reset on ContainerNotFound.
test/Extensions/AzureBlobPayloads.Tests/BlobPayloadStoreTests.cs	Introduces unit tests for initialization caching behavior and failure/cancellation scenarios.
test/Extensions/AzureBlobPayloads.Tests/Usings.cs	Adds global test usings for FluentAssertions/Moq/Xunit.
test/Extensions/AzureBlobPayloads.Tests/AzureBlobPayloads.Tests.csproj	Adds new test project referencing Azure Blob payloads extension.
Microsoft.DurableTask.sln	Wires the new test project into the solution under an Extensions folder.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/Extensions/AzureBlobPayloads/PayloadStore/BlobPayloadStore.cs:236

• In the non-NETSTANDARD2_0 branch, awaiting initializationTask.WaitAsync(cancellationToken) can throw OperationCanceledException without ever observing the underlying initializationTask. If all callers cancel before the shared initialization later faults, the fault may go unobserved and can surface via TaskScheduler.UnobservedTaskException on finalization. Consider mirroring the NETSTANDARD2_0 cancellation path by attaching a fault-observing continuation when cancellation happens first.

        await initializationTask.WaitAsync(cancellationToken).ConfigureAwait(false);
#endif

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[YunchuWang]

Verified locally on this branch: builds clean, 9/9 tests pass, no new analyzer warnings on BlobPayloadStore.cs, and the modified .sln restores cleanly (project GUID, all six configuration mappings, and the nested-project entries are all correct).

The optimization is right and the win is real. On a worker running the default MaximumConcurrentActivityWorkItems = 100 * ProcessorCount, this roughly halves requests on the upload path — and more importantly, a fan-out orchestrator reply that externalizes 500 action inputs goes from 500 redundant container calls to zero, since ExternalizeRequestPayloadsAsync walks actions sequentially with no Task.WhenAll. That is the #771 case and it needs no concurrency at all to hurt.

Things I specifically checked and found correct:

• Publish-before-start — PublishNewInitializer completes the CompareExchange before anything touches .Value, with LazyThreadSafetyMode.ExecutionAndPublication as a backstop. This properly fixes the original "an async method starts on call, not on await" flaw.
• Generation-precise invalidation — using the initializer instance as the CAS comparand in both the ContainerNotFound handler and the failure self-heal. I traced racing publishers, stale invalidation, and the fault self-heal path and could not construct an interleaving that produces a redundant create or clobbers a newer initializer.
• Retry scope — the try covers OpenWriteAsync, the writes, FlushAsync and the using disposal, so ContainerNotFound is caught wherever the SDK surfaces it, and retryAfterContainerNotFound correctly bounds it to one attempt.
• Singleton registration — AddSingleton<PayloadStore> in both DI extensions, so the cache is actually effective. Worth noting this is a new, unstated coupling: BlobPayloadStore is now stateful and only pays off as a singleton.
• DownloadAsync / IsKnownPayloadToken — untouched, correctly so.

One item below (the uncancellable initializer) I would like addressed before merge. The design comment is a recommendation rather than a blocker — I have included the places where the current approach is genuinely better than the alternative I sketch.

[YunchuWang]

Follow-up: end-to-end verification against real storage

Separate from the inline comments — this is about test kind rather than test content, so it did not fit on a line.

As far as I can tell from the PR description and discussion, this has not been exercised against real (or emulated) storage. The Testing section lists focused unit coverage, and the strongest runtime claim is that the project builds across all target frameworks. CI (validate-build.yml) runs dotnet test on windows-latest with no Azurite service and no storage configuration, so there is no integration coverage there either. Apologies if I have missed a manual run that happened outside the PR — if so, please ignore this.

Worth noting this is also the first test project this extension has ever had, which is a genuine improvement. The observation below is only that all eight tests use Mock<BlobContainerClient> and assert call counts on it — which validates the coordination logic well (I traced it and could not break it), but by construction cannot validate anything where the mock's behavior diverges from real Azure.

One concrete instance of that divergence

UploadAsync_ContainerDeletedAfterInitialization_ResetsCacheAndRecreatesContainer passes because the mocked re-create returns success immediately. Real Azure does not behave that way.

From the Delete Container REST docs:

When a container is deleted, a container with the same name can't be created for at least 30 seconds... While the container is being deleted, attempts to create a container of the same name fail with status code 409 (Conflict).

And BlobContainerClient.CreateIfNotExistsInternal in Azure.Storage.Blobs 12.27.0 (the version pinned in Directory.Packages.props) swallows only one error code:

catch (RequestFailedException storageRequestFailedException)
when (storageRequestFailedException.ErrorCode == BlobErrorCode.ContainerAlreadyExists)

ContainerBeingDeleted is not swallowed, and 409 is not in Azure.Core's retriable set (408/429/5xx). So against real storage:

container deleted by operator
  -> OpenWriteAsync            -> 404 ContainerNotFound
  -> cache invalidated, retryAfterContainerNotFound = false, continue
  -> CreateIfNotExistsAsync    -> 409 ContainerBeingDeleted     <-- mock returns success here
  -> not swallowed by the SDK, not retried by the pipeline
  -> throws, and the one-shot retry budget is already spent -> upload fails

To be fair about what this is and is not:

• Not a regression. Before this PR every upload called CreateIfNotExists, so it would hit the same 409. Behavior really is preserved, as the description says.
• It does work once the deletion has fully settled (roughly 30s+), which is the path the unit test models.
• The gap is that the commit is titled "Retry upload after container deletion" and the description says it "recreates the container, and retries that same upload once" — and in the timing most likely to occur in practice (operator deletes the container, traffic arrives immediately after) it will not. A mock cannot distinguish those two worlds.

I am not suggesting this needs to be fixed here — handling ContainerBeingDeleted properly means waiting out a 30s window, which is arguably out of scope. Mostly I would not want the test name and commit title to imply a guarantee that does not hold, so a comment noting the limitation would be enough.

Two smaller gaps

• The central premise — container already exists, so skipping CreateIfNotExists still leaves OpenWriteAsync working — is the entire point of the change and is only ever exercised against a mock.
• The request-count reduction that motivates the PR has not been measured; it currently exists as an argument rather than an observation.

Why this is a cheap ask

samples/LargePayloadConsoleApp already exists and is already in the solution, and is more or less exactly this harness — its README describes externalizing large payloads to blob storage over Durable Task Scheduler with no sidecar, defaulting to Azurite. It sets ThresholdBytes = 1024 so everything externalizes, and covers large orchestration input, activity I/O, sub-orchestrations, external events, custom status, entity input/output/state, oversized rejection, and ThreeLargeActivities — three parallel 13MB activities, which is the fan-out shape this PR is optimizing.

dotnet run against Azurite plus a DTS connection string is maybe ten minutes. Given this extension ships against real DTS and real storage, and this PR changes the write path for every externalized payload, that seems proportionate — and it would let you attach a measured before/after transaction count, which would make the case for the change stronger than any of the unit tests can.

[berndverst]

Thanks for calling this out. I made the deletion limitation explicit in both the focused test and PR description: the recovery test models re-creation after deletion has settled, while an immediate real-Azure retry can surface 409 ContainerBeingDeleted and still propagates exactly as it did before caching. I also renamed the test accordingly.

I could not run LargePayloadConsoleApp end to end because this environment has no DURABLE_TASK_SCHEDULER_CONNECTION_STRING. I did run the final merged BlobPayloadStore directly against Azurite: two uploads and two downloads round-tripped successfully, and the Azurite request log recorded exactly one PUT ...?restype=container request. That verifies both the steady-state skip against an emulator and the request-count reduction on the changed storage path. The PR testing section now records both that result and the DTS prerequisite limitation.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Extensions/AzureBlobPayloads/PayloadStore/BlobPayloadStore.cs:94

• The internal test-only constructor initializes clientOptions with default settings, but the public constructor configures important retry/network-timeout settings. Since clientOptions is used (e.g., for cross-account reads in DownloadAsync), tests using this constructor can exercise different behavior than production, and future tests may miss issues tied to retry configuration.

    {
        this.options = options ?? throw new ArgumentNullException(nameof(options));
        this.containerClient = containerClient ?? throw new ArgumentNullException(nameof(containerClient));
        this.clientOptions = new BlobClientOptions();
    }