镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Add orchestration restart options and source instance metadata - #833

Open
Varshitha Bachu (bachuv) wants to merge 2 commits into
mainfrom
vabachu/net-sdk-orchestration-cloning
Open

Varshitha Bachu (bachuv) wants to merge 2 commits into
mainfrom
vabachu/net-sdk-orchestration-cloning

Conversation

@bachuv

@bachuv Varshitha Bachu (bachuv) commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

What changed?

  • Add RestartAsync(string instanceId, string newInstanceId, string? version, CancellationToken cancellation) with four required arguments. The existing bool overload remains unchanged, including calls using default.
  • Use the separate CloneInstance RPC for explicit target IDs and version overrides. The existing RestartInstanceRequest is unchanged.
  • Restart using the source orchestration's original name and input without copying execution history. version: null uses the backend's restart default; version: "" explicitly selects an unversioned orchestration.
  • Expose immediate source lineage through TaskOrchestrationContext.SourceInstanceId in both worker paths, preserving IDs as opaque strings, including dt: and dto: prefixes.
  • Refresh the protobuf definitions and provenance, update coverage and the README example, and guard scheduling-log argument evaluation when Information logging is disabled.
string cloneId = await client.RestartAsync(
    "source-instance",
    "clone-instance",
    version: "v2",
    cancellation: CancellationToken.None);

Why is this change needed?

  • Enable orchestration cloning with a caller-selected target ID, a version override, and immediate source lineage, supporting .NET isolated Durable Functions integration.

Issues / work items


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; existing API signatures and wire field numbers remain unchanged.
      • Migration guidance: No migration for existing callers. The new overload requires a backend supporting CloneInstance.

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: Client API and RPC handling, source metadata, worker property conversion, protobuf refresh, logging guard, tests, and documentation.
  • What you changed after AI output: Iterated on review feedback to use the four-argument overload and separate clone RPC, preserve opaque source IDs, add the logging guard, and simplify the README.

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

Testing

Automated tests

  • Result: Passed; 68 focused tests rerun against commit 844a78e.
  • dotnet test .\test\Client\Grpc.Tests\Client.Grpc.Tests.csproj --no-restore --filter 'FullyQualifiedName~GrpcDurableTaskClientRestartTests|FullyQualifiedName~GrpcDurableTaskClientTests.ScheduleNewOrchestrationInstanceAsync' --verbosity quiet -clp:ErrorsOnly — Passed, 37 tests.
  • dotnet test .\test\Worker\Grpc.Tests\Worker.Grpc.Tests.csproj --no-restore --filter 'FullyQualifiedName~GrpcOrchestrationRunnerTests|FullyQualifiedName~OnRunOrchestratorAsync_Properties_ExposeImmediateSource' --verbosity quiet -clp:ErrorsOnly — Passed, 27 tests.
  • dotnet test .\test\Grpc.IntegrationTests\Grpc.IntegrationTests.csproj --no-restore --filter 'FullyQualifiedName~DurableTaskGrpcClientIntegrationTests.RestartAsync' --verbosity quiet -clp:ErrorsOnly — Passed, 4 tests.
  • Coverage includes legacy default-literal calls, clone request/error handling, unsupported-server behavior without fallback, opaque source IDs during execution and replay, and unchanged date conversion for unrelated properties.
  • The full solution suite and DTS emulator tests were not run.

Manual validation (only if runtime/behavior changed)

  • Environment (OS, .NET version, components): Windows, .NET SDK 10.0.403; local SDK client/worker and in-process gRPC integration tests.
  • Steps + observed results: No separate manual validation against a deployed Durable Functions or DTS backend was performed.
  • Evidence (optional): Automated results above.

Notes for reviewers

  • No breaking change — the existing bool restart overload and its RPC contract remain unchanged. The new context member is virtual rather than abstract; serializer defaults are unchanged.
  • The four-argument overload requires a backend supporting CloneInstance. Older servers return UNIMPLEMENTED, surfaced as NotSupportedException, without performing a legacy restart. The client never falls back to RestartInstance.
  • A mismatched response instance ID causes InvalidOperationException. The client cannot independently verify whether a supporting backend honored a version override.
  • SourceInstanceId identifies the immediate source when source and target IDs differ. It is separate from parent metadata and is not inherited by sub-orchestrations; same-ID lineage is backend-specific.
  • The orchestration-service client shim does not implement the new overload.

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.

🟡 Changes recommended

The new RestartAsync overload makes previously valid default-literal calls fail to compile, while property unpacking can also lose backend-supplied lineage for date-like source IDs.

2 open issues
What changed in this PR

This PR adds caller-selected restart IDs and optional version overrides to the Durable Task SDK, plus immediate source-instance metadata for cloned orchestrations.

Changes:

  • Add restart options and gRPC handling, including validation and returned-ID checks.
  • Expose SourceInstanceId and forward request properties through the standalone worker.
  • Add focused tests and document usage, backend requirements, and protobuf provenance.
File Description
test/​Worker/​Grpc.Tests/​GrpcOrchestrationRunnerTests.cs Test lineage during execution and replay.
test/​Worker/​Grpc.Tests/​GrpcDurableTaskWorkerTests.cs Test standalone worker lineage forwarding.
test/​Worker/​Core.Tests/​Shims/​TaskOrchestrationContextWrapperTests.cs Test property-backed source metadata.
test/​Client/​Grpc.Tests/​GrpcDurableTaskClientRestartTests.cs Cover restart options, validation, compatibility, and errors.
src/​Worker/​Grpc/​GrpcDurableTaskWorker.Processor.cs Forward request properties into orchestration contexts.
src/​Grpc/​versions.txt Record provenance of the imported protobuf patch.
src/​Grpc/​orchestrator_service.proto Add optional target-ID and version fields.
src/​Client/​Grpc/​GrpcDurableTaskClient.cs Implement explicit restart options and shared error handling.
src/​Client/​Core/​RestartOrchestrationOptions.cs Define target-ID and version options.
src/​Client/​Core/​DurableTaskClient.cs Add the virtual restart-options overload.
src/​Abstractions/​TaskOrchestrationContext.cs Expose immediate source-instance metadata.
README.md Document cloning usage and compatibility limitations.

Review effort: Balanced

Comment thread src/Client/Core/DurableTaskClient.cs Resolved

@berndverst Bernd Verst (berndverst) left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The SDK-side approach largely accomplishes the stated goal for a supporting backend, and I don't see a material new performance regression. The property-conversion comparison was an isolated microbenchmark with the default null logger factory, not an end-to-end throughput benchmark. Backend support remains a deployment prerequisite, as documented.

Requesting changes for the two existing inline findings, both independently reproduced: restart overload source compatibility and opaque source-ID preservation.

The unrelated, pre-existing logging improvement request is now a non-blocking file-level comment.

Copilot AI balanced review requested due to automatic review settings November 10, 2026 17:44

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.

🟡 Changes recommended

The PR description advertises an unimplemented restart API and outdated compatibility behavior, so callers following it would encounter compilation errors and misleading older-server guidance.

1 open issue
2 resolved since last review

Review effort: Balanced

Comment thread src/Client/Core/DurableTaskClient.cs Resolved
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.

3 participants