Skip to content

Add large-payload blob auto-purge with a persistent orchestration - #758

Merged
wangbill (YunchuWang) merged 81 commits into
mainfrom
yunchuwang-wangbill-blob-payload-autopurge-sdk
Sep 21, 2026
Merged

wangbill (YunchuWang) merged 81 commits into
mainfrom
yunchuwang-wangbill-blob-payload-autopurge-sdk

Conversation

@YunchuWang

@YunchuWang wangbill (YunchuWang) commented Jul 8, 2026 •

Copy link
Copy Markdown
Member

Summary

Large orchestration payloads are externalized to Azure Blob Storage by the AzureBlobPayloads extension, with a token persisted in SQL instead of the payload bytes. When orchestration state is purged, DTS cannot delete the backing blob itself: it has no customer storage credentials. The customer's worker performs that deletion.

This PR implements the worker/SDK side. The backend preserves cleanup work in a durable tombstone ledger; the worker fetches due tombstones, deletes eligible blobs, and reports every row's outcome so the backend can resolve, reschedule, or quarantine it. The backend prevents a tombstone from being fetched while a live payload row still references the same token.

Companion changes: contract in microsoft/durabletask-protobuf#76, backend in AAPT-DTMB PR 16368738.

gRPC contract and explicit opt-in

The contract is in src/Grpc/durable-task-scheduler/large_payload_purge.proto, under the separate microsoft.durabletask.largepayloads.LargePayloadPurge service:

RPC Caller and purpose
SetLargePayloadAutoPurge An explicit Durable Task client call persists the task hub's enabled/disabled choice.
GetLargePayloadTombstones The worker fetches a bounded batch of due cleanup records.
ReportLargePayloadPurgeResults The worker reports each attempted cleanup outcome.

These are not methods on TaskHubSidecarService. GetWorkItemsRequest has no auto-purge opt-in field, and worker connection does not announce or change this setting.

The existing refresh-protos.ps1 script refreshed all three SDK-vendored proto files from upstream main commit 882583c1177706e148c3b146d4b2ba1c1d242e4c; commit c2c6f83fe6ae4a38a0fb3c460d47390e3136880d records the regenerated three-entry versions.txt. All three downloaded contract contents matched upstream and the prior SDK Git blobs. No generated proto was manually edited.

DurableTaskClient.SetLargePayloadAutoPurgeAsync(enabled, batchSize, cancellationToken) writes the backend setting first. On enable, it always schedules the reserved fixed-ID purge orchestration without a preliminary GetInstance query, using an explicit terminal-only replacement policy. DTS deduplicates existing live instances; the client handles AlreadyExists narrowly, waits for the expected ID/name to be Running, then reliably enqueues its idempotent SetBatchSize configuration event. Disable only writes the backend flag and leaves the runner alive to wait durably; it does not terminate or suspend it. These steps are not atomic, and failures/cancellation are surfaced without rolling back earlier successful steps. Entity support is not required. Batch size configures the SDK runner, not the backend setting RPC.

The last successful explicit setting call wins; making no call leaves the setting unchanged. Disabling prevents new tombstones and new fetches while retaining existing tombstones for later re-enable. It does not recreate cleanup records for instances purged while disabled. Calls already in flight may complete.

The wire records use opaque correlation tokens:

  • LargePayloadTombstone { tombstone_token, payload_token }
  • LargePayloadPurgeResult { tombstone_token, disposition }

The SDK echoes tombstone_token unchanged; database identity and revision interpretation belong to the backend. Failure reports use the backend's revision guard; duplicate/stale reports must not inflate retry attempts, and successful deletion remains idempotent.

Dispositions

Disposition Meaning Examples
Deleted Tombstone resolved Blob deleted, already absent, or not store-owned.
Retry Work remains recoverable; backend reschedules Storage unreachable, authorization failures, throttling/5xx, timeout, or an unsupported token version.
Quarantined Evidence retained for a deterministic failure Malformed v2 token, HTTP 400, or an unexpected legacy v1 token.

The worker reports the entire batch, including retryable rows. Backend SQL owns per-row retry scheduling; the SDK job waits on a one-minute durable backoff timer when an entire batch is retryable or a cycle fails, rather than imposing a per-row retry schedule. Failure detail is logged at the classification site without exposing raw payload tokens; it is not carried in extra wire reason/error-code fields.

The local PayloadDeleteOutcome API reserves Unspecified = 0; Deleted, AlreadyAbsent, and NotStoreOwned are explicitly numbered 1, 2, and 3. Only those three known success values resolve tombstones. Default/unspecified or unknown values are logged and reported as Retry. This local enum is distinct from the protobuf disposition enum, whose wire numbers are unchanged. The numeric correction is to the new, unreleased API in this PR; private consumers compiled against earlier PR snapshots must be rebuilt together.

Blob ownership and retention

Token syntax alone is not proof of store ownership. UploadAsync writes the fixed metadata marker managed_by=dts; deletion re-reads metadata and uses the ETag from that read in an If-Match condition.

The marker is an ownership convention within customer-controlled storage, not an authenticated or unforgeable provenance claim. A principal permitted to modify metadata can copy it; ETag conditions only protect the interval between the read and delete. Protecting payload storage and preventing customer-authorized writers from forging markers or manipulating references is the customer's responsibility. This PR accepts that trust boundary and does not add signed markers or a separate ownership ledger; the marker is not an authorization boundary against untrusted metadata writers.

The ownership HEAD treats a 404 as AlreadyAbsent only when Azure identifies BlobNotFound or ContainerNotFound. Unknown, empty, or missing error codes propagate as storage failures and remain retryable instead of resolving the tombstone on an uncertain response. Known container absence remains terminal, consistent with the pinned Blob SDK's DeleteIfExistsAsync behavior. Ownership markers, If-Match, cross-account checks, and soft-delete/version retention limits are unchanged.

  • Marker present: request deletion with the ETag condition, preventing a concurrent overwrite from being deleted using an outdated ownership check.
  • Marker absent: leave the blob untouched and report Deleted to resolve cleanup work that is not the store's to perform. This is logged distinctly.

