Repository navigation
Preserve Workflow Streams after workflow closure #1994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Preserve retained Workflow Stream items after workflow completion, failure, cancellation, termination, or timeout by draining them through read-only queries before subscriptions finish. | ||
| Preserve the original Workflow Streams poll error when the follow-up execution description fails with an RPC error. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,7 +51,8 @@ | |
| ) | ||
|
|
||
| _PUBLISH_SIGNAL = "__temporal_workflow_stream_publish" | ||
| _POLL_UPDATE = "__temporal_workflow_stream_poll" | ||
| # Updates and queries share a name because they implement the same poll protocol. | ||
| _POLL = "__temporal_workflow_stream_poll" | ||
| _OFFSET_QUERY = "__temporal_workflow_stream_offset" | ||
|
|
||
| _MAX_POLL_RESPONSE_BYTES = 1_000_000 | ||
|
|
@@ -84,6 +85,7 @@ class WorkflowStream: | |
|
|
||
| - ``__temporal_workflow_stream_publish`` signal — external publish with dedup | ||
| - ``__temporal_workflow_stream_poll`` update — long-poll subscription | ||
| - ``__temporal_workflow_stream_poll`` query — immediate retained-item read | ||
| - ``__temporal_workflow_stream_offset`` query — current log length | ||
|
|
||
| Note: | ||
|
|
@@ -155,9 +157,8 @@ def __init__(self, prior_state: WorkflowStreamState | None = None) -> None: | |
| self._topic_types: dict[str, type[Any]] = {} | ||
|
|
||
| workflow.set_signal_handler(_PUBLISH_SIGNAL, self._on_publish) | ||
| workflow.set_update_handler( | ||
| _POLL_UPDATE, self._on_poll, validator=self._validate_poll | ||
| ) | ||
| workflow.set_update_handler(_POLL, self._on_poll, validator=self._validate_poll) | ||
| workflow.set_query_handler(_POLL, self._collect_batch) | ||
| workflow.set_query_handler(_OFFSET_QUERY, self._on_offset) | ||
|
|
||
| def _publish_to_topic(self, topic: str, value: Any) -> None: | ||
|
|
@@ -337,9 +338,9 @@ async def continue_as_new( | |
| def truncate(self, up_to_offset: int) -> None: | ||
| """Discard log entries before ``up_to_offset``. | ||
|
|
||
| After truncation, polls requesting an offset before the new | ||
| base will receive an ApplicationError. All global offsets | ||
| remain monotonic. | ||
| After truncation, poll updates requesting a nonzero offset before | ||
| the new base receive an ApplicationError. Poll queries read from | ||
| the retained base instead. All global offsets remain monotonic. | ||
|
|
||
| Raises ApplicationError (not ValueError) when ``up_to_offset`` | ||
| is past the end of the log so that callers invoking this from | ||
|
|
@@ -407,21 +408,18 @@ async def _on_poll(self, payload: PollInput) -> PollResult: | |
| or self._detaching | ||
| ), | ||
| ) | ||
| log_offset = payload.from_offset - self._base_offset | ||
| if log_offset < 0: | ||
| if payload.from_offset == 0: | ||
| # "From the beginning" — start at whatever is available. | ||
| log_offset = 0 | ||
| else: | ||
| # Subscriber had a specific position that's been | ||
| # truncated. ApplicationError fails this update (client | ||
| # gets the error) without crashing the workflow task — | ||
| # avoids a poison pill during replay. | ||
| raise ApplicationError( | ||
| f"Requested offset {payload.from_offset} has been truncated. " | ||
| f"Current base offset is {self._base_offset}.", | ||
| type=TRUNCATED_OFFSET_ERROR_TYPE, | ||
| ) | ||
| if payload.from_offset != 0 and payload.from_offset < self._base_offset: | ||
| # ApplicationError fails this update without failing the workflow task. | ||
| raise ApplicationError( | ||
| f"Requested offset {payload.from_offset} has been truncated. " | ||
| f"Current base offset is {self._base_offset}.", | ||
| type=TRUNCATED_OFFSET_ERROR_TYPE, | ||
| ) | ||
| return self._collect_batch(payload) | ||
|
|
||
| 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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| all_new = self._log[log_offset:] | ||
| if payload.topics: | ||
| topic_set = set(payload.topics) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.