Repository navigation
Preserve Workflow Streams after workflow closure - #1994
Conversation
moetemp
left a comment
moetemp
left a comment
There was a problem hiding this comment.
Took a pass through the closed-stream drain. Looks clean. Left a few questions, mostly around the Update-to-Query hand-off and the cost of draining a large tail. Nothing blocking from my side.
|
|
||
| def _collect_batch(self, payload: PollInput) -> PollResult: | ||
| """Read retained items immediately without changing stream state.""" | ||
| log_offset = max(0, payload.from_offset - self._base_offset) |
There was a problem hiding this comment.
Each page is one query. On a closed run the worker replays history to answer every query. So a large retained tail turns into one full replay per page. Is matching the 1MB Update cap worth it for closed-run reads, or could a drain read use a bigger single-shot budget to cut the replays? We can defer if the retained tail stays small in practice.
There was a problem hiding this comment.
I recommend keeping the current ~1 MB cap for this PR. I ran 600 local dev-server drains using the existing native build: 10 workloads × 0.5/1/1.5 MB estimated budgets × 20 repetitions, varying tail/history size, caching, filtering, and AES-GCM encoding. Every measured Query page recreated the workflow. For a Signal-backed ~5 MB tail, 1.5 MB pages reduced median drain time from 1.42 s to 1.06 s (six replays to four), but increased first-item latency from 247 ms to 279 ms.
The larger budget also exposed missing JSON/envelope overhead in the estimator: with 16-byte items, the 1.5 MB estimate produced a 2.38 MB response and failed with WorkflowQueryFailedError: Blob data size exceeds limit; the current budget succeeded. I'd defer a larger Query-specific budget until response-size accounting and codec headroom are addressed.
What was changed
Active subscriptions continue polling through Updates. After Describe confirms completion, failure, cancellation, termination, or timeout, subscriptions query the confirmed run for all remaining retained pages and finish after final delivery. The synchronous
__temporal_workflow_stream_pollQuery sharesPollInput/PollResultand read-only batch collection with the Update. It returns immediately and clamps stale offsets to the retained base.Accepted-run tracking is local to each subscription attempt. Query errors, failed descriptions, and unconfirmed closure remain errors; terminal reads stay pinned across workflow ID reuse. Existing truncation, draining, cancellation, conversion, and continue-as-new behavior is preserved.
Why?
Subscribing after closure, or closing a run during an accepted poll, could previously end the subscription before its unread retained tail was delivered. Query draining makes retained stream contents available after closure without adding history events.
Checklist
poe lintpassed.Updated the stream guide/API comments and release notes. Workers must be upgraded before upgraded clients use closed-stream reads; retained history and an available compatible worker are required.
Subscription signatures and existing wire types are unchanged. No Server, Core, or protobuf change is required. Upgraded clients surface missing poll Query errors from older workers.
Coordinated SDK PRs