Only recognized v2 tokens are deleted. Legacy v1 tokens are not eligible for automatic deletion; unsupported future prefixes remain retryable. A successful delete means the current object is deleted or the cleanup reference is otherwise resolved, not that soft-deleted data, retained versions, or physical storage bytes have been reclaimed.

Validation

The internal version-policy follow-up 611f654e1c569584f3c8479a3f3145589e26cc73 passed 216 extension cases, 47 core version/factory/filter cases, 27 gRPC option/filter controls and 13 loopback cases (303 total). Before the change, 12 targeted cases failed for inherited client versions, restricted internal filter versions, or internal work rejected by a strict/current-or-older worker. Actual client request capture, GetWorkItems frame capture and real-worker dispatch through orchestration/activity shims verify the separate gates. Business task mismatch behavior, task-kind/name-prefix isolation, disabled-extension workers, custom orchestration filters and immutable per-worker exemption snapshots are covered. Previous loop/control/transport cases are retained, with the three internal strict-filter cases renamed to reflect the intentionally changed wildcard policy. Worker.Grpc and AzureBlobPayloads netstandard2.0 builds passed; normalized warning sets match the prior baseline. No new deployed DTS/Azure or cross-SDK replay compatibility claim is made.

Named-worker purge transport isolation in bb582d8488117aa1193654f643151761ac5947d5 changes one production DI file, using keyed invoker/client registrations and the worker's named activity factories. The address-only shutdown regression first failed through the real Get/Report activities when the last-started worker disposed its owned channel. After the fix, 190 focused extension cases and seven loopback HTTP/2 cases passed; all 189 previous extension cases remain included. The loopback cases verify the surviving worker's original connection and synthetic authentication chain without restarting or republishing it, reversed/default-named order, active-worker routing, separate providers, and missing-own-transport errors. An additional recreation regression verifies that replacing one worker's channel does not redirect another worker's activities. The netstandard2.0 build passed with 0 errors and 38 warnings in unchanged files. These are local SDK/transport regressions, not deployed DTS/Azure E2E.

Follow-up fbc1fd3f80fbc71212c77d954b2579337937cdfd preserves internal purge names in nonempty worker filters, and c18dea6015d328970c6abb11581a3a289baeb318 makes unknown/missing-code ownership HEAD404 responses retryable. Expected pre-fix failures were observed for missing internal filter names and uncertain-404 success classification. Final offline regressions passed 189 extension cases (including all 110 prior loop/control cases), 34 core filter cases, and 14 gRPC filter cases. The netstandard2.0 build succeeded with 0 errors and 38 warnings in unchanged files. Those two changes did not address named-worker shared-transport lifetime or custom-provider ArgumentException classification. The former is addressed by the per-worker transport follow-up below; the latter remains an accepted custom-provider limitation. No new cloud E2E claim is made.

The loop simplification is split into 88c3dbde4745f1fca8d413f5e5a5c7d6ad7d2454 (local cycle counting and the durable-wait helper) and b34f04046715fe86d184ab8daf9a4219e7a37b2a (waiting directly for configuration after an unsupported RPC). The latter deliberately defers fifth-cycle ContinueAsNew until configuration arrives. The final focused offline regression run passed 110 cases, including real-executor replay, waiting/wake, late-event carryover and client lifecycle coverage. The netstandard2.0 build completed with 0 errors and 38 warnings in unchanged files. Dynamic batch configuration, custom status and diagnostic counts remain supported. No new cloud E2E or throughput claim is made.

The entity-free lifecycle was introduced in e3fce5dd6eea0a97ed03833c588eab3ca5d05d26: 108 focused local regression cases and the netstandard2.0 build passed. Follow-up 8ff4746cf30e124c63a643af74c1345c61cee32e removes the pre-start lookup and relies on backend start deduplication; its 34 focused client cases and netstandard2.0 build passed. Earlier validation below remains historical evidence for its stated commits.

Review-comment validation at fbd55d6 used 41 existing focused tests and seven local behavioral probes. The probes verified response disposal through the pinned gRPC transport and reproduced the previous unsafe treatment of default/unknown payload-delete outcomes; they were not a claim that those outcomes were safe.

Post-fix focused regression runs passed: 56 AzureBlobPayloads tests covering deletion, enum values, client control, and worker transport, plus two HelloDeadline tests. New regression cases first failed against the pre-fix implementation, then passed with the default/unknown-outcome guard. The previously unawaited asynchronous assertion in PurgeTransportTests is also awaited now.

No new cloud E2E run was performed for the earlier outcome-safety fix 49eee4e3. The S1-S3 and separate 20,000-business-instance/100,000-target-blob E2E results used the pinned fbd55d6 SDK. A later focused real-Azure cross-container regression used 09b6e7e2, as described below.

Same-account container changes

DownloadAsync and DeleteAsync share client selection. V2 references in another container on the same configured account/service endpoint reuse the configured client's credentials and pipeline, including Shared Key and the original SAS restrictions. Scheme, host, port, and the SDK-derived account path must match; account-key credentials are not forwarded to a different account/endpoint. The existing cross-account identity path and ownership/ETag safeguards remain unchanged.

The focused regression run for 09b6e7e2 passed 49 cases. Real Azure validation used an account-key connection string with payload Credential=null: one orchestration paused with three old-container blobs; a fresh worker configured for the new container read the old references, wrote two new-container blobs, completed the orchestration, and auto-deleted all five targets after exact instance purge. Both containers ended empty and the instance was absent; auto-purge was disabled and both workers exited normally.

The original private controller reported an IOException during final reporting after these successful operations. A separate read-only verifier, using the unchanged product binary, reconfirmed all final cloud conditions without repeating seed/purge or manually deleting blobs. Those original-run results are postcondition verification, not an uninterrupted controller PASS. A subsequent fresh real-Azure run with the same product binary completed the full Shared Key migration, replay, exact instance purge, five original BlobNotFound checks, empty old/new containers, auto-purge shutdown, and both worker exits successfully. Its controller exited 0 with an atomic PASS report at 2026-09-14 22:48:21 UTC. Only the private harness journal/report handling changed; the first run's evidence remains intact. See the detailed fix and validation reply.

