Repository navigation
Fix payload retries and entity cleanup after storage failures - #816
Open
wangbill (YunchuWang) wants to merge 6 commits into
Open
wangbill (YunchuWang) wants to merge 6 commits into
wangbill (YunchuWang) wants to merge 6 commits into
Conversation
Skip recognized payload tokens before size checks so completion retries remain idempotent. Prepare orchestration payloads before terminal cleanup and chunking, preserve entity release actions, and reuse outstanding unlock obligations. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 78dff439-4d84-4d70-921c-f57421cbc383
Reuse the worker's friend-visible validation helpers without duplicating Check and Verify. Retain assembly-local compiler compatibility types for the public record, order the internal preparation member consistently, and remove changelog whitespace. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 78dff439-4d84-4d70-921c-f57421cbc383
This was referenced Oct 19, 2026
Open
| // the orchestration terminates without leaving its entities locked. | ||
| P.OrchestratorAction[] unlocks = response.Actions.Where(action => | ||
| action.SendEntityMessage?.EntityUnlockSent is not null | ||
| || (action.SendEvent is { Name: "release", Instance: { } instance } |
Member
Author
There was a problem hiding this comment.
why need this check, and isentityinstance check, seem unnccessary
Restore the original fake payload tokens, consolidate terminal unlock synthesis after response preparation, and cover real 403 uploads plus normal entity cleanup. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Replace production friend access with a narrow infrastructure callback setter and restore the Blob extension's own shared helpers. Preserve preparation and cleanup ordering, and cover the configuration contract and named registration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment on lines
+46
to
+48
| P.OrchestratorAction[] unlocks = response.Actions.Where(action => | ||
| action.SendEntityMessage?.EntityUnlockSent is not null | ||
| || (action.SendEvent is { Name: "release", Instance: { } instance } |
Restore converter-side missing entity unlocks with the shared helper while retaining the processor's post-preparation fallback. Cover standalone finalization, null and disabled state, and idempotent repeated cleanup without changing the payload architecture. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
wangbill (YunchuWang)
marked this pull request as ready for review
October 27, 2026 17:08
wangbill (YunchuWang)
requested review from
halspang and
sophiatev
and
a balanced review from Copilot
October 27, 2026 17:08
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Replay-sensitive entity cleanup and completion ordering warrant maintainer review, particularly without reported scheduler-backed validation.
No open issues 🎉
What changed in this PR
This PR fixes payload retry handling and entity-lock cleanup in the Durable Task SDK’s gRPC worker and Azure Blob payload extension.
Changes:
- Preserve recognized payload tokens during repeated externalization.
- Prepare orchestration payloads before final cleanup and chunking, preserving entity releases when storage failures force completion.
- Add an infrastructure callback and regression coverage for retries, registration, and cleanup.
| File | Description |
|---|---|
test/Worker/Grpc.Tests/GrpcDurableTaskWorkerOptionsInternalTests.cs |
Tests callback defaults, registration, and null validation. |
test/Grpc.IntegrationTests/LargePayloadTests.cs |
Tests stable payload tokens across completion retries. |
test/Grpc.IntegrationTests/AzureBlobPayloadsSideCarInterceptorTests.cs |
Covers repeated externalization and preserved entity releases. |
test/Extensions/AzureBlobPayloads.Tests/OrchestratorPayloadFailureTests.cs |
Adds worker replay tests for storage failures and entity cleanup. |
test/Extensions/AzureBlobPayloads.Tests/DependencyInjection/UseExternalizedPayloadsTests.cs |
Verifies preparation registration and named-worker isolation. |
test/Client/Grpc.Tests/ProtoUtilsTests.cs |
Tests complete, idempotent converter cleanup. |
src/Worker/Grpc/Internal/InternalOptionsExtensions.cs |
Adds the response-preparation infrastructure API. |
src/Worker/Grpc/GrpcDurableTaskWorkerOptions.cs |
Stores the optional preparation callback. |
src/Worker/Grpc/GrpcDurableTaskWorker.Processor.cs |
Runs preparation before fallback cleanup and chunking. |
src/Shared/Grpc/ProtoUtils.cs |
Extracts reusable cleanup while preserving converter finalization. |
src/Extensions/AzureBlobPayloads/Interceptors/PayloadInterceptor.cs |
Skips externalization for recognized tokens. |
src/Extensions/AzureBlobPayloads/Interceptors/AzureBlobPayloadsSideCarInterceptor.cs |
Preserves entity releases during permanent-failure conversion. |
src/Extensions/AzureBlobPayloads/DependencyInjection/DurableTaskWorkerBuilderExtensions.AzureBlobPayloads.cs |
Registers preparation using the shared interceptor instance. |
CHANGELOG.md |
Documents retry fixes, cleanup behavior, and the infrastructure API. |
Review effort: Balanced
Preserve payload preparation and entity cleanup alongside main's purge contract and entity tags. Keep the two PR notes under Unreleased and retain the current release history. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Cleanup spans replay obligations, storage-failure response replacement, and scheduler commit ordering, while the regression harness does not verify actual lock release, warranting maintainer validation.
No open issues 🎉
Review effort: Balanced
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What changed?
InternalOptionsExtensions.SetOrchestratorResponsePreparationin the worker's existing.Internalnamespace, instead of granting the production Blob assembly access to worker internals.CHANGELOG.md.Why is this change needed?
Issues / work items
Project checklist
release_notes.mdAI-assisted code disclosure (required)
Was an AI tool used? (select one)
If AI was used:
AI verification (required if AI was used):
Human attestations and the backport decision are intentionally left for review.
Testing
Automated tests
Earlier recorded validation
Earlier infrastructure-registration validation
Earlier infrastructure-registration metadata inspection also confirmed local
Check/Verifydefinitions in all four extension DLLs. Onnetstandard2.0, the localIsExternalInitremains internal and the publicBlobPurgeOutcomeinit setter retains its local required modifier. No dependency restore or installation was needed for that earlier follow-up.Converter-invariant follow-up
The enabled direct-converter regression reproduced a single completion with two missing unlocks before restoration. All 146 focused cases now pass, including enabled/null/disabled direct conversion and all unchanged 12 native cases. This converter-only follow-up did not rerun the full solution or the earlier multi-TFM/metadata checks. Original mixed bytes outside the four restored converter lines are preserved.
Manual validation (only if runtime/behavior changed)
blob:v1:fake token above the 16-byte threshold, does not upload a token, and returns the original output/custom status.UploadAsyncwithRequestFailedException(403), for a returning orchestration or a pending entity call. Inline and streamed histories both preserve the completion token and produce two targeted unlocks before one failed completion, without business actions or chunk fragments. Cap cases perform no upload; 403 cases perform exactly one.Notes for reviewers
Microsoft.DurableTask.Worker.Grpc.Internal; existing public/protected signatures, protobuf fields, converter defaults, and successful replay behavior are unchanged. The method has the same infrastructure compatibility caveats as neighboring configuration methods, not a new general-purpose public options property.Func<OrchestratorResponse, CancellationToken, Task>, with no Blob types or reverse dependency. The option remains internal, and the Blob implementation method remains internal. Registration shares the existing interceptor. Native processing remains convert/finalize -> prepare payload -> idempotent fallback cleanup -> chunk/send. The second unary pass still skips known tokens.EntityConversionState.InsertMissingUnlockshelper. The converter returns a complete terminal response independently; the fallback covers direct terminal responses and pending responses made terminal by payload failure.ResetObligationsdrains synthesized obligations, explicitly emitted releases remove their own obligations, and permanent-failure conversion preserves existing release actions, preventing duplicate unlocks.SharedSection Core, and all local compiler compatibility types are retained; both project files are back to their pre-PR contents. The extension compiles its ownCheck/Verify, and .NET Standard public-record metadata is preserved.CHANGELOG.md; this repository does not currently have the template'srelease_notes.md.