From ccee539580c2aa3559d1b4a57513bad90c198991 Mon Sep 17 00:00:00 2001 From: wangbill Date: Wed, 8 Jul 2026 14:45:34 -0700 Subject: [PATCH 1/9] Add PurgeExternalPayloads RPC for large-payload blob auto-purge 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> --- protos/orchestrator_service.proto | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/protos/orchestrator_service.proto b/protos/orchestrator_service.proto index 3d9194a..a762a19 100644 --- a/protos/orchestrator_service.proto +++ b/protos/orchestrator_service.proto @@ -786,6 +786,12 @@ service TaskHubSidecarService { rpc PurgeInstances(PurgeInstancesRequest) returns (PurgeInstancesResponse); rpc GetWorkItems(GetWorkItemsRequest) returns (stream WorkItem); + + // Streams tombstoned large-payload blob tokens to a connected worker for deletion; the worker + // acks each deleted blob so the backend can hard-delete the corresponding row. Idempotent and + // safe with multiple worker replicas (unacked rows stay tombstoned and are re-streamed). + rpc PurgeExternalPayloads(stream PayloadPurged) returns (stream TombstonedPayload); + rpc CompleteActivityTask(ActivityResponse) returns (CompleteTaskResponse); rpc CompleteOrchestratorTask(OrchestratorResponse) returns (CompleteTaskResponse); rpc CompleteEntityTask(EntityBatchResult) returns (CompleteTaskResponse); @@ -825,6 +831,21 @@ service TaskHubSidecarService { rpc SkipGracefulOrchestrationTerminations(SkipGracefulOrchestrationTerminationsRequest) returns (SkipGracefulOrchestrationTerminationsResponse); } +// server -> client: a tombstoned payload row whose blob the worker must delete. +message TombstonedPayload { + int32 partitionId = 1; + int64 instanceKey = 2; + int64 payloadId = 3; + string token = 4; // e.g. "blob:v1::" +} + +// client -> server: ack sent after the worker deletes the blob; backend hard-deletes that row. +message PayloadPurged { + int32 partitionId = 1; + int64 instanceKey = 2; + int64 payloadId = 3; +} + message GetWorkItemsRequest { int32 maxConcurrentOrchestrationWorkItems = 1; int32 maxConcurrentActivityWorkItems = 2; From 26213a95c70db0a44f629af0ed4af4560884d114 Mon Sep 17 00:00:00 2001 From: wangbill Date: Wed, 8 Jul 2026 15:30:17 -0700 Subject: [PATCH 2/9] Rename PayloadPurged to PayloadPurgeAck 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> --- protos/orchestrator_service.proto | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/protos/orchestrator_service.proto b/protos/orchestrator_service.proto index a762a19..a67bf3b 100644 --- a/protos/orchestrator_service.proto +++ b/protos/orchestrator_service.proto @@ -790,7 +790,7 @@ service TaskHubSidecarService { // Streams tombstoned large-payload blob tokens to a connected worker for deletion; the worker // acks each deleted blob so the backend can hard-delete the corresponding row. Idempotent and // safe with multiple worker replicas (unacked rows stay tombstoned and are re-streamed). - rpc PurgeExternalPayloads(stream PayloadPurged) returns (stream TombstonedPayload); + rpc PurgeExternalPayloads(stream PayloadPurgeAck) returns (stream TombstonedPayload); rpc CompleteActivityTask(ActivityResponse) returns (CompleteTaskResponse); rpc CompleteOrchestratorTask(OrchestratorResponse) returns (CompleteTaskResponse); @@ -840,7 +840,7 @@ message TombstonedPayload { } // client -> server: ack sent after the worker deletes the blob; backend hard-deletes that row. -message PayloadPurged { +message PayloadPurgeAck { int32 partitionId = 1; int64 instanceKey = 2; int64 payloadId = 3; From c28312c2ceef4ed297836f7d7abe332a60963387 Mon Sep 17 00:00:00 2001 From: wangbill Date: Thu, 9 Jul 2026 10:27:01 -0700 Subject: [PATCH 3/9] Add PurgeExternalPayloads RPC to BackendService 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> --- protos/backend_service.proto | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/protos/backend_service.proto b/protos/backend_service.proto index 29d19c7..23b49a0 100644 --- a/protos/backend_service.proto +++ b/protos/backend_service.proto @@ -57,6 +57,11 @@ service BackendService { // Starts a server stream for receiving work items rpc GetWorkItems (GetWorkItemsRequest) returns (stream WorkItem); + // Streams tombstoned large-payload blob tokens to a connected worker for deletion; the worker + // acks each deleted blob so the backend can hard-delete the corresponding row. Idempotent and + // safe with multiple worker replicas (unacked rows stay tombstoned and are re-streamed). + rpc PurgeExternalPayloads(stream PayloadPurgeAck) returns (stream TombstonedPayload); + // Gets orchestration runtime state (history, etc.) for a given orchestration instance. rpc GetOrchestrationRuntimeState (GetOrchestrationRuntimeStateRequest) returns (GetOrchestrationRuntimeStateResponse); From 008f92348a10542c912a1a87df512e61e1f003d4 Mon Sep 17 00:00:00 2001 From: wangbill Date: Mon, 13 Jul 2026 11:44:41 -0700 Subject: [PATCH 4/9] Replace bidi PurgeExternalPayloads with two unary RPCs 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> --- protos/backend_service.proto | 13 +++++++++---- protos/orchestrator_service.proto | 32 +++++++++++++++++++++++++++---- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/protos/backend_service.proto b/protos/backend_service.proto index 23b49a0..22f03a9 100644 --- a/protos/backend_service.proto +++ b/protos/backend_service.proto @@ -57,10 +57,15 @@ service BackendService { // Starts a server stream for receiving work items rpc GetWorkItems (GetWorkItemsRequest) returns (stream WorkItem); - // Streams tombstoned large-payload blob tokens to a connected worker for deletion; the worker - // acks each deleted blob so the backend can hard-delete the corresponding row. Idempotent and - // safe with multiple worker replicas (unacked rows stay tombstoned and are re-streamed). - rpc PurgeExternalPayloads(stream PayloadPurgeAck) returns (stream TombstonedPayload); + // Fetches up to `limit` tombstoned large-payload rows whose external blobs the worker must + // delete. The worker (which has storage credentials) deletes each blob, then calls + // AckPurgedPayloads so the backend can hard-delete those rows. Idempotent and safe under + // retries/duplicate callers (unacked rows stay tombstoned and are returned again). + rpc GetTombstonedPayloads(GetTombstonedPayloadsRequest) returns (GetTombstonedPayloadsResponse); + + // Acks tombstoned payloads whose external blobs the worker has deleted; the backend hard-deletes + // the corresponding rows. Idempotent and safe under retries/duplicate callers. + rpc AckPurgedPayloads(AckPurgedPayloadsRequest) returns (AckPurgedPayloadsResponse); // Gets orchestration runtime state (history, etc.) for a given orchestration instance. rpc GetOrchestrationRuntimeState (GetOrchestrationRuntimeStateRequest) returns (GetOrchestrationRuntimeStateResponse); diff --git a/protos/orchestrator_service.proto b/protos/orchestrator_service.proto index a67bf3b..e1faae5 100644 --- a/protos/orchestrator_service.proto +++ b/protos/orchestrator_service.proto @@ -787,10 +787,15 @@ service TaskHubSidecarService { rpc GetWorkItems(GetWorkItemsRequest) returns (stream WorkItem); - // Streams tombstoned large-payload blob tokens to a connected worker for deletion; the worker - // acks each deleted blob so the backend can hard-delete the corresponding row. Idempotent and - // safe with multiple worker replicas (unacked rows stay tombstoned and are re-streamed). - rpc PurgeExternalPayloads(stream PayloadPurgeAck) returns (stream TombstonedPayload); + // Fetches up to `limit` tombstoned large-payload rows whose external blobs the worker must + // delete. The worker (which has storage credentials) deletes each blob, then calls + // AckPurgedPayloads so the backend can hard-delete those rows. Idempotent and safe under + // retries/duplicate callers (unacked rows stay tombstoned and are returned again). + rpc GetTombstonedPayloads(GetTombstonedPayloadsRequest) returns (GetTombstonedPayloadsResponse); + + // Acks tombstoned payloads whose external blobs the worker has deleted; the backend hard-deletes + // the corresponding rows. Idempotent and safe under retries/duplicate callers. + rpc AckPurgedPayloads(AckPurgedPayloadsRequest) returns (AckPurgedPayloadsResponse); rpc CompleteActivityTask(ActivityResponse) returns (CompleteTaskResponse); rpc CompleteOrchestratorTask(OrchestratorResponse) returns (CompleteTaskResponse); @@ -846,6 +851,25 @@ message PayloadPurgeAck { int64 payloadId = 3; } +// client -> server: request up to `limit` tombstoned payload rows to purge. +message GetTombstonedPayloadsRequest { + int32 limit = 1; +} + +// server -> client: the tombstoned payload rows whose blobs the worker must delete. +message GetTombstonedPayloadsResponse { + repeated TombstonedPayload payloads = 1; +} + +// client -> server: acks for payloads whose blobs the worker has deleted. +message AckPurgedPayloadsRequest { + repeated PayloadPurgeAck acks = 1; +} + +// server -> client: response acknowledging the hard-deletes. +message AckPurgedPayloadsResponse { +} + message GetWorkItemsRequest { int32 maxConcurrentOrchestrationWorkItems = 1; int32 maxConcurrentActivityWorkItems = 2; From 6ea941f47cb2ba852d43a248669a7cf0e2c2e352 Mon Sep 17 00:00:00 2001 From: wangbill Date: Tue, 11 Aug 2026 14:51:28 -0700 Subject: [PATCH 5/9] Replace purge contract with disposition-aware large-payload RPCs 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> --- protos/backend_service.proto | 10 -- protos/orchestrator_service.proto | 158 +++++++++++++++++++++++++----- 2 files changed, 131 insertions(+), 37 deletions(-) diff --git a/protos/backend_service.proto b/protos/backend_service.proto index 22f03a9..29d19c7 100644 --- a/protos/backend_service.proto +++ b/protos/backend_service.proto @@ -57,16 +57,6 @@ service BackendService { // Starts a server stream for receiving work items rpc GetWorkItems (GetWorkItemsRequest) returns (stream WorkItem); - // Fetches up to `limit` tombstoned large-payload rows whose external blobs the worker must - // delete. The worker (which has storage credentials) deletes each blob, then calls - // AckPurgedPayloads so the backend can hard-delete those rows. Idempotent and safe under - // retries/duplicate callers (unacked rows stay tombstoned and are returned again). - rpc GetTombstonedPayloads(GetTombstonedPayloadsRequest) returns (GetTombstonedPayloadsResponse); - - // Acks tombstoned payloads whose external blobs the worker has deleted; the backend hard-deletes - // the corresponding rows. Idempotent and safe under retries/duplicate callers. - rpc AckPurgedPayloads(AckPurgedPayloadsRequest) returns (AckPurgedPayloadsResponse); - // Gets orchestration runtime state (history, etc.) for a given orchestration instance. rpc GetOrchestrationRuntimeState (GetOrchestrationRuntimeStateRequest) returns (GetOrchestrationRuntimeStateResponse); diff --git a/protos/orchestrator_service.proto b/protos/orchestrator_service.proto index e1faae5..fc007e9 100644 --- a/protos/orchestrator_service.proto +++ b/protos/orchestrator_service.proto @@ -787,15 +787,16 @@ service TaskHubSidecarService { rpc GetWorkItems(GetWorkItemsRequest) returns (stream WorkItem); - // Fetches up to `limit` tombstoned large-payload rows whose external blobs the worker must - // delete. The worker (which has storage credentials) deletes each blob, then calls - // AckPurgedPayloads so the backend can hard-delete those rows. Idempotent and safe under - // retries/duplicate callers (unacked rows stay tombstoned and are returned again). - rpc GetTombstonedPayloads(GetTombstonedPayloadsRequest) returns (GetTombstonedPayloadsResponse); + // Returns a bounded, deterministically ordered batch of due large-payload tombstones whose + // external blobs the worker must delete. Scoped to the caller's authenticated task hub. + // Only rows that are pending and whose next attempt time has arrived are returned; a row stays + // pending until its outcome is reported, so this is safe under retries and duplicate callers. + rpc GetLargePayloadTombstones(GetLargePayloadTombstonesRequest) returns (GetLargePayloadTombstonesResponse); - // Acks tombstoned payloads whose external blobs the worker has deleted; the backend hard-deletes - // the corresponding rows. Idempotent and safe under retries/duplicate callers. - rpc AckPurgedPayloads(AckPurgedPayloadsRequest) returns (AckPurgedPayloadsResponse); + // Reports the outcome of each attempted blob deletion. The backend owns retry scheduling: it + // deletes rows reported as DELETED, reschedules RETRY with a reason-appropriate next attempt, + // and moves QUARANTINED rows out of the active fetch while preserving their evidence. + rpc ReportLargePayloadPurgeResults(ReportLargePayloadPurgeResultsRequest) returns (ReportLargePayloadPurgeResultsResponse); rpc CompleteActivityTask(ActivityResponse) returns (CompleteTaskResponse); rpc CompleteOrchestratorTask(OrchestratorResponse) returns (CompleteTaskResponse); @@ -836,38 +837,131 @@ service TaskHubSidecarService { rpc SkipGracefulOrchestrationTerminations(SkipGracefulOrchestrationTerminationsRequest) returns (SkipGracefulOrchestrationTerminationsResponse); } -// server -> client: a tombstoned payload row whose blob the worker must delete. -message TombstonedPayload { - int32 partitionId = 1; - int64 instanceKey = 2; - int64 payloadId = 3; - string token = 4; // e.g. "blob:v1::" +// server -> client: one tombstoned large-payload row whose external blob the worker must delete. +message LargePayloadTombstone { + int32 partitionId = 1; + int64 instanceKey = 2; + int64 payloadId = 3; + + // A self-describing SDK v2 token: "blob:v2:{fullBlobUrl}". + // Legacy v1 tokens are never tombstoned: v1 carries a container name but not the storage + // account, so a delete against the configured account cannot be verified. The backend + // hard-deletes v1 payload rows instead (design §8). + string token = 4; + + // Optimistic-concurrency guard. The worker echoes this value back unmodified in the result so + // the backend can reject duplicate or stale reports without taking a per-row lease. + int64 revision = 5; } -// client -> server: ack sent after the worker deletes the blob; backend hard-deletes that row. -message PayloadPurgeAck { +// The outcome of a single blob deletion attempt. The split is by whether a failure can self-heal. +enum LargePayloadPurgeDisposition { + LARGE_PAYLOAD_PURGE_DISPOSITION_UNSPECIFIED = 0; + + // Terminal success. The blob was deleted, was already absent, or was deliberately left in + // place because it is not owned by the payload store (design §5.5). The backend deletes the + // tombstone in all three cases. + LARGE_PAYLOAD_PURGE_DISPOSITION_DELETED = 1; + + // The failure may self-heal, so the row stays pending and the backend sets the next attempt. + LARGE_PAYLOAD_PURGE_DISPOSITION_RETRY = 2; + + // A deterministic failure or protocol violation that retrying can never fix. The backend + // preserves the evidence, alerts, and stops automatic retries. + LARGE_PAYLOAD_PURGE_DISPOSITION_QUARANTINED = 3; +} + +// A stable, bounded reason for a disposition. Never carries a token or raw exception text +// (design §7: tokens expose storage account, container, and blob path). +enum LargePayloadPurgeReason { + LARGE_PAYLOAD_PURGE_REASON_UNSPECIFIED = 0; + + // --- Reported with DELETED --- + + // The blob was deleted by this attempt. + LARGE_PAYLOAD_PURGE_REASON_BLOB_DELETED = 1; + + // The blob was already absent. Deletion is idempotent, so this is a success. + LARGE_PAYLOAD_PURGE_REASON_BLOB_ALREADY_ABSENT = 2; + + // The blob did not carry the payload store's ownership marker, so it was left untouched + // (design §5.5). This is an expected outcome, not a defect: the token text merely matched the + // v2 grammar. The tombstone is still resolved because the blob is not the store's to delete. + LARGE_PAYLOAD_PURGE_REASON_BLOB_NOT_STORE_OWNED = 3; + + // --- Reported with RETRY --- + + // Network failure, timeout, storage outage, throttling, or a 5xx response. + LARGE_PAYLOAD_PURGE_REASON_TRANSIENT_STORAGE_FAILURE = 10; + + // The registered payload store does not implement deletion. Every payload would fail the same + // way, so the work is kept recoverable until an operator registers a store that can delete. + LARGE_PAYLOAD_PURGE_REASON_STORE_CANNOT_DELETE = 11; + + // The token is well formed but points at a storage account this worker's credential cannot + // reach. Recoverable after a configuration or credential change. + LARGE_PAYLOAD_PURGE_REASON_STORAGE_ACCOUNT_UNREACHABLE = 12; + + // The token uses a recognized-but-newer version prefix this worker does not understand. + // Recoverable after an SDK upgrade, so it earns a long defer rather than quarantine. + LARGE_PAYLOAD_PURGE_REASON_UNSUPPORTED_TOKEN_VERSION = 13; + + // Authorization failed in a way that may be transient or reconfigurable (401/403). + LARGE_PAYLOAD_PURGE_REASON_STORAGE_AUTHORIZATION_FAILED = 14; + + // --- Reported with QUARANTINED --- + + // The token carries a known version prefix but its body does not parse. Because the SDK and + // backend control both sides of the protocol, this indicates a producer, corruption, or + // compatibility bug. + LARGE_PAYLOAD_PURGE_REASON_MALFORMED_TOKEN = 20; + + // Storage rejected a request generated from a well-formed token as permanently invalid + // (HTTP 400, e.g. InvalidUri / InvalidResourceName). Retrying can never succeed. + LARGE_PAYLOAD_PURGE_REASON_INVALID_STORAGE_REQUEST = 21; + + // A legacy v1 token reached the worker. This is an invariant violation, because the backend + // excludes v1 at insertion time. It cannot be safely deleted (no storage account in the token) + // and cannot be fixed by retrying, so the evidence is preserved instead (design §6). + LARGE_PAYLOAD_PURGE_REASON_LEGACY_V1_TOKEN = 22; +} + +// client -> server: the outcome of exactly one tombstoned row. +message LargePayloadPurgeResult { + // Row identity, echoed from the corresponding LargePayloadTombstone. int32 partitionId = 1; int64 instanceKey = 2; - int64 payloadId = 3; + int64 payloadId = 3; + + // Echoed unmodified from the fetched tombstone; used as a compare-and-swap guard. + int64 revision = 4; + + LargePayloadPurgeDisposition disposition = 5; + LargePayloadPurgeReason reason = 6; + + // Optional bounded, sanitized storage status or error code for diagnostics + // (for example "BlobNotFound" or "409"). Must never contain a token or raw exception text. + string storageErrorCode = 7; } -// client -> server: request up to `limit` tombstoned payload rows to purge. -message GetTombstonedPayloadsRequest { +// client -> server: request up to `limit` due tombstones for the caller's task hub. +message GetLargePayloadTombstonesRequest { + // The maximum number of rows to return. The service clamps this to its own maximum. int32 limit = 1; } -// server -> client: the tombstoned payload rows whose blobs the worker must delete. -message GetTombstonedPayloadsResponse { - repeated TombstonedPayload payloads = 1; +// server -> client: the due tombstones whose blobs the worker must delete. +message GetLargePayloadTombstonesResponse { + repeated LargePayloadTombstone tombstones = 1; } -// client -> server: acks for payloads whose blobs the worker has deleted. -message AckPurgedPayloadsRequest { - repeated PayloadPurgeAck acks = 1; +// client -> server: a bounded batch of purge outcomes. +message ReportLargePayloadPurgeResultsRequest { + repeated LargePayloadPurgeResult results = 1; } -// server -> client: response acknowledging the hard-deletes. -message AckPurgedPayloadsResponse { +// server -> client: acknowledgement that the reported outcomes were recorded. +message ReportLargePayloadPurgeResultsResponse { } message GetWorkItemsRequest { @@ -877,6 +971,16 @@ message GetWorkItemsRequest { repeated WorkerCapability capabilities = 10; WorkItemFilters workItemFilters = 11; + + // Task-hub scoped opt-in for large-payload blob auto-purge. + // + // This is a setting, not a WorkerCapability: WORKER_CAPABILITY_LARGE_PAYLOADS means a worker + // *can resolve* externalized payloads, not that the customer opted into *deleting* them. + // A capability list is presence-only and cannot express an explicit false, so a three-state + // wrapper is used instead: + // absent -> the worker has no opinion (older SDK); the persisted setting is unchanged. + // true/false -> an explicit customer choice; persisted only when the value actually differs. + google.protobuf.BoolValue large_payload_auto_purge_enabled = 12; } enum WorkerCapability { From 0e5e4c6ae6f1c089e668b99438183f32cfe34b28 Mon Sep 17 00:00:00 2001 From: wangbill Date: Tue, 11 Aug 2026 16:03:24 -0700 Subject: [PATCH 6/9] Narrow LargePayloadPurgeReason to 7 values and drop design-doc refs 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> --- protos/orchestrator_service.proto | 89 ++++++++++++++++--------------- 1 file changed, 47 insertions(+), 42 deletions(-) diff --git a/protos/orchestrator_service.proto b/protos/orchestrator_service.proto index fc007e9..02ec0ae 100644 --- a/protos/orchestrator_service.proto +++ b/protos/orchestrator_service.proto @@ -793,9 +793,10 @@ service TaskHubSidecarService { // pending until its outcome is reported, so this is safe under retries and duplicate callers. rpc GetLargePayloadTombstones(GetLargePayloadTombstonesRequest) returns (GetLargePayloadTombstonesResponse); - // Reports the outcome of each attempted blob deletion. The backend owns retry scheduling: it - // deletes rows reported as DELETED, reschedules RETRY with a reason-appropriate next attempt, - // and moves QUARANTINED rows out of the active fetch while preserving their evidence. + // Reports the outcome of each attempted blob deletion. The backend owns retry scheduling and + // branches solely on `disposition`: it deletes rows reported as DELETED, reschedules RETRY on + // its own backoff, and moves QUARANTINED rows out of the active fetch while preserving their + // evidence. The worker never computes a retry delay. rpc ReportLargePayloadPurgeResults(ReportLargePayloadPurgeResultsRequest) returns (ReportLargePayloadPurgeResultsResponse); rpc CompleteActivityTask(ActivityResponse) returns (CompleteTaskResponse); @@ -846,7 +847,7 @@ message LargePayloadTombstone { // A self-describing SDK v2 token: "blob:v2:{fullBlobUrl}". // Legacy v1 tokens are never tombstoned: v1 carries a container name but not the storage // account, so a delete against the configured account cannot be verified. The backend - // hard-deletes v1 payload rows instead (design §8). + // hard-deletes v1 payload rows instead. string token = 4; // Optimistic-concurrency guard. The worker echoes this value back unmodified in the result so @@ -856,11 +857,16 @@ message LargePayloadTombstone { // The outcome of a single blob deletion attempt. The split is by whether a failure can self-heal. enum LargePayloadPurgeDisposition { + // Required: proto3 reserves 0 as the first value, and scalars have no field presence, so an + // unset field arrives as 0. Keeping 0 meaningless is load-bearing here: if 0 meant DELETED, a + // client that failed to set this field would make the backend delete tombstones and orphan the + // blobs permanently. The backend must reject a result carrying this value. LARGE_PAYLOAD_PURGE_DISPOSITION_UNSPECIFIED = 0; - // Terminal success. The blob was deleted, was already absent, or was deliberately left in - // place because it is not owned by the payload store (design §5.5). The backend deletes the - // tombstone in all three cases. + // Terminal success: the tombstone is resolved and the backend deletes it. Covers the blob being + // deleted, the blob already being absent, and the blob being deliberately left in place because + // the payload store does not own it. All three are terminal because none of them can be + // improved by trying again. LARGE_PAYLOAD_PURGE_DISPOSITION_DELETED = 1; // The failure may self-heal, so the row stays pending and the backend sets the next attempt. @@ -871,59 +877,58 @@ enum LargePayloadPurgeDisposition { LARGE_PAYLOAD_PURGE_DISPOSITION_QUARANTINED = 3; } -// A stable, bounded reason for a disposition. Never carries a token or raw exception text -// (design §7: tokens expose storage account, container, and blob path). +// Why a row received its disposition. Diagnostic only: the backend acts on `disposition` alone and +// never branches on this value, so it exists to make a stuck or non-reclaiming ledger explainable +// without access to worker logs, which run in the customer's process. +// +// Deliberately coarse. Granularity matches the number of distinct operator responses, not the +// number of distinct causes, because `storageErrorCode` already carries the specific storage status. +// Values must never carry a token or raw exception text: a token exposes the storage account, +// container, and blob path. enum LargePayloadPurgeReason { LARGE_PAYLOAD_PURGE_REASON_UNSPECIFIED = 0; // --- Reported with DELETED --- - // The blob was deleted by this attempt. + // The blob was deleted by this attempt, reclaiming its bytes. LARGE_PAYLOAD_PURGE_REASON_BLOB_DELETED = 1; - // The blob was already absent. Deletion is idempotent, so this is a success. + // The blob was already absent, so deletion was a no-op and no bytes were reclaimed by this + // attempt. Deletion is idempotent, so this is a success rather than a failure. Reported + // separately from BLOB_DELETED because a high rate of it indicates duplicate tombstones. LARGE_PAYLOAD_PURGE_REASON_BLOB_ALREADY_ABSENT = 2; - // The blob did not carry the payload store's ownership marker, so it was left untouched - // (design §5.5). This is an expected outcome, not a defect: the token text merely matched the - // v2 grammar. The tombstone is still resolved because the blob is not the store's to delete. + // The blob was left in place because it did not carry the payload store's ownership marker, + // meaning the store did not write it. The token text merely matched the v2 grammar; the payload + // column is customer-writable, so matching text is not proof of ownership. This is an expected + // outcome rather than a defect. The tombstone is still resolved, because a blob the store does + // not own will never become deletable and retrying forever would leak the row. Reported + // separately from BLOB_DELETED so that "resolved without reclaiming bytes" stays countable. LARGE_PAYLOAD_PURGE_REASON_BLOB_NOT_STORE_OWNED = 3; // --- Reported with RETRY --- - // Network failure, timeout, storage outage, throttling, or a 5xx response. - LARGE_PAYLOAD_PURGE_REASON_TRANSIENT_STORAGE_FAILURE = 10; + // The deletion failed against storage: network failure, timeout, outage, throttling, an + // unreachable account, or an authorization failure. All of these are reconfigurable or + // self-healing, and they are not subdivided here because `storageErrorCode` already carries the + // specific status for diagnostics. Subdividing would encode the same fact twice. + LARGE_PAYLOAD_PURGE_REASON_STORAGE_FAILURE = 10; - // The registered payload store does not implement deletion. Every payload would fail the same - // way, so the work is kept recoverable until an operator registers a store that can delete. + // The registered payload store does not implement deletion. Every payload fails the same way, + // so this is a deployment-wide condition rather than a per-row one, and it is kept recoverable + // until an operator registers a store that can delete. `storageErrorCode` is empty because + // storage was never contacted, which is why this cannot be folded into STORAGE_FAILURE. LARGE_PAYLOAD_PURGE_REASON_STORE_CANNOT_DELETE = 11; - // The token is well formed but points at a storage account this worker's credential cannot - // reach. Recoverable after a configuration or credential change. - LARGE_PAYLOAD_PURGE_REASON_STORAGE_ACCOUNT_UNREACHABLE = 12; - - // The token uses a recognized-but-newer version prefix this worker does not understand. - // Recoverable after an SDK upgrade, so it earns a long defer rather than quarantine. - LARGE_PAYLOAD_PURGE_REASON_UNSUPPORTED_TOKEN_VERSION = 13; - - // Authorization failed in a way that may be transient or reconfigurable (401/403). - LARGE_PAYLOAD_PURGE_REASON_STORAGE_AUTHORIZATION_FAILED = 14; - // --- Reported with QUARANTINED --- - // The token carries a known version prefix but its body does not parse. Because the SDK and - // backend control both sides of the protocol, this indicates a producer, corruption, or - // compatibility bug. - LARGE_PAYLOAD_PURGE_REASON_MALFORMED_TOKEN = 20; - - // Storage rejected a request generated from a well-formed token as permanently invalid - // (HTTP 400, e.g. InvalidUri / InvalidResourceName). Retrying can never succeed. - LARGE_PAYLOAD_PURGE_REASON_INVALID_STORAGE_REQUEST = 21; - - // A legacy v1 token reached the worker. This is an invariant violation, because the backend - // excludes v1 at insertion time. It cannot be safely deleted (no storage account in the token) - // and cannot be fixed by retrying, so the evidence is preserved instead (design §6). - LARGE_PAYLOAD_PURGE_REASON_LEGACY_V1_TOKEN = 22; + // The token cannot be acted on and no retry can change that: its body does not parse, it names + // a version this worker does not support, or storage rejected it as permanently invalid. The + // producer and consumer of this token are both controlled by the SDK and backend, so reaching + // this state indicates a producer, corruption, or compatibility bug and the evidence is + // preserved for investigation rather than discarded. `storageErrorCode` distinguishes the + // storage-rejected case, where it is populated, from the parse cases, where it is empty. + LARGE_PAYLOAD_PURGE_REASON_TOKEN_NOT_PURGEABLE = 20; } // client -> server: the outcome of exactly one tombstoned row. From 0551d87712f797619bdfa057e606556e32e6b6f6 Mon Sep 17 00:00:00 2001 From: wangbill Date: Tue, 11 Aug 2026 16:16:27 -0700 Subject: [PATCH 7/9] Fix false reason-to-disposition mapping in LargePayloadPurgeReason 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> --- protos/orchestrator_service.proto | 69 +++++++++++++++++-------------- 1 file changed, 39 insertions(+), 30 deletions(-) diff --git a/protos/orchestrator_service.proto b/protos/orchestrator_service.proto index 02ec0ae..863f41b 100644 --- a/protos/orchestrator_service.proto +++ b/protos/orchestrator_service.proto @@ -885,49 +885,58 @@ enum LargePayloadPurgeDisposition { // number of distinct causes, because `storageErrorCode` already carries the specific storage status. // Values must never carry a token or raw exception text: a token exposes the storage account, // container, and blob path. +// +// Reason and disposition are orthogonal and there is no fixed mapping between them. A reason says +// what the worker found; a disposition says whether trying again can change it. Most reasons occur +// with exactly one disposition, but TOKEN_NOT_PURGEABLE deliberately occurs with two. Do not assert +// a reason-to-disposition mapping anywhere. enum LargePayloadPurgeReason { LARGE_PAYLOAD_PURGE_REASON_UNSPECIFIED = 0; - // --- Reported with DELETED --- - - // The blob was deleted by this attempt, reclaiming its bytes. + // Reported with DELETED. The blob was deleted by this attempt, reclaiming its bytes. LARGE_PAYLOAD_PURGE_REASON_BLOB_DELETED = 1; - // The blob was already absent, so deletion was a no-op and no bytes were reclaimed by this - // attempt. Deletion is idempotent, so this is a success rather than a failure. Reported - // separately from BLOB_DELETED because a high rate of it indicates duplicate tombstones. + // Reported with DELETED. The blob was already absent, so deletion was a no-op and no bytes were + // reclaimed by this attempt. Deletion is idempotent, so this is a success rather than a failure. + // Reported separately from BLOB_DELETED because a high rate of it indicates duplicate tombstones. LARGE_PAYLOAD_PURGE_REASON_BLOB_ALREADY_ABSENT = 2; - // The blob was left in place because it did not carry the payload store's ownership marker, - // meaning the store did not write it. The token text merely matched the v2 grammar; the payload - // column is customer-writable, so matching text is not proof of ownership. This is an expected - // outcome rather than a defect. The tombstone is still resolved, because a blob the store does - // not own will never become deletable and retrying forever would leak the row. Reported - // separately from BLOB_DELETED so that "resolved without reclaiming bytes" stays countable. + // Reported with DELETED. The blob was left in place because it did not carry the payload store's + // ownership marker, meaning the store did not write it. The token text merely matched the v2 + // grammar; the payload column is customer-writable, so matching text is not proof of ownership. + // This is an expected outcome rather than a defect. The tombstone is still resolved, because a + // blob the store does not own will never become deletable and retrying forever would leak the + // row. Reported separately so that "resolved without reclaiming bytes" stays countable. LARGE_PAYLOAD_PURGE_REASON_BLOB_NOT_STORE_OWNED = 3; - // --- Reported with RETRY --- - - // The deletion failed against storage: network failure, timeout, outage, throttling, an - // unreachable account, or an authorization failure. All of these are reconfigurable or - // self-healing, and they are not subdivided here because `storageErrorCode` already carries the - // specific status for diagnostics. Subdividing would encode the same fact twice. + // Reported with RETRY. The deletion failed against storage: network failure, timeout, outage, + // throttling, an unreachable account, or an authorization failure. All of these are + // reconfigurable or self-healing, and they are not subdivided here because `storageErrorCode` + // already carries the specific status. Subdividing would encode the same fact twice. LARGE_PAYLOAD_PURGE_REASON_STORAGE_FAILURE = 10; - // The registered payload store does not implement deletion. Every payload fails the same way, - // so this is a deployment-wide condition rather than a per-row one, and it is kept recoverable - // until an operator registers a store that can delete. `storageErrorCode` is empty because - // storage was never contacted, which is why this cannot be folded into STORAGE_FAILURE. + // Reported with RETRY. The registered payload store does not implement deletion. Every payload + // fails the same way, so this is a deployment-wide condition rather than a per-row one, and it + // is kept recoverable until an operator registers a store that can delete. `storageErrorCode` is + // empty because storage was never contacted, which is why this cannot be folded into + // STORAGE_FAILURE. LARGE_PAYLOAD_PURGE_REASON_STORE_CANNOT_DELETE = 11; - // --- Reported with QUARANTINED --- - - // The token cannot be acted on and no retry can change that: its body does not parse, it names - // a version this worker does not support, or storage rejected it as permanently invalid. The - // producer and consumer of this token are both controlled by the SDK and backend, so reaching - // this state indicates a producer, corruption, or compatibility bug and the evidence is - // preserved for investigation rather than discarded. `storageErrorCode` distinguishes the - // storage-rejected case, where it is populated, from the parse cases, where it is empty. + // The worker could not act on the token. This reason is reported with TWO dispositions, and a + // consumer must not assume either one. + // + // Reported with QUARANTINED when the token can never become usable: its body does not parse, it + // is a legacy v1 token, or storage rejected a well-formed token as permanently invalid. The SDK + // and backend control both sides of this protocol, so reaching that state indicates a producer, + // corruption, or compatibility bug, and the evidence is preserved rather than discarded. + // + // Reported with RETRY in exactly one case: the token names a version this worker does not + // understand. Nothing is wrong with that token, since a newer worker can read it, so an SDK + // upgrade resolves it. The asymmetry is deliberate: quarantining it would be permanent and + // unrecoverable, whereas a retry that never succeeds only leaves the row idle and visible. + // + // `storageErrorCode` distinguishes the storage-rejected case, where it is populated, from the + // parse and unknown-version cases, where it is empty. LARGE_PAYLOAD_PURGE_REASON_TOKEN_NOT_PURGEABLE = 20; } From d0eb8a609ece4465fe96253c04e0f9beb9d76a47 Mon Sep 17 00:00:00 2001 From: wangbill Date: Tue, 11 Aug 2026 16:33:45 -0700 Subject: [PATCH 8/9] Remove write-only reason and storage_error_code from purge result 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 --- protos/orchestrator_service.proto | 79 ++++--------------------------- 1 file changed, 9 insertions(+), 70 deletions(-) diff --git a/protos/orchestrator_service.proto b/protos/orchestrator_service.proto index 863f41b..51f490a 100644 --- a/protos/orchestrator_service.proto +++ b/protos/orchestrator_service.proto @@ -872,74 +872,14 @@ enum LargePayloadPurgeDisposition { // The failure may self-heal, so the row stays pending and the backend sets the next attempt. LARGE_PAYLOAD_PURGE_DISPOSITION_RETRY = 2; - // A deterministic failure or protocol violation that retrying can never fix. The backend - // preserves the evidence, alerts, and stops automatic retries. + // A deterministic failure or protocol violation that retrying can never fix. The row leaves the + // polling set but is never deleted or expired: it keeps the token, which after the payload row is + // gone is the only durable record of the blob, so discarding it would orphan the blob silently. + // Resolving a quarantined row is a deliberate operator action. Why it failed is not recorded here + // and is not meant to be; that detail lives in the worker's telemetry at full fidelity. LARGE_PAYLOAD_PURGE_DISPOSITION_QUARANTINED = 3; } -// Why a row received its disposition. Diagnostic only: the backend acts on `disposition` alone and -// never branches on this value, so it exists to make a stuck or non-reclaiming ledger explainable -// without access to worker logs, which run in the customer's process. -// -// Deliberately coarse. Granularity matches the number of distinct operator responses, not the -// number of distinct causes, because `storageErrorCode` already carries the specific storage status. -// Values must never carry a token or raw exception text: a token exposes the storage account, -// container, and blob path. -// -// Reason and disposition are orthogonal and there is no fixed mapping between them. A reason says -// what the worker found; a disposition says whether trying again can change it. Most reasons occur -// with exactly one disposition, but TOKEN_NOT_PURGEABLE deliberately occurs with two. Do not assert -// a reason-to-disposition mapping anywhere. -enum LargePayloadPurgeReason { - LARGE_PAYLOAD_PURGE_REASON_UNSPECIFIED = 0; - - // Reported with DELETED. The blob was deleted by this attempt, reclaiming its bytes. - LARGE_PAYLOAD_PURGE_REASON_BLOB_DELETED = 1; - - // Reported with DELETED. The blob was already absent, so deletion was a no-op and no bytes were - // reclaimed by this attempt. Deletion is idempotent, so this is a success rather than a failure. - // Reported separately from BLOB_DELETED because a high rate of it indicates duplicate tombstones. - LARGE_PAYLOAD_PURGE_REASON_BLOB_ALREADY_ABSENT = 2; - - // Reported with DELETED. The blob was left in place because it did not carry the payload store's - // ownership marker, meaning the store did not write it. The token text merely matched the v2 - // grammar; the payload column is customer-writable, so matching text is not proof of ownership. - // This is an expected outcome rather than a defect. The tombstone is still resolved, because a - // blob the store does not own will never become deletable and retrying forever would leak the - // row. Reported separately so that "resolved without reclaiming bytes" stays countable. - LARGE_PAYLOAD_PURGE_REASON_BLOB_NOT_STORE_OWNED = 3; - - // Reported with RETRY. The deletion failed against storage: network failure, timeout, outage, - // throttling, an unreachable account, or an authorization failure. All of these are - // reconfigurable or self-healing, and they are not subdivided here because `storageErrorCode` - // already carries the specific status. Subdividing would encode the same fact twice. - LARGE_PAYLOAD_PURGE_REASON_STORAGE_FAILURE = 10; - - // Reported with RETRY. The registered payload store does not implement deletion. Every payload - // fails the same way, so this is a deployment-wide condition rather than a per-row one, and it - // is kept recoverable until an operator registers a store that can delete. `storageErrorCode` is - // empty because storage was never contacted, which is why this cannot be folded into - // STORAGE_FAILURE. - LARGE_PAYLOAD_PURGE_REASON_STORE_CANNOT_DELETE = 11; - - // The worker could not act on the token. This reason is reported with TWO dispositions, and a - // consumer must not assume either one. - // - // Reported with QUARANTINED when the token can never become usable: its body does not parse, it - // is a legacy v1 token, or storage rejected a well-formed token as permanently invalid. The SDK - // and backend control both sides of this protocol, so reaching that state indicates a producer, - // corruption, or compatibility bug, and the evidence is preserved rather than discarded. - // - // Reported with RETRY in exactly one case: the token names a version this worker does not - // understand. Nothing is wrong with that token, since a newer worker can read it, so an SDK - // upgrade resolves it. The asymmetry is deliberate: quarantining it would be permanent and - // unrecoverable, whereas a retry that never succeeds only leaves the row idle and visible. - // - // `storageErrorCode` distinguishes the storage-rejected case, where it is populated, from the - // parse and unknown-version cases, where it is empty. - LARGE_PAYLOAD_PURGE_REASON_TOKEN_NOT_PURGEABLE = 20; -} - // client -> server: the outcome of exactly one tombstoned row. message LargePayloadPurgeResult { // Row identity, echoed from the corresponding LargePayloadTombstone. @@ -950,12 +890,11 @@ message LargePayloadPurgeResult { // Echoed unmodified from the fetched tombstone; used as a compare-and-swap guard. int64 revision = 4; + // The only field the backend acts on. Deliberately the only outcome field on this message: + // anything finer would be write-only. Failure detail stays in the worker's own telemetry, which + // holds the full exception rather than a lossy classification, and a row is correlated to it by + // (partitionId, instanceKey, payloadId) plus the ledger's LastAttemptAt. LargePayloadPurgeDisposition disposition = 5; - LargePayloadPurgeReason reason = 6; - - // Optional bounded, sanitized storage status or error code for diagnostics - // (for example "BlobNotFound" or "409"). Must never contain a token or raw exception text. - string storageErrorCode = 7; } // client -> server: request up to `limit` due tombstones for the caller's task hub. From d94c1d786fa1452c8288248c55857c40b5de8d8f Mon Sep 17 00:00:00 2001 From: wangbill Date: Wed, 12 Aug 2026 21:49:03 -0700 Subject: [PATCH 9/9] Drop unresolvable LastAttemptAt from disposition comment 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 --- protos/orchestrator_service.proto | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protos/orchestrator_service.proto b/protos/orchestrator_service.proto index 51f490a..080b697 100644 --- a/protos/orchestrator_service.proto +++ b/protos/orchestrator_service.proto @@ -893,7 +893,7 @@ message LargePayloadPurgeResult { // The only field the backend acts on. Deliberately the only outcome field on this message: // anything finer would be write-only. Failure detail stays in the worker's own telemetry, which // holds the full exception rather than a lossy classification, and a row is correlated to it by - // (partitionId, instanceKey, payloadId) plus the ledger's LastAttemptAt. + // (partitionId, instanceKey, payloadId). LargePayloadPurgeDisposition disposition = 5; }