Persistent orchestrator lifecycle

The task hub uses one long-lived orchestration with fixed instance ID BlobPurgeJob-__dt_blob_payload_autopurge__, reserved for this SDK-owned task. There is no auto-purge entity, custom activation generation, generation-specific runner ID, or enable-time enumeration/purge of old runners.

This reserved ID is a documented SDK contract, not a backend-enforced ownership namespace. Applications must not use it for their own instances. The terminal-replacement policy applies regardless of an existing terminal instance's orchestration name, so using this exact ID for application code can cause its terminal instance and history to be replaced. The post-start ID/name validation detects an unexpected live runner; it cannot detect a terminal instance that has already been replaced. A preliminary read would not make ownership enforcement atomic, and no backend reservation is added here.

On enable, the client:

  1. Writes the backend auto-purge setting.
  2. Always schedules the fixed ID, without a preliminary existence query, using an explicit reuse policy that permits replacement only for Completed, Failed, Terminated, and Canceled. Running, Pending, Suspended, and ContinuedAsNew are not replaceable. DTS performs deduplication; AlreadyExists is handled narrowly rather than treated as an enable failure.
  3. Waits for the expected runner to be Running and validates its returned ID/name/status, then reliably enqueues an idempotent SetBatchSize configuration event. A successful call confirms actual start and event acceptance, not that the latest configuration has already been applied.

A manually suspended runner, an unexpected returned ID/name/status, or a runner that becomes terminal while waiting is an explicit error rather than a successful enable or forced termination. Setting, start, wait, and event failures/cancellation propagate without rollback; retrying the explicit enable is the recovery path. These operations are not atomic with independently issued management or conflicting control operations.

Disable only writes the backend setting. It does not create, terminate, suspend, or wait for an orchestration. The runner remains logically Running and idles on a one-minute durable timer when fetch returns empty or is declined because auto-purge is disabled. No worker thread remains occupied during the durable wait. An enable configuration event can interrupt the idle/backoff wait. Work already fetched before disable may finish.

The runner carries only the current batch size and diagnostic count in its ContinueAsNew input and exposes bounded custom status. The five-cycle counter is local to each execution and is reconstructed through replay; it resets at rollover. A single configuration waiter drains delivered and buffered updates before rollover, and unprocessed/late events are preserved for the next execution. The one-minute idle/backoff helper reuses that waiter and cancels and observes a timer interrupted by configuration. When a backend RPC is unsupported, the runner logs the condition and waits for configuration directly in the failure handler, rather than persisting a separate unsupported flag or polling the unsupported operation. If this happens on the fifth attempted cycle, the current execution waits until configuration arrives; it applies the delivered updates before ContinueAsNew. The next execution receives the latest batch size and diagnostic count.

The existing Get / parallel Delete / Report activity structure is retained: up to four 50-token chunk activities concurrently, with up to eight storage deletes per chunk (32 per cycle). Blob ownership, ETag conditions, dispositions, opaque tombstone correlation, activity retries, and 60-second per-attempt RPC/delete limits are unchanged. Customer externalization thresholds remain unchanged; there is no AUX naming/container change or forced-inline exception.

Entity support is no longer required by auto-purge, and extension registration preserves the caller's entity-support setting. The enabling client needs the relevant setting, metadata-read, orchestration-start, and raise-event permissions.

The entity-free lifecycle refactor passed 108 focused local cases covering public-client start-policy/wait/idempotence/error/cancellation behavior, actual SDK executor replay/configuration/timer/CAN/fan-out behavior, entity-option preservation, and existing deletion/RPC/transport/interceptor controls. The direct-schedule follow-up passed 34 focused client cases, including rejection of any preliminary GetInstance call, AlreadyExists handling, and start-wait validation. The netstandard2.0 product build succeeded for both changes. No new cloud run or throughput equivalence claim is made.

Per-worker purge transport

Each worker has its own purge client and rebindable invoker. Its named Get/Report activity factories use that same worker's client, so starting, recreating, or stopping another worker cannot redirect those activities to the other worker's channel. The client reuses its worker's effective post-interceptor transport, including authentication and channel recreation; no extra connection or cross-worker fallback mechanism is introduced. The PayloadStore remains shared under the existing per-host storage-configuration constraint. This does not change public client APIs, activity names or payloads, the fixed runner ID, or the wire contract, and does not add support for different storage/backend configurations in one host.

Required work-item filters

Workers using externalized payloads keep the purge orchestrator and its three activities in their nonempty work-item filters, regardless of the relative order of standard filter and payload-extension registration calls. Only these internal task entries use wildcard version lists, including when callers previously supplied version restrictions for the reserved names. Customer orchestration/activity/entity filters and their version restrictions remain unchanged. No filters, an explicit null, and an all-empty filter set remain unfiltered; partial filter sets gain the internal orchestration/activity categories needed by auto-purge. Caller-owned filter lists and workers without this extension are not changed.

Purge workflow upgrade policy

The release policy for this SDK-owned workflow is compatibility-first: normal SDK updates must preserve replay compatibility of the orchestration's recorded actions and the input/output/behavior contracts of its activities. Merely deploying a newer SDK or exempting these tasks from customer business-version checks does not make an incompatible workflow change safe. ContinueAsNew bounds history; it is not an automatic binary-upgrade or routing barrier.

If an unavoidable change is replay-incompatible, use an explicit, coordinated client-management operation in a maintenance window rather than preserving multiple workflow implementations indefinitely:

  1. Coordinate one management operation for the task hub and prevent competing enable/restart operations. While a compatible worker can still process management work, use the client's TerminateInstanceAsync for only the SDK-reserved purge instance, BlobPurgeJob-dt_blob_payload_autopurge. Await WaitForInstanceCompletionAsync with an appropriate cancellation/deadline and inspect its terminal status before attempting replacement; accepting the terminate request is not completion.
  2. Drain or stop the old worker deployment and account for already-running purge activities before admitting incompatible new purge work. Terminating an orchestration does not cancel in-flight activities. The client cannot shut down customer worker processes; deployment coordination must ensure no incompatible old worker can claim the replacement orchestration or its activities. This is not an instruction to terminate customer business orchestrations.
  3. Deploy/start the compatible new workers, then call SetLargePayloadAutoPurgeAsync(true, desiredBatchSize). Its existing terminal-instance replacement policy creates a new execution with fresh history under the reserved fixed ID, waits for Running and enqueues the batch configuration. An existing active runner is not implicitly reset by enable.

