Add large-payload tombstone fetch and purge-result RPCs for blob auto-purge - #76
Open
wangbill (YunchuWang) wants to merge 9 commits into
Open
Add large-payload tombstone fetch and purge-result RPCs for blob auto-purge#76wangbill (YunchuWang) wants to merge 9 commits into
wangbill (YunchuWang) wants to merge 9 commits into
Conversation
Adds the authoritative bidirectional streaming RPC and its two messages (TombstonedPayload, PayloadPurged) that the backend and worker use to purge externalized large-payload blobs. The backend soft-deletes blob-externalized payload rows and streams the tombstoned tokens to a connected worker (which has storage credentials); the worker deletes each blob and acks so the backend can hard-delete the row. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Pure rename of the client->server ack message; field numbers/types unchanged and wire-compatible. Reads correctly (the row is not yet purged when the worker sends it) and the ...Ack suffix signals the upstream direction in the bidi signature. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
wangbill (YunchuWang)
marked this pull request as ready for review
July 8, 2026 23:28
The DTFx AzureManaged SDK dials only BackendService, so it needs to drain externalized large-payload blobs via that service. Mirrors the RPC already on TaskHubSidecarService; reuses TombstonedPayload and PayloadPurgeAck, which are defined in orchestrator_service.proto (already imported here). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
wangbill (YunchuWang)
marked this pull request as draft
July 9, 2026 17:27
Replaces the bidirectional streaming PurgeExternalPayloads on both TaskHubSidecarService and BackendService with two unary RPCs: GetTombstonedPayloads(limit) to fetch tombstoned large-payload rows and AckPurgedPayloads(acks) to confirm blob deletion so the backend hard-deletes those rows. Adds the four request/response messages to orchestrator_service.proto; backend_service.proto reuses them via its existing import. TombstonedPayload and PayloadPurgeAck are unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
wangbill (YunchuWang)
added a commit
to microsoft/durabletask-dotnet
that referenced
this pull request
Jul 13, 2026
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>
wangbill (YunchuWang)
added a commit
to microsoft/durabletask-dotnet
that referenced
this pull request
Jul 13, 2026
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>
wangbill (YunchuWang)
added a commit
to microsoft/durabletask-dotnet
that referenced
this pull request
Jul 13, 2026
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>
wangbill (YunchuWang)
marked this pull request as ready for review
July 15, 2026 00:05
Aligns the contract with the finalized large-payload blob auto-purge
design. The previous shape (a fetch RPC plus a bare identity-only ack)
is superseded and removed rather than extended.
protos/backend_service.proto is reverted to main. The RPCs live only on
TaskHubSidecarService in orchestrator_service.proto: the backend serves
both services on one endpoint and the worker already reaches
TaskHubSidecarService there, so authorization stays identical to the
existing worker stream and nothing is added to the backend-facing
service (design section 3).
orchestrator_service.proto:
- Replace GetTombstonedPayloads/AckPurgedPayloads with
GetLargePayloadTombstones and ReportLargePayloadPurgeResults. An
identity-only ack cannot distinguish success from an unfixable
failure, and the backend, not the worker, owns retry scheduling.
- Add LargePayloadPurgeDisposition with exactly three dispositions:
DELETED, RETRY, and QUARANTINED. There is deliberately no Discarded;
it was success-shaped and destroyed evidence.
- Add LargePayloadPurgeReason as a stable, bounded reason code. It never
carries a token or raw exception text, because tokens expose the
storage account, container, and blob path (design section 7).
BLOB_NOT_STORE_OWNED sits under DELETED rather than QUARANTINED: an
unmarked blob is an expected outcome, a customer's own blob whose
reference happened to match the token grammar, not a defect
(design section 5.5).
- Carry revision on both the tombstone and the result as a
compare-and-swap guard, so duplicate or stale reports are no-ops
without a per-row lease (design section 5.3).
- Add google.protobuf.BoolValue large_payload_auto_purge_enabled = 12 to
the existing GetWorkItemsRequest. This is a setting, not a
WorkerCapability: a repeated capability enum is presence-only and
cannot express an explicit false, so it could never turn the feature
off (design section 3.1).
- Document the token as blob:v2:{fullBlobUrl}. Legacy v1 tokens are
never tombstoned, because v1 carries a container name but not the
storage account (design sections 5.4 and 8).
Definitions are byte-identical to the canonical contract shared with
durabletask-dotnet#758.
Verified: protos/backend_service.proto diffs empty against main; field
12 was previously unused in GetWorkItemsRequest (1, 2, 3, 10, 11);
google/protobuf/wrappers.proto was already imported and is not
re-imported; all three protos compile with protoc.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Review feedback: "why do we need purgereason code?" The pushback was correct and the enum was too granular. The backend has zero branches on `reason`. LargePayloadPurgePolicy never references it, and LastFailureReason appears only in tests and one SELECT, so the field is write-only. `disposition` alone drives every backend action. That makes `reason` diagnostics, and its granularity should match the number of distinct operator responses rather than the number of distinct causes. `storageErrorCode` already carries the specific storage status, so four of the eleven values were encoding the same fact twice. Collapsed 11 values to 7: - TRANSIENT_STORAGE_FAILURE, STORAGE_ACCOUNT_UNREACHABLE, and STORAGE_AUTHORIZATION_FAILED become STORAGE_FAILURE. All three are reconfigurable or self-healing and produce the same operator response; the specific status stays available in storageErrorCode. - MALFORMED_TOKEN, INVALID_STORAGE_REQUEST, LEGACY_V1_TOKEN, and UNSUPPORTED_TOKEN_VERSION become TOKEN_NOT_PURGEABLE. Each means the token cannot be acted on and no retry changes that. storageErrorCode distinguishes the storage-rejected case, where it is populated, from the parse cases, where it is empty. STORE_CANNOT_DELETE stays separate from STORAGE_FAILURE because storage is never contacted, so storageErrorCode is empty and the two are not diagnosable the same way. BLOB_NOT_STORE_OWNED stays because without it a DELETED row cannot distinguish reclaiming a blob from deliberately leaving one alone, and the purged-count metric depends on that distinction staying countable. Dispositions are unchanged: the enum still has exactly the same four values with the same numbers. UNSUPPORTED_TOKEN_VERSION was previously a RETRY reason and folds into a QUARANTINED one, but no row's disposition flips as a result; only the reason narrows, and an unreadable token still cannot be deleted. Also from review: - Removed every design-doc section reference. This is a public repo and external readers cannot resolve them. Five sites are rewritten to be self-contained: the token field, DISPOSITION_DELETED, the reason enum header, BLOB_NOT_STORE_OWNED, and the former LEGACY_V1_TOKEN. The file now contains no non-ASCII bytes, which also removes the mojibake the section signs produced in the diff view. - Documented why UNSPECIFIED exists, in answer to "why need unspecified when can that happen" and "no need for this?". It can happen: proto3 has no field presence for scalars, so an unset field arrives as 0. If 0 meant DELETED, a client that failed to set the field would make the backend delete tombstones and orphan the blobs permanently. It is also mandatory, since proto3 requires the first enum value to be zero, and WorkerCapability in this same file follows the identical convention. - Corrected the ReportLargePayloadPurgeResults comment. It claimed the backend "reschedules RETRY with a reason-appropriate next attempt", which is false because the backend never reads reason. It now states that the backend branches solely on disposition and that the worker never computes a retry delay. Definitions remain byte-identical to the canonical contract shared with durabletask-dotnet#758. Verified: protos/backend_service.proto still diffs empty against main; all seven removed value names occur zero times; the reason enum has exactly 7 values and the disposition enum still has 4; protoc compiles all three protos. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Comment-only. No enum value, field number, or disposition changes; the compiled descriptor surface is byte-for-byte identical before and after. The reason enum grouped its values under "--- Reported with DELETED ---", "--- Reported with RETRY ---", and "--- Reported with QUARANTINED ---" banners, which asserted a 1:1 reason-to-disposition mapping. That mapping does not hold. TOKEN_NOT_PURGEABLE sat under the QUARANTINED banner and claimed "no retry can change that: ... it names a version this worker does not support", which is false: the unknown-version case is reported with RETRY, because a newer worker can read that token and an SDK upgrade resolves it. The banners were the actual defect rather than that one sentence, so fixing the sentence alone would have left the grouping asserting something untrue. The banners are removed, and each value now names its own disposition inline. TOKEN_NOT_PURGEABLE documents both dispositions explicitly and states that a consumer must not assume either. The enum header now states that reason and disposition are orthogonal with no fixed mapping, and that no reason-to-disposition mapping may be asserted anywhere. This matters because this file is the public contract: a backend implementer reading the old grouping could reasonably have added an assertion that TOKEN_NOT_PURGEABLE never arrives with RETRY, which would reject valid reports. Also records why the asymmetry is deliberate. Quarantining an unknown-version token is permanent and unrecoverable, whereas a retry that never succeeds only leaves the row idle and visible, so the recoverable failure mode is the correct default. Definitions remain byte-identical to the canonical contract shared with durabletask-dotnet#758. Verified: the descriptor surface (all enum values and numbers, all field numbers, all RPC signatures) is unchanged, 63 entries before and after; the file remains at 0 non-ASCII bytes; the three group banners and the false claim occur zero times; protoc compiles; backend_service.proto still diffs empty against main. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Both fields were 100% write-only across the system: the backend persists LastFailureReason and StorageErrorCode but no SELECT, WHERE, API, or alert reads either, and every backend action branches on disposition alone. The number of distinct operator responses driven by reason is zero, so the granularity rule that produced the 11->7 collapse, applied to the field itself, says it should not exist. Failure detail already lives in the worker's telemetry at full exception fidelity; these columns were a lossy copy nobody queried. Deleting reason also removes the reason-to-disposition mapping hazard entirely, including the TOKEN_NOT_PURGEABLE case that spanned two dispositions. No reserved statements: these messages have never shipped in a release, so no deployed peer has ever seen field 6 or 7. disposition stays at field 5; the numbering gap costs nothing and renumbering would be one more way for the three copies to diverge. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 38169e74-8326-44b2-ae45-77d14e34ba32
The correlation advice named a column a worker cannot obtain. LargePayloadTombstone carries only partitionId, instanceKey, payloadId, token, and revision, so LastAttemptAt is never sent to the worker and it has no way to query it. The remaining three identifiers are real fields on both LargePayloadTombstone and LargePayloadPurgeResult, so the advice stays correct and becomes executable. LastAttemptAt is also a backend SQL column name rather than a proto field. This repo is public and cannot reference it, so the comment would rot silently. Comment-only: generated output is byte-identical before and after. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 38169e74-8326-44b2-ae45-77d14e34ba32
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
Adds the authoritative proto contract for the large-payload blob auto-purge feature. This is the source-of-truth definition that the other PRs vendor locally and re-sync from once this merges:
The definitions here are byte-identical to the canonical contract shared with durabletask-dotnet #758.
Context
When an orchestration payload exceeds a threshold it is externalized to Azure Blob Storage and a token is persisted in the backend's SQL
Payloads.Textcolumn instead of the payload bytes. Purging the orchestration removes the SQL state, but the backend cannot delete the blob because it does not hold the customer's storage credentials, so the blob is orphaned.Fix: the backend keeps the durable cleanup work and hands it to a worker that does have storage credentials. The backend records a tombstone for each externalized payload leaving live state; the worker fetches a bounded batch of due tombstones, deletes each blob, and reports the outcome of every attempt. The backend then resolves, reschedules, or quarantines each row.
Change
Edits only
protos/orchestrator_service.proto. Nothing is added to the backend-facing service: the backend serves both services on one endpoint and the worker already reachesTaskHubSidecarServicethere, so authorization stays identical to the existing worker stream.Opt-in on the existing handshake —
google.protobuf.BoolValue large_payload_auto_purge_enabled = 12on the existingGetWorkItemsRequest.GetWorkItemsis already task-hub scoped and authenticated, so the setting binds to that task hub without a new RPC. This is a setting, not aWorkerCapability:WORKER_CAPABILITY_LARGE_PAYLOADSmeans a worker can resolve externalized payloads, not that the customer opted into deleting them. Arepeatedcapability enum is presence-only and could never express an explicitfalse, so it could not turn the feature off.Two bounded unary RPCs, placed immediately after
GetWorkItems:GetLargePayloadTombstonesReportLargePayloadPurgeResultsDispositions — exactly three:
DELETED,RETRY,QUARANTINED. The split is by whether a failure can self-heal. There is deliberately noDiscarded: it was success-shaped and destroyed evidence.No reason or error-code fields -
dispositionis the only outcome field onLargePayloadPurgeResult. Earlier revisions of this PR also carried aLargePayloadPurgeReasonenum and astorageErrorCodestring; both were removed because they were write-only. The backend persists them, but noSELECT,WHERE, API, or alert reads either one, and every backend action branches ondispositionalone. Failure detail already lives in the worker's telemetry at full exception fidelity, where a row is correlated by(partitionId, instanceKey, payloadId), so the fields were a lossy copy nobody queried. Noreservedstatements were added: these messages have never shipped in a release, so no deployed peer has ever seen field6or7.dispositionstays at field5rather than being renumbered to close the gap.revision— carried on both the tombstone and the result as a compare-and-swap guard, so duplicate or stale reports are no-ops without a per-row lease.Tokens — documented as
blob:v2:{fullBlobUrl}. Legacyv1tokens are never tombstoned:v1carries a container name but not the storage account, so a delete cannot be verified against the configured account. The backend hard-deletesv1payload rows instead, and av1token reaching the worker is an invariant violation that is quarantined rather than discarded.Supersedes the earlier shape
This replaces the previous draft contract, which is fully removed rather than extended:
GetTombstonedPayloads/AckPurgedPayloads→ renamed and reshaped. The old pair was a fetch plus a bare identity-only ack, which cannot distinguish success from an unfixable failure and left retry scheduling ambiguous.TombstonedPayload,PayloadPurgeAck,GetTombstonedPayloadsRequest,GetTombstonedPayloadsResponse,AckPurgedPayloadsRequest,AckPurgedPayloadsResponse→ removed.protos/backend_service.proto→ reverted; it is now byte-identical tomain.Because none of the related PRs has merged, this is not a compatibility constraint.
Verification
git diff origin/main -- protos/backend_service.protois empty.12was previously unused inGetWorkItemsRequest(which used1, 2, 3, 10, 11); no field number collides.google/protobuf/wrappers.protowas already imported; no duplicate import added.protoc(no errors; the only warnings are pre-existing unused imports inbackend_service.proto). Descriptor introspection confirms the field numbers, RPC wiring, and enum values, and that no removed symbol survives.No language stubs were regenerated: this is a proto-only repo (SDKs consume the protos via git submodule and generate code at build time into gitignored
build/dirs; no generated stubs are checked in).