Skip to content

[Brook] Dropped the cache-off eviction reason, since no Stream closes at a run end. - #209

Closed
moetemp wants to merge 4 commits into
moe/brook/v4/c7-no-worker-extensionfrom
moe/brook/v5/c8-no-eviction-reason
Closed

moetemp wants to merge 4 commits into
moe/brook/v4/c7-no-worker-extensionfrom
moe/brook/v5/c8-no-eviction-reason

Conversation

@moetemp

@moetemp moetemp commented Oct 24, 2026 •

Copy link
Copy Markdown
Owner

This PR drops the cache-off eviction reason from Core and adds tests for the Local Activity's workflow_task.

Brook, the Redis Streams stack, closed a run's Streams at the eviction after an accepted completion. It needed the reason WORKFLOW_EXECUTION_ENDING for that eviction with the Workflow cache off. After the SDK review, a run end closes nothing on Redis, so Core can use the upstream reason again.

What changed?

  • 320e725f reverts b565c433. With the Workflow cache off, a completed run's eviction has the reason CACHE_FULL again, as upstream. The revert also removes the test of that reason.
  • Field 20, ActivityTask.Start.workflow_task, stays. The CHANGELOG.md entry now names all of its fields. These are the started event id, attempt and start time, previous_started_event_id, and failed_tasks (the last 100 failed or timed out Workflow Tasks).
  • core_tests/local_activity_workflow_task.rs has new mock-poller tests. A reschedule after a backoff timer gets the later task. failed_tasks has the failed tasks of every History page. The Local Activities of one activation get the same task.
  • The comments use literal wording. The proto comments of workflow_task and WorkflowTaskAttempt say that the values come from the poll response and the History. They also say that Core sets only the last 100 failed tasks.

Why?

An independent review of Brook and Jetty found that the CHANGELOG.md entry left out two fields of workflow_task. The Streams crate uses these fields to withdraw the Records of Local Activities whose Workflow Task never completed. The code was correct, but backoffs and paged History had no test.

How did you test it?

cargo test -p temporalio-sdk-core --lib passes, with the new tests. cargo fmt --all --check and clippy on temporalio-sdk-core and temporalio-protos pass. On the fork CI, every job passes except the ubuntu-arm jobs and Heavy Tests, which need runners that the fork doesn't have.

The crate s23 and the superproject branch temporalio/internal-ai-prototypes 2026/10/ai198-streams pin f7826366, the tested commit. The head commit after it changes only comments and docs, so the pins don't change.

  • Unit Tests
  • Staging
  • End to End Tests

…n end.

Brook used it to close a run's Streams at the eviction after an accepted completion. A run end now closes nothing on Redis, so Core keeps upstream behavior.
Covers a reschedule after a backoff timer, failed tasks on more than one history page, and two Local Activities of one activation.
@moetemp

moetemp commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Closed in favor of the review series. Its net change is C1, #211, cut fresh onto upstream main (plan: Brook: code review and upstream plan). The branch stays, since the superproject pins it and Jetty's #210 builds on it.

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.

1 participant