Do not confuse this with disable followed by enable. Disable only changes the backend flag and pauses new fetches; it does not terminate/reset the runner. It also stops creation of new tombstones, so payload references retired while disabled are not recovered merely by re-enabling. The reset procedure can leave the ledger setting enabled while workers are coordinated; existing tombstones remain the cleanup source of truth.

This is an operational upgrade contract using existing client-management APIs, not a new automatic reset API, worker-startup reconciliation loop, or per-task upgrade framework. It does not promise arbitrary incompatible SDKs can coexist, instantaneous activity cancellation, or a zero-downtime breaking rollout. A future incompatible release must document its specific deployment/drain requirements and validate that upgrade path; no new mixed-SDK/deployed-Azure upgrade experiment is claimed here.

Internal purge version policy

The SDK-owned purge workflow is independent of the customer's business version policy. New purge instances explicitly use an empty version rather than inheriting the enabling client's DefaultVersion. On workers that register the payload extension, an internal, default-off task-specific exemption lets only the purge orchestrator and its three activities bypass the worker's business version matching check. The worker still applies the configured Strict, CurrentOrOlder, None, Reject and Fail behavior to customer tasks. The extension does not rewrite customer version options or runtime history and does not start a separate worker or connection. This prevents business-version filters/checks from excluding internal purge work; it does not bypass authentication, custom orchestration filters, concurrency limits, operator suspension, disabled cleanup, or the need for a running worker. It is not a guarantee of replay compatibility between arbitrary SDK implementations.

Cross-assembly registration follows the existing Microsoft.DurableTask.Worker.Grpc.Internal.InternalOptionsExtensions pattern. It adds one metadata-public infrastructure hook, documented with the same non-public-support compatibility caveat as the existing hooks; the exemption sets remain internal and are snapshotted when a worker is constructed. It does not add or alter customer version-strategy settings.

CAN payload reclamation

The existing full activity-result payloads can still be externalized. Removing the entity does not remove the need to reclaim externalized history payloads retired by ContinueAsNew.

Companion AAPT-DTMB PR 17171573 was merged into main on 2026-09-17: merge commit a6fb9f92407aedb6bfa2c25371eafaf86656f6ea, from source f75d8b153ea08d7b72835686aaee8ce3764c2537. It routes saved history and replaced input references retired by non-rewind CAN through the opt-in tombstone path while preserving current/shared input references. The existing disabled hard-delete branch and live-token guard are unchanged. This confirms the backend fix was merged, not that it has been deployed; no new paired SQL/Azure reclamation E2E is claimed.

This does not reconstruct references already lost before the backend change, promise AUX cleanup on disable, or claim that every other payload-retirement path is covered. The separately reproduced inbox-only immediate-CAN gap remains outside this SDK lifecycle change. Historical Azure E2E evidence remains tied to its original commits.

Notes

  • PayloadStore.DeleteAsync is virtual with a default that throws NotSupportedException, so existing upload/download implementations are not required to add an override. Deletion requires an implementation that explicitly supports it.
  • Worker registration makes the purge orchestrator and activities available; it does not enable auto-purge. There is no auto-purge entity or hosted-service reconciliation loop.
  • PurgedCount counts resolved Deleted rows, including not-store-owned objects; it is not a physical-blob or reclaimed-byte counter.
  • Repeated enable updates batch size through a configuration event without replacing the live fixed-ID runner.
  • This change depends on a backend implementing the dedicated LargePayloadPurge service.

@YunchuWang
wangbill (YunchuWang) force-pushed the yunchuwang-wangbill-blob-payload-autopurge-sdk branch 2 times, most recently from 82ae04d to 0ac2dc3 Compare July 8, 2026 20:50
@YunchuWang
wangbill (YunchuWang) force-pushed the yunchuwang-wangbill-blob-payload-autopurge-sdk branch 4 times, most recently from 0752610 to cab0e9a Compare July 13, 2026 19:07
@YunchuWang wangbill (YunchuWang) changed the title Add large-payload blob auto-purge (worker/SDK side) Add large-payload blob auto-purge (opt-in singleton job, worker/SDK side) Jul 13, 2026
@YunchuWang
wangbill (YunchuWang) force-pushed the yunchuwang-wangbill-blob-payload-autopurge-sdk branch from cab0e9a to c05b15a Compare July 13, 2026 20:38
Large orchestration payloads are externalized to Azure Blob Storage as
`blob:v1:<container>:<blobName>` tokens. The DTS backend stores those tokens but
cannot delete the backing blobs (it has no storage credentials) — only this SDK
can. This adds an opt-in, whole-scheduler singleton durable entity +
orchestration job (mirroring src/ExportHistory) that drains payload rows the
backend has soft-deleted and deletes their blobs, then acks so the backend can
hard-delete the rows.

Design:
- PayloadStore.DeleteAsync is virtual (default throws NotSupportedException so it
  is non-breaking for existing external subclasses); BlobPayloadStore overrides
  it to decode the token and call DeleteIfExistsAsync (idempotent).
- BlobPurgeJob (TaskEntity singleton): Create is a no-op when already Active so
  racing client processes don't disturb the running job; Run starts a fixed-id
  orchestrator.
- BlobPurgeJobOrchestrator (perpetual): fetch a batch of tombstones, delete the
  blobs with capped parallelism, ack the successful deletions (failed tokens stay
  tombstoned to retry), idle on a timer when empty, ContinueAsNew periodically.
