Repository navigation
Conversation
moetemp
force-pushed
the
moe/brook/review/streams-k4-memory-store
branch
from
October 27, 2026 07:35
c2df597 to
7fe70de
Compare
moetemp
force-pushed
the
moe/brook/review/streams-k3-store-trait
branch
from
October 27, 2026 07:35
4a9d1b5 to
5e50e8d
Compare
moetemp
force-pushed
the
moe/brook/review/streams-k4-memory-store
branch
from
October 27, 2026 08:17
7fe70de to
12ed854
Compare
moetemp
force-pushed
the
moe/brook/review/streams-k3-store-trait
branch
from
October 27, 2026 08:17
5e50e8d to
92528cb
Compare
moetemp
force-pushed
the
moe/brook/review/streams-k3-store-trait
branch
from
October 27, 2026 16:55
92528cb to
a6020ca
Compare
moetemp
force-pushed
the
moe/brook/review/streams-k4-memory-store
branch
2 times, most recently
from
October 27, 2026 23:46
2a15176 to
f2287d8
Compare
moetemp
force-pushed
the
moe/brook/review/streams-k3-store-trait
branch
2 times, most recently
from
October 28, 2026 08:55
614cf9c to
3fcf1b2
Compare
moetemp
force-pushed
the
moe/brook/review/streams-k4-memory-store
branch
3 times, most recently
from
October 29, 2026 08:37
cb14cea to
0e7433f
Compare
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
force-pushed
the
moe/brook/review/streams-k4-memory-store
branch
from
October 29, 2026 09:17
0e7433f to
167d0f8
Compare
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.
Owner
Author
|
Closed in favor of #228, the combined branch, which has this change. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed?
This PR adds the memory Store and the shared Store suite.
testing, behind thetest-supportfeature, 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_RETENTIONandDEFAULT_SAFETY_TTLgive every Store the same defaults.stream_expired,stream_not_ownerorstream_closed. A Workflow's create doesn't take a client's Stream. A create of an open Stream fails withstream_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.stream_storage_corrupt, which a retry can't fix.producersset. So the meta keeps the Producers of about one Retention.newestkeeps the next offsets above the old ones. A write after the log expired raisestrimmedtonewest, so a Reader behind it getsstream_expired.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#1and#2).Part of the Brook review series.
Why?
The same rules exist in Lua, in this Store, in
gen_vectors.pyand 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-featuresandcargo docpass at this commit. The suite runs on the memory Store here, and on Redis in R1.