Skip to content

Fix payload retries and entity cleanup after storage failures - #816

Draft
wangbill (YunchuWang) wants to merge 2 commits into
mainfrom
yunchuwang-public-sdk-payload-fixes
Draft

wangbill (YunchuWang) wants to merge 2 commits into
mainfrom
yunchuwang-public-sdk-payload-fixes

Conversation

@YunchuWang

Copy link
Copy Markdown
Member

Summary

What changed?

  • Keep recognized payload-store tokens unchanged on repeated externalization, including batched completion fields and rewind history.
  • Prepare native orchestration payloads before terminal entity cleanup and chunking. Preserve existing entity release actions and reuse the worker's outstanding-unlock cleanup when a permanent storage failure turns a pending response into a failed completion.
  • Add completion-retry and entity-lock regression coverage, and update CHANGELOG.md.

Why is this change needed?

  • Completion retries reuse mutable protobuf responses. When a recognized token exceeds the configured externalization threshold, uploading it again creates a nested reference instead of preserving the original payload.
  • A permanent orchestration payload failure previously cleared all actions, including entity releases. If the orchestration was still waiting on an entity call, terminal cleanup had not run at all. Both cases can leave entities locked after the orchestration fails.

Issues / work items

  • Resolves: N/A; independently reproduced against the public SDK.
  • Related: N/A.

Project checklist

  • Release notes are not required for the next release
    • Otherwise: Notes added to release_notes.md
  • Backport is not required
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • All required tests have been added/updated (unit tests, E2E tests)
  • Breaking change?
    • If yes:
      • Impact: N/A.
      • Migration guidance: N/A.

AI-assisted code disclosure (required)

Was an AI tool used? (select one)

  • No
  • Yes, AI helped write parts of this PR (e.g., GitHub Copilot)
  • Yes, an AI agent generated most of this PR

If AI was used:

  • Tool(s): GitHub Copilot.
  • AI-assisted areas/files: Payload interceptors, worker preparation/cleanup wiring, shared entity conversion, regression tests, and the changelog entry.
  • What you changed after AI output: Iterated on native replay coverage, restored unrelated original file bytes, and removed new helper-visibility/member-order warnings while preserving the existing compiler compatibility types.

AI verification (required if AI was used):

  • I understand the code and can explain it
  • I verified referenced APIs/types exist and are correct
  • I reviewed edge cases/failure paths (timeouts, retries, cancellation, exceptions)
  • I reviewed concurrency/async behavior
  • I checked for unintended breaking or behavior changes

Human attestations and the backport decision are intentionally left for review.


Testing

Automated tests

  • Result: Passed.
dotnet test .\test\Extensions\AzureBlobPayloads.Tests\AzureBlobPayloads.Tests.csproj --configuration Release --no-restore --logger "console;verbosity=minimal"
# Passed: 229, Failed: 0, Skipped: 0.

dotnet test .\test\Grpc.IntegrationTests\Grpc.IntegrationTests.csproj --configuration Release --no-restore --filter "FullyQualifiedName~AzureBlobPayloadsSideCarInterceptorTests|FullyQualifiedName~LargePayloadTests|FullyQualifiedName~AutochunkTests" --logger "console;verbosity=minimal"
# Passed: 64, Failed: 0, Skipped: 0.

dotnet test .\test\Worker\Grpc.Tests\Worker.Grpc.Tests.csproj --configuration Release --no-restore --filter "FullyQualifiedName~PayloadInterceptorTests|FullyQualifiedName~GrpcDurableTaskWorkerTests|FullyQualifiedName~ExecuteWithRetryTests|FullyQualifiedName~GrpcOrchestrationRunnerTests" --logger "console;verbosity=minimal"
# Passed: 59, Failed: 0, Skipped: 0.

dotnet build .\src\Extensions\AzureBlobPayloads\AzureBlobPayloads.csproj --configuration Release --no-restore --no-incremental -p:BuildProjectReferences=false --verbosity quiet
# Passed for netstandard2.0, net6.0, net8.0, and net10.0; 0 errors.
# Four existing SA1204 warnings remain, one per target framework.

git diff --check origin/main...HEAD
# Passed.

Manual validation (only if runtime/behavior changed)

  • Environment (OS, .NET version, components): Windows, .NET SDK 10.0.402 / runtime 10.0.13; local HTTP/2 in-memory sidecar and native worker replay harness.
  • Steps + observed results: The following reproductions are automated; no manual cloud validation was performed.
    1. Fail the first activity or orchestration completion after payload externalization. The retry preserves the same long blob:v2: token, does not upload a token, and returns the original output/custom status.
    2. Replay an orchestration that acquired two entity locks, then exceed the payload cap in its result or a pending entity call. Both inline and streamed history produce two unlocks before one failed completion, without emitting chunk fragments.
    3. Confirm already-emitted typed and legacy entity releases survive a permanent payload failure, while unrelated actions are discarded.
  • Evidence (optional): These regressions failed before the relevant fixes. Hosted CI and real Azure Storage/service validation have not run locally.

Notes for reviewers

  • No breaking change: public/protected signatures, protobuf fields, converter defaults, and successful replay behavior are unchanged. The public record's assembly-local IsExternalInit modifier is retained on .NET Standard.
  • The preparation callback is internal and shares the registered interceptor. The second unary pass skips already-known tokens. Duplicate validation helpers are excluded rather than suppressing compiler warnings or exposing a public preparation API.
  • Completion RPCs are unary. Current server streams deliver work items and history with payload-free request messages, so this PR does not add a generic outbound-streaming adapter.
  • Release notes are in the existing CHANGELOG.md; this repository does not currently have the template's release_notes.md.

wangbill (YunchuWang) and others added 2 commits October 16, 2026 14:58
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
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.

1 participant