- ExecuteBlobPurgeJobOperationOrchestrator bridges client -> entity.
- Two new unary RPCs on TaskHubSidecarService: GetTombstonedPayloads /
  AckPurgedPayloads (authoritative proto follow-up: microsoft/durabletask-protobuf#76).
- LargePayloadStorageOptions gains AutoPurge (opt-in, default false) and
  PayloadPurgeBatchSize (default 500).
- Client-side BlobPurgeJobStarter (IHostedService) ensures the singleton job when
  AutoPurge is enabled, without blocking host startup. Worker always registers the
  entity/orchestrators/activities so a client-enabled job has something to run.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@YunchuWang
wangbill (YunchuWang) force-pushed the yunchuwang-wangbill-blob-payload-autopurge-sdk branch from c05b15a to 306d19f Compare July 13, 2026 22:43
Comment thread src/Client/Core/PayloadPurgeAckDto.cs Outdated
Comment thread src/Client/Core/TombstonedPayloadDto.cs Outdated
Comment thread src/Client/Grpc/GrpcDurableTaskClient.cs Outdated
Comment thread src/Extensions/AzureBlobPayloads/AutoPurge/Client/BlobPurgeJobStarter.cs Outdated
Comment thread src/Extensions/AzureBlobPayloads/AutoPurge/Client/BlobPurgeJobStarter.cs Outdated
Comment thread src/Extensions/AzureBlobPayloads/AutoPurge/Models/BlobPurgeJobStatus.cs Outdated
Comment thread src/Extensions/AzureBlobPayloads/AutoPurge/Models/BlobPurgeJobStatus.cs Outdated
Comment thread src/Extensions/AzureBlobPayloads/AutoPurge/Client/BlobPurgeJobStarter.cs Outdated
Comment thread src/Extensions/AzureBlobPayloads/AutoPurge/Client/BlobPurgeJobStarter.cs Outdated
Comment thread src/Extensions/AzureBlobPayloads/AutoPurge/Client/BlobPurgeJobStarter.cs Outdated
Comment thread src/Extensions/AzureBlobPayloads/AutoPurge/Client/BlobPurgeJobStarter.cs Outdated
…er simplification

- Drop the `Dto` suffix now that the payload records are first-class public
  types in `Microsoft.DurableTask.Client` (`TombstonedPayload`,
  `PayloadPurgeAck`).
- Collapse the magic `500` batch-size literal into a single
  `BlobPurgeConstants.DefaultBatchSize` used everywhere.
- Rename `BlobPurgeJobStatus.Stopped` -> `Pending` (still the zero value) and
  remove the dead `Failed` member (nothing ever set it; the job self-heals).
- Make the perpetual orchestrator self-heal: wrap each cycle in try/catch so a
  transient backend/entity/activity failure logs, backs off, and continues
  instead of failing the orchestration and killing the eternal loop.
- Ack poison tokens: `DeleteExternalBlobActivity` now returns a three-way
  `BlobDeleteResult` (Deleted/Discarded/Retry). Malformed tokens are discarded
  and acked so the backend can clear the stuck row instead of re-streaming it
  forever; transient failures stay tombstoned to retry.
- Replace the single-value `BlobPurgeJobCreationOptions` record with a plain
  `int` on `BlobPurgeJob.Create`.
- Guard the client fetch RPC: `GetTombstonedPayloadsAsync` throws
  `ArgumentOutOfRangeException` unless `0 < limit < 1000`.
- Simplify `BlobPurgeJobStarter` to a fixed-instance-id fire-once: drop the
  entity-active pre-check and schedule the Create bridge once with a fixed
  instance id, retrying only until the backend is reachable.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

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.

Snapshot eligible runner metadata after enqueueing Create, read and exclude the current generation, then purge old IDs nonrecursively. Preserve paging, cancellation, zero-count idempotence, and failure propagation; document required permissions and caller coordination.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 44c27836-c49c-45fd-ae2b-3309f9c3f0f0

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.

🔵 Needs a closer look

Two moderate findings remain unresolved: multi-worker invoker sharing and missing proto provenance manifest regeneration.

Review details

Suppressed comments (2)

src/Extensions/AzureBlobPayloads/DependencyInjection/DurableTaskWorkerBuilderExtensions.AzureBlobPayloads.cs:120

  • RebindableCallInvoker is registered once for the whole service provider, while every named worker installs its own CallInvokerPublisher. With multiple workers, the last worker to start or recreate overwrites current, so an activity dispatched by another worker can send purge RPCs through the wrong worker's effective channel/interceptor chain (and through a disposed channel if that worker stops). This makes the documented multi-worker support unsafe; scope the purge client/invoker to the worker or reject multiple worker registrations instead of sharing this singleton.
        builder.Services.TryAddSingleton<RebindableCallInvoker>();
        builder.Services.TryAddSingleton(
            sp => new LargePayloadPurgeClient(sp.GetRequiredService<RebindableCallInvoker>()));

src/Grpc/refresh-protos.ps1:29

  • This adds large_payload_purge.proto to the refresh list, but the committed src/Grpc/versions.txt still contains only the two previous proto URLs. The README defines that file as the provenance manifest for every downloaded proto, so please run refresh-protos.ps1 and commit the regenerated manifest (and matching downloaded proto) with this change; otherwise the new generated contract has no recorded source commit.
    @{
        SourcePath = "durable-task-scheduler/large_payload_purge.proto"
    }
  • Files reviewed: 45/46 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Pause through the backend setting and idempotently ensure the fixed runner with an explicit terminal-only replacement policy. Wait for actual start before enqueueing configuration. Preserve configuration across durable waits and continue-as-new, remove generation and entity dependencies, and cover the lifecycle with real SDK executor tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 44c27836-c49c-45fd-ae2b-3309f9c3f0f0

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.

🔵 Needs a closer look

Resolve the multi-worker transport binding issue and add the missing proto provenance entry.

Review details

Suppressed comments (2)

src/Extensions/AzureBlobPayloads/DependencyInjection/DurableTaskWorkerBuilderExtensions.AzureBlobPayloads.cs:109

  • RebindableCallInvoker is registered once for the whole service provider, while every named GrpcDurableTaskWorker publishes its own effective invoker into it. With two named workers (the remarks above advertise same-configuration named workers as supported), whichever worker starts or reconnects last wins; an activity dispatched by the other worker can then fetch/report tombstones through the wrong sidecar or task hub. Scope the purge transport to the worker executing the activity, or explicitly reject this multi-worker configuration.
        builder.Services.TryAddSingleton<RebindableCallInvoker>();
        builder.Services.TryAddSingleton(
            sp => new LargePayloadPurgeClient(sp.GetRequiredService<RebindableCallInvoker>()));

src/Grpc/refresh-protos.ps1:28

  • The refresh list now includes large_payload_purge.proto, but src/Grpc/versions.txt still has only the two older proto URLs. That leaves the checked-in provenance manifest inconsistent with the script and means a future refresh cannot reproduce the new contract set. Run the refresh and commit the corresponding URL entry in versions.txt.
        SourcePath = "durable-task-scheduler/large_payload_purge.proto"
  • Files reviewed: 39/40 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Schedule the reserved fixed runner directly on each enable with the existing safe reuse policy. Remove the preflight metadata read while retaining narrow AlreadyExists handling, actual-start identity/status checks, and configuration delivery.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 44c27836-c49c-45fd-ae2b-3309f9c3f0f0

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

Unresolved contract compatibility and lifecycle, registration, and proto-manifest issues must be addressed.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

src/Extensions/AzureBlobPayloads/AutoPurge/Client/DurableTaskClientExtensions.AzureBlobPayloads.cs:79

  • ContinuedAsNew in this list is not actually represented in the reuse policy: GrpcDurableTaskClient passes these values through ProtoUtils.ConvertDedupeStatusesToReusePolicy, whose GetAllStatuses() omits ContinuedAsNew (ProtoUtils.cs:20-30). An instance observed in that state can therefore be treated as replaceable, contrary to the terminal-only policy. Include the status in the conversion's complete set and add an assertion for the generated policy, or otherwise reject this unsupported status.
            DedupeStatuses = ["Running", "Pending", "Suspended", "ContinuedAsNew"],

src/Extensions/AzureBlobPayloads/DependencyInjection/DurableTaskWorkerBuilderExtensions.AzureBlobPayloads.cs:109

  • TryAddSingleton makes the rebindable invoker and purge client host-wide, but each named worker's PostConfigure callback binds that same object. When worker B starts or recreates, it overwrites the transport published by worker A, so A's purge activities can fetch/report against B's endpoint and credentials. This contradicts the claim that several named workers sharing one payload configuration are supported; scope the purge client/invoker per worker or reject multi-worker registration.
        builder.Services.TryAddSingleton<RebindableCallInvoker>();
        builder.Services.TryAddSingleton(
            sp => new LargePayloadPurgeClient(sp.GetRequiredService<RebindableCallInvoker>()));

src/Grpc/refresh-protos.ps1:28

  • This new entry is now part of the refresh set, but versions.txt still contains only the two older proto URLs. The refresh script's manifest loop writes one provenance URL per $protoFiles entry, so please commit the corresponding large_payload_purge.proto URL as well; otherwise the checked-in source manifest is stale and cannot reproduce all downloaded contracts.
        SourcePath = "durable-task-scheduler/large_payload_purge.proto"
  • Files reviewed: 39/40 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread src/Grpc/durable-task-scheduler/large_payload_purge.proto
Run src/Grpc/refresh-protos.ps1 with its default main branch, pinning all three downloaded proto URLs to upstream commit 882583c1177706e148c3b146d4b2ba1c1d242e4c. Proto content is unchanged; the generated versions manifest now includes the large-payload purge source.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 44c27836-c49c-45fd-ae2b-3309f9c3f0f0

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

Unresolved critical findings affect purge execution and safe tombstone resolution.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

src/Extensions/AzureBlobPayloads/DependencyInjection/DurableTaskWorkerBuilderExtensions.AzureBlobPayloads.cs:109

  • This registration is host-wide, but every named GrpcDurableTaskWorker publishes its effective invoker into the same RebindableCallInvoker. With two named workers, whichever worker starts or recreates last wins, so purge activities dispatched by the other worker can send fetch/report RPCs through the wrong task-hub transport and credentials. That contradicts the surrounding claim that multiple named workers are supported; make the purge client/activity transport worker-scoped or reject this configuration.
        builder.Services.TryAddSingleton<RebindableCallInvoker>();
        builder.Services.TryAddSingleton(
            sp => new LargePayloadPurgeClient(sp.GetRequiredService<RebindableCallInvoker>()));

src/Extensions/AzureBlobPayloads/PayloadStore/PayloadStore.cs:36

  • PayloadStore is a public type in a shipped package, so adding this virtual member can change dispatch for an existing derived type that already has a same-signature DeleteAsync helper: it hides rather than overrides the new base member, and calls through PayloadStore (as the purge activity does) will hit the throwing default. Avoid a colliding contract or document this as a breaking-change migration requiring explicit overrides; the current summary does not call out this compatibility impact.
    /// <summary>
    /// Deletes the payload referenced by the token. Implementations that support deletion must be
    /// idempotent: deleting a payload that no longer exists is a no-op and must not throw.
    /// </summary>
    /// <remarks>
    /// The default implementation throws <see cref="NotSupportedException"/>. Stores that externalize
    /// payloads to deletable storage (for example Azure Blob Storage) should override it. It is declared
    /// virtual rather than abstract so existing upload/download implementations are not required to add an
    /// override. A preexisting same-named method does not automatically override this member; subclasses that
    /// support deletion must explicitly override it.
  • Files reviewed: 40/41 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread src/Extensions/AzureBlobPayloads/PayloadStore/BlobPayloadStore.cs Outdated
Keep the cycle counter local to each execution and drive rollover tests through actual cycles. Extract timer-versus-configuration waiting without changing unsupported-backend behavior or registering another event waiter.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 44c27836-c49c-45fd-ae2b-3309f9c3f0f0
Remove persistent unsupported state and await the existing configuration waiter inside the failure handler. On the fifth attempt, remain in the current execution until configuration is drained before continue-as-new. Preserve batch/count diagnostics, late carryover events, and timer cleanup with real-executor regressions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 44c27836-c49c-45fd-ae2b-3309f9c3f0f0

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.

🔵 Needs a closer look

Three moderate unresolved issues remain in exception classification, multi-worker transport scoping, and explicit work-item filter handling.

Review details

Suppressed comments (3)

src/Extensions/AzureBlobPayloads/AutoPurge/Activities/DeleteExternalBlobActivity.cs:194

  • This catch assumes every ArgumentException from PayloadStore.DeleteAsync means that the v2 token failed to decode. PayloadStore is a public injectable abstraction and its contract does not reserve ArgumentException for token parsing, so a custom store can raise one for a recoverable storage/configuration failure; this code would quarantine the tombstone permanently instead of retrying it. Use a dedicated token-format exception (or otherwise restrict this branch to the blob decoder) and let unrelated argument failures follow retry classification.
        catch (ArgumentException)
        {
            // The prefix gate above proves this is a v2 token, so the only remaining decode failure is a v2
            // body that does not parse. The SDK and backend control both sides of the protocol, so that
            // indicates a producer, corruption, or compatibility bug; retrying can never fix it.
            this.logger.BlobPurgeDeleteQuarantined("MalformedToken", null);
            return new BlobPurgeOutcome(LargePayloadPurgeDisposition.Quarantined);

src/Extensions/AzureBlobPayloads/DependencyInjection/DurableTaskWorkerBuilderExtensions.AzureBlobPayloads.cs:109

  • These registrations are unkeyed, so every named worker in the host shares one RebindableCallInvoker and one LargePayloadPurgeClient. Each worker's PostConfigure installs its own Rebind callback, and ExecuteAsync invokes it at startup and after channel recreation; the last worker to connect can therefore redirect purge fetch/report calls from another worker to the wrong endpoint or interceptor/auth chain. Scope the purge transport/client/activity to the worker, or explicitly reject multiple named gRPC workers instead of claiming that several named workers are supported.
        builder.Services.TryAddSingleton<RebindableCallInvoker>();
        builder.Services.TryAddSingleton(
            sp => new LargePayloadPurgeClient(sp.GetRequiredService<RebindableCallInvoker>()));

src/Extensions/AzureBlobPayloads/DependencyInjection/DurableTaskWorkerBuilderExtensions.AzureBlobPayloads.cs:119

  • Adding these types to the registry is not sufficient when the worker uses explicit UseWorkItemFilters(...): that API overwrites the named filters with only the caller-provided orchestration/activity names (src/Worker/Core/DependencyInjection/DurableTaskWorkerBuilderExtensions.cs:165-180). The internal purge names therefore are not dispatched, so an enabled runner can remain permanently idle while the backend continues to hold tombstones. Have this extension augment or reject explicit filters for the purge task names before enabling this feature.
        builder.AddTasks(r =>
        {
            r.AddOrchestrator<BlobPurgeJobOrchestrator>();
            r.AddActivity<GetLargePayloadTombstonesActivity>();
            r.AddActivity<DeleteExternalBlobActivity>();
  • Files reviewed: 40/41 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Merge required names after named filter configuration while preserving unfiltered workers, explicit versions, and caller-owned lists. Cover actual GetWorkItems frames and named options lifetimes.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c50c4706-834c-4fe9-b421-605bff3343b1
Require BlobNotFound or ContainerNotFound before confirming absence. Preserve ambiguous storage exceptions and verify real Azure HTTP, deletion activity, and generated report wire behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c50c4706-834c-4fe9-b421-605bff3343b1

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

Three unresolved findings affect runner versioning, multi-worker transport isolation, and blob ownership validation.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

src/Extensions/AzureBlobPayloads/DependencyInjection/DurableTaskWorkerBuilderExtensions.AzureBlobPayloads.cs:127

  • RebindableCallInvoker is registered unkeyed, so every named worker in a host shares this same instance. Each worker publishes its own effective invoker; when worker B starts or recreates its channel, purge activities dispatched by worker A can route through B's sidecar/auth/task-hub transport. That makes the documented multi-named-worker case unsafe even when storage options match. Scope the purge client/activities to the worker, or reject multiple worker transports instead of sharing this singleton.
        builder.Services.TryAddSingleton<RebindableCallInvoker>();
        builder.Services.TryAddSingleton(
            sp => new LargePayloadPurgeClient(sp.GetRequiredService<RebindableCallInvoker>()));
  • Files reviewed: 42/43 changed files
  • Comments generated: 2
  • Review effort level: Lite

Comment thread src/Extensions/AzureBlobPayloads/PayloadStore/BlobPayloadStore.cs
@YunchuWang

Copy link
Copy Markdown
Member Author

Disposition for the custom-provider ArgumentException concern in this review: accepted limitation / no code change in this PR.

The recommended implementation for this feature is the built-in BlobPayloadStore. Its public constructor already validates the required connection string or AccountUri/Credential configuration; the missing-endpoint example was an intentionally constructed custom-provider probe, not an observed built-in configuration-validation failure.

Custom PayloadStore implementations remain injectable, but their authors are responsible for validating their configuration early and adapting to the auto-purge classifier's current behavior. In particular, ArgumentException from DeleteAsync is classified as malformed input and reported as Quarantined; a custom provider must not use that exception for a recoverable operational/configuration failure if it expects automatic retries.

We are not expanding this PR to generalize arbitrary custom-provider exception semantics. The probe's result remains valid: using that exception for a repairable failure can leave a retained tombstone quarantined and therefore outside normal automatic retry, even after configuration is repaired. This is an explicit scope/risk acceptance, not a claim that the classifier was changed or that all ArgumentException instances intrinsically mean malformed tokens.

This concern appeared only as a suppressed item in the review summary, not as an unresolved inline thread, so there is no separate GitHub thread to mark resolved. Recording the disposition here rather than closing an unrelated thread.

Keep the existing case-insensitive predicate deferred over the same ordered activity names, without changing version or filter behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c50c4706-834c-4fe9-b421-605bff3343b1

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.

🔵 Needs a closer look

It spans a new persistent protocol, durable orchestration, storage deletion semantics, and worker transport integration.

Review details
  • Files reviewed: 42/43 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Bind per-worker keyed clients and invokers through registered Get/Report activity factories. Preserve each worker channel lifecycle and the shared payload store, with real owned-channel shutdown and recreation regressions.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c50c4706-834c-4fe9-b421-605bff3343b1
Comment thread test/Grpc.IntegrationTests/NamedPurgeTransportTests.cs Fixed
Comment thread test/Grpc.IntegrationTests/NamedPurgeTransportTests.cs Fixed

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.

🔵 Needs a closer look

Strict workers may reject the unversioned purge runner, preventing startup from completing.

Review details

Suppressed comments (1)

src/Extensions/AzureBlobPayloads/AutoPurge/Client/DurableTaskClientExtensions.AzureBlobPayloads.cs:84

  • With a Strict worker, AddAutoPurgeFilters advertises the runner and its activities only under workerOptions.Versioning.Version, but this start request leaves StartOrchestrationOptions.Version unset. Unless the caller separately configures the client with the exact same UseDefaultVersion, the runner is stamped unversioned, is filtered/rejected by that worker, and this WaitForInstanceStartAsync never completes. Please make the version requirement explicit or propagate a compatible version and add a strict-version lifecycle test.
            string instanceId = await client.ScheduleNewOrchestrationInstanceAsync(
                nameof(BlobPurgeJobOrchestrator), new BlobPurgeJobRunRequest(batchSize), options, cancellationToken);
  • Files reviewed: 43/44 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Start new purge runners explicitly unversioned, normalize only internal task filters to wildcard versions, and snapshot task-kind-specific infrastructure exemptions in each worker. Preserve customer version policies, named transports, and ordinary factory checks.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c50c4706-834c-4fe9-b421-605bff3343b1

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.

Use scoped host disposal while preserving shutdown semantics. Document expected request cancellation and retain the narrow experimental orchestration-filter regression.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: c50c4706-834c-4fe9-b421-605bff3343b1

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.

🔵 Needs a closer look

Two unresolved moderate findings affect dedupe-state enforcement and mixed-worker purge-task routing.

Review details

Suppressed comments (2)

src/Extensions/AzureBlobPayloads/AutoPurge/Client/DurableTaskClientExtensions.AzureBlobPayloads.cs:83

  • ContinuedAsNew is accepted here but is silently dropped when GrpcDurableTaskClient converts DedupeStatuses: ProtoUtils.GetAllStatuses() only enumerates seven statuses and omits P.OrchestrationStatus.ContinuedAsNew (src/Client/Grpc/ProtoUtils.cs:20-30). If the reserved instance is observed in that state, this request does not enforce the stated non-replacement policy and may replace it; either include the status in the conversion/tests or remove the claim from this policy.
            DedupeStatuses = ["Running", "Pending", "Suspended", "ContinuedAsNew"],

src/Extensions/AzureBlobPayloads/DependencyInjection/DurableTaskWorkerBuilderExtensions.AzureBlobPayloads.cs:134

  • These reserved tasks are registered only on workers that call UseExternalizedPayloads, but a worker without that extension and without UseWorkItemFilters is documented to process all work items (src/Worker/Core/DurableTaskWorkerWorkItemFilters.cs:7-11). In a mixed task hub it can therefore claim BlobPurgeJobOrchestrator or one of these activities, report TaskNotFound, and repeatedly consume/fail purge work before an enabled worker handles it. Please either add a backend-recognized routing/exclusion mechanism for these names or explicitly enforce/document that every worker connected to a hub with auto-purge enabled registers this extension.
        builder.AddTasks(r =>
        {
            r.AddOrchestrator<BlobPurgeJobOrchestrator>();
            r.AddActivity(nameof(GetLargePayloadTombstonesActivity), sp =>
                ActivatorUtilities.CreateInstance<GetLargePayloadTombstonesActivity>(
  • Files reviewed: 45/46 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@halspang

Copy link
Copy Markdown
Member

Overall, I think things are looking pretty good! One last thing I want to think about before moving forward, what would the upgrade path be for this orchestration/flow? If we change the orchestration, we could run into non-determinism issues. Have you thought about this/do we have a plan for it? It'd be good to be able to iterate on it :)

This could be as simple as turning the feature on and off again, btw. Just curious to see what we've thought about.

@YunchuWang

Copy link
Copy Markdown
Member Author

halspang, following up on your upgrade-path question: we will use a compatibility-first policy for this internal workflow. Normal SDK updates must preserve orchestration replay behavior and compatible activity contracts, so compatible releases can roll out without resetting the runner.

For an unavoidable replay-incompatible change, the fallback is an explicit client-managed reset in a coordinated maintenance window: terminate only the SDK's reserved purge instance, wait for a confirmed terminal state, account for its in-flight activities and retire/drain incompatible old workers, then start the updated workers and enable auto-purge to create a fresh execution/history. The existing enable path can replace a terminal fixed-ID runner; it does not replace an active one. Deployment coordination is required so an old worker cannot subsequently pick up the new flow, and concurrent management calls must be serialized by the caller.

One important distinction from simply toggling the feature: today's disable operation pauses new fetches but leaves the orchestration alive. Terminate is also asynchronous and does not cancel activities already executing. Also, disabling stops new tombstone creation, so we should not automatically toggle the backend flag off as a supposedly lossless reset step. The flag can remain enabled while the runner and worker deployment are coordinated.

I've added this upgrade policy and the explicit client-management sequence to the PR description. We are not adding a worker-startup terminator, automatic flow migration, or general per-task version-strategy framework for this PR. The current business-version exemption is not a claim of cross-SDK replay compatibility. This documents the chosen operational contract; a future incompatible release still needs release-specific upgrade validation rather than an unqualified promise that off/on is sufficient.

This was a top-level PR comment, not a resolvable inline review thread, so I am recording the response here rather than marking an unrelated thread 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.

4 participants