Skip to content

feat: withdraw orphaned Local Activity Records and a retry run's Records - #2

Closed
moetemp wants to merge 44 commits into
moe/brook/review/r1-redis-storefrom
moe/brook/review/r2-withdrawals
Closed

moetemp wants to merge 44 commits into
moe/brook/review/r1-redis-storefrom
moe/brook/review/r2-withdrawals

Conversation

@moetemp

@moetemp moetemp commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

What changed?

This PR adds the withdrawal rules of temporalio-streams to the Lua scripts.

  • A Local Activity write with its run's task facts withdraws the Records of the run's tasks that never completed, and of the lower attempts of its own task and of the completed task. It uses the attempt and started time that Core sends (temporalio/sdk-rust#1703).
  • Each withdrawn task has its own ow|<run>|<task key> field, so the fence reads one field.
  • tf|<run> keeps the task facts of the run's highest task. A later write of a task that these facts show never completed is fenced, since a task can fail before its call writes.
  • A Local Activity write with no task facts, as from an older Core, still goes in oa|<run>, so a later write's facts can withdraw it.
  • A retry run's first write withdraws and fences the previous run, so the Redis Store declares RUN_FENCE. An empty batch that withdraws a run keeps its Producer's held epoch.
  • The append script refuses malformed task facts before its first write.

It depends on #1, and on moetemp/sdk-rust#218, which has the rules and their cases. It replaces the Lua part of moetemp/sdk-streams#50.

Why?

The orphan rule is the hardest Lua to review, so it's in its own PR.

Some Records stay, in both Stores. A failed task that no later write covers keeps its Records, and so does a failed task older than the last 100 that Core sends. docs/redis.md lists these and the other cases.

How did you test it?

The shared Store and service suites run on Redis with the rules' cases, standalone with a replica and on a cluster of three primaries with one replica each. tests/acl.rs also runs an orphan append and a retry run's create with only the documented commands. A unit case checks that malformed task facts write nothing. The parity check of #1 also runs retry-run withdrawals and Local Activity task facts on both Stores. CI also runs Redis 7.0 and a FIPS build.

@moetemp
moetemp force-pushed the moe/brook/review/r2-withdrawals branch from 665fb1d to 9b5fcbc Compare October 27, 2026 07:43
@moetemp
moetemp force-pushed the moe/brook/review/r1-redis-store branch from f05f8bd to e674889 Compare October 27, 2026 07:43
@moetemp
moetemp force-pushed the moe/brook/review/r2-withdrawals branch from 9b5fcbc to b63c189 Compare October 27, 2026 08:17
@moetemp
moetemp force-pushed the moe/brook/review/r1-redis-store branch 2 times, most recently from d154bd1 to 5434259 Compare October 27, 2026 16:55
@moetemp
moetemp force-pushed the moe/brook/review/r2-withdrawals branch from b63c189 to 602c740 Compare October 27, 2026 16:55
@moetemp
moetemp force-pushed the moe/brook/review/r1-redis-store branch from 5434259 to 9df6e26 Compare October 27, 2026 23:47
@moetemp
moetemp force-pushed the moe/brook/review/r2-withdrawals branch from 602c740 to 0983d95 Compare October 27, 2026 23:47
@moetemp
moetemp force-pushed the moe/brook/review/r1-redis-store branch from 9df6e26 to 4ac6a50 Compare October 28, 2026 08:56
@moetemp
moetemp force-pushed the moe/brook/review/r2-withdrawals branch 2 times, most recently from 1f162d3 to baa1d3c Compare October 29, 2026 08:40
@moetemp
moetemp force-pushed the moe/brook/review/r1-redis-store branch from 4ac6a50 to 6f16043 Compare October 29, 2026 08:40
@moetemp
moetemp force-pushed the moe/brook/review/r2-withdrawals branch from baa1d3c to f5c9cf3 Compare October 29, 2026 09:18
@moetemp
moetemp force-pushed the moe/brook/review/r1-redis-store branch from 6f16043 to a08b2a8 Compare October 29, 2026 09:18
@moetemp
moetemp force-pushed the moe/brook/review/r2-withdrawals branch from f5c9cf3 to 476d7b5 Compare October 29, 2026 10:04
@moetemp
moetemp force-pushed the moe/brook/review/r1-redis-store branch from a08b2a8 to 4281b5a Compare October 29, 2026 10:04
@moetemp
moetemp force-pushed the moe/brook/review/r2-withdrawals branch from 476d7b5 to 8f1f7bd Compare October 29, 2026 15:17
@moetemp
moetemp force-pushed the moe/brook/review/r1-redis-store branch from 4281b5a to 6adb884 Compare October 29, 2026 15:17
The Lua scripts apply the withdrawal rules of `temporalio-streams`. A Local
Activity write with its run's task facts withdraws the Records of the run's
failed tasks, and a retry run's first write withdraws and fences the previous
run. The Redis Store declares `RUN_FENCE`.
@moetemp
moetemp force-pushed the moe/brook/review/r2-withdrawals branch from 8f1f7bd to 3b554a4 Compare October 29, 2026 16:13
@moetemp
moetemp force-pushed the moe/brook/review/r1-redis-store branch from 6adb884 to 63486ac Compare October 29, 2026 16:13
R2's withdrawal code now returns R1's Redis error type, in the moved store.rs.
# Conflicts:
#	crates/streams-redis/lua/append.lua
@moetemp moetemp changed the title [Brook R2] Withdrew orphaned Local Activity Records and a retry run's Records. feat: withdraw orphaned Local Activity Records and a retry run's Records Oct 30, 2026
moetemp and others added 21 commits November 4, 2026 02:16
# Conflicts:
#	crates/streams-redis/docs/redis.md
#	crates/streams-redis/lua/append.lua
#	crates/streams-redis/src/store.rs
Only a newer epoch's item withdraws the held epoch's Records. So the Producer's next epoch still withdraws them, as in the memory Store.
A write from an older Core gives no task facts. Its task now goes in `oa` too, so that a later write's facts can withdraw its Records.
A task can fail before its call writes, so no orphan item withdrew it. `tf|<run>` keeps the facts of the run's highest task, and an append checks its task against them.
R1 moves the Producer state to one binary p| field and decodes each script answer into a typed reply. To match the crate, R2's oa|, tf| and ow| fields now hold 24-byte task keys, and started() reads their first 8 bytes.
TaskArgs is untracked, a task with no facts, or a task with facts, and it writes the four script arguments. The script now takes an empty completed key for no facts and 8-byte failed ids, so it parses no text list.
Flag implements FromRedisValue, so the create and append replies decode their flag in the tuple, and a create flag of 1 has a unit case. The malformed facts test checks the mapped reason, not the code string.
Maple's review shaped the types, the argument encoding and the tests of this PR.

Co-authored-by: Maple Xu <45406854+xumaple@users.noreply.github.com>
R1 follows the crate's new wire. R2's create keeps its withdrawal on the path that uses an existing Stream, after the conflict check, and its Local Activity task comes from the epoch's case.
R1 splits the storage reasons and closes with no Record. R2's close keeps its retry run fence, and a partial task key or short task facts are storage corrupt.
# Conflicts:
#	crates/streams-redis/lua/append.lua
#	crates/streams-redis/src/store.rs
A retry run's creates and appends withdraw the previous run, and Local Activities of three Workflow Tasks give random task facts. These rules are the longest part of the append script.
# Conflicts:
#	crates/streams-redis/tests/parity.rs
A batch gives its task facts and its run to withdraw in StreamWriter, and the orphan facts' StreamSuperseded has a writer. The create keeps its own withdraw_run.
@moetemp
moetemp marked this pull request as draft November 9, 2026 08:02
@moetemp

moetemp commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

Closing this. Supersession moves to an opt-in client helper, so the Redis Store keeps no withdrawal or task facts. #1 is now the base Store in that final form, with no withdrawal machinery, and #3 rebases onto it. The withdrawal rules are no longer part of the Store.

@moetemp moetemp closed this Nov 9, 2026
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