Skip to content

[Brook K4] Added the memory Store and the shared Store suite. - #215

Closed
moetemp wants to merge 28 commits into
moe/brook/review/streams-k3-store-traitfrom
moe/brook/review/streams-k4-memory-store
Closed

moetemp wants to merge 28 commits into
moe/brook/review/streams-k3-store-traitfrom
moe/brook/review/streams-k4-memory-store

Conversation

@moetemp

@moetemp moetemp commented Oct 27, 2026 •

Copy link
Copy Markdown
Owner

What changed?

This PR adds the memory Store and the shared Store suite.

  • The memory Store keeps the Redis layout and runs the steps of the Lua scripts in the same order.
  • The suite is in testing, behind the test-support feature, so the Redis crate runs the same cases. The name of each case says the guarantee it checks, and a failed run lists the cases that failed.
  • DEFAULT_RETENTION and DEFAULT_SAFETY_TTL give every Store the same defaults.
  • A create of an expired, other-owner or closed Stream fails with stream_expired, stream_not_owner or stream_closed. A Workflow's create doesn't take a client's Stream. A create of an open Stream fails with stream_already_exists, unless it uses the Stream or retries the create that made it, which the Store knows by the request id that it keeps.
  • A Stream with no owner takes a close from a Workflow or a client. A close writes no entry, and it wakes the waiting reads.
  • Stored data that isn't valid fails with stream_storage_corrupt, which a retry can't fix.
  • Each append removes the fence and dedupe fields of up to 64 Producers whose last append is older than the Retention, from the producers set. So the meta keeps the Producers of about one Retention.
  • An open Workflow Stream's meta keeps the safety TTL, but its log expires one Retention after the last write, since a read hides every Record by then. The meta's newest keeps the next offsets above the old ones. A write after the log expired raises trimmed to newest, so a Reader behind it gets stream_expired.
  • No retry run or orphan rules yet, so no RUN_FENCE. K7 adds them.

Depends on #214 (K3). Review lane is the Redis and Lua reviewers, as the reference for R1 and R2 (temporal-community/sdk-streams#1 and #2).
Part of the Brook review series.

Why?

The same rules exist in Lua, in this Store, in gen_vectors.py and in the Jetty server. Plain Rust is easier to check first, and the suite fails when two Stores differ.

How did you test it?

cargo fmt, cargo clippy -- -D warnings (with and without features), cargo test -p temporalio-streams --all-features and cargo doc pass at this commit. The suite runs on the memory Store here, and on Redis in R1.

@moetemp
moetemp force-pushed the moe/brook/review/streams-k4-memory-store branch from c2df597 to 7fe70de Compare October 27, 2026 07:35
@moetemp
moetemp force-pushed the moe/brook/review/streams-k3-store-trait branch from 4a9d1b5 to 5e50e8d Compare October 27, 2026 07:35
@moetemp
moetemp force-pushed the moe/brook/review/streams-k4-memory-store branch from 7fe70de to 12ed854 Compare October 27, 2026 08:17
@moetemp
moetemp force-pushed the moe/brook/review/streams-k3-store-trait branch from 5e50e8d to 92528cb Compare October 27, 2026 08:17
@moetemp moetemp added the skip-changelog Changelog entry rides another PR label Oct 27, 2026
@moetemp
moetemp force-pushed the moe/brook/review/streams-k3-store-trait branch from 92528cb to a6020ca Compare October 27, 2026 16:55
@moetemp
moetemp force-pushed the moe/brook/review/streams-k4-memory-store branch 2 times, most recently from 2a15176 to f2287d8 Compare October 27, 2026 23:46
@moetemp
moetemp force-pushed the moe/brook/review/streams-k3-store-trait branch 2 times, most recently from 614cf9c to 3fcf1b2 Compare October 28, 2026 08:55
@moetemp
moetemp force-pushed the moe/brook/review/streams-k4-memory-store branch 3 times, most recently from cb14cea to 0e7433f Compare October 29, 2026 08:37
The memory Store keeps the Redis key layout and runs the steps of the Lua
scripts in the same order. The `test-support` feature exports the suite, so
the Redis Store and any other Store run the same cases. The retry run and
orphan rules come later, so the memory Store lacks the run fence.
@moetemp
moetemp force-pushed the moe/brook/review/streams-k4-memory-store branch from 0e7433f to 167d0f8 Compare October 29, 2026 09:17
The memory Store drops its default_retention, since K3 removed it from the trait.
Only the shared suites and the crate's tests call them. The README says that each Store reads fields.json through the Store suite.
The memory Store takes the number under the lock of the dedupe check. The Store suite checks the
numbering, the dedupe of a taken number, and batches that arrive at once.
Redis keeps hash fields and values as bytes, and the binary keys and digest come next. The raw
meta calls of the suites take and give bytes too.
`p|<kind>|<producer id>` replaces `e|`, `d|` and `h|` with a fixed 129-byte value, which saves
meta bytes. A Store refuses another length or flag bit, so a later format can tell them apart. The
vectors and a raw-field case compare the bytes of every Store.
A create keeps its request id as rid, so a retry of it succeeds. A Stream with no owner takes a close from a Workflow or a client. The Store suite checks both, and the Producer's kind comes from its epoch case.
Stored data that isn't valid is corrupt, and ids that don't fit this millisecond are unavailable, since a later write fits. The Store suite checks that a close ends a wait.
The proto type is now StreamRef, so the code and the docs say ref where they mean it.
A newer format can change the Producer field, so the memory Store now gives stream_format_too_new and not stream_storage_corrupt, as the Redis Store does. The shared suite checks it on every Store.
The review asked to stop representing invalid states. The Producer kind is now a oneof with run_id only in the run-based variants, so a client can't carry a run. A write's task facts and withdraw_run move into RunWithdrawal, which only a run writer sets.
@moetemp

moetemp commented Nov 10, 2026

Copy link
Copy Markdown
Owner Author

Closed in favor of #228, the combined branch, which has this change.

@moetemp moetemp closed this Nov 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Changelog entry rides another PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant