Repository navigation
Conversation
1 of 3 tasks
moetemp
force-pushed
the
moe/brook/review/k4-memory-store
branch
from
October 25, 2026 22:10
a6127cf to
5c5c7d9
Compare
moetemp
force-pushed
the
moe/brook/review/k5-redis-store
branch
2 times, most recently
from
October 25, 2026 22:17
4be1ac8 to
d351adb
Compare
moetemp
force-pushed
the
moe/brook/review/k4-memory-store
branch
from
October 25, 2026 22:17
5c5c7d9 to
3a142d0
Compare
moetemp
force-pushed
the
moe/brook/review/k5-redis-store
branch
from
October 26, 2026 01:08
d351adb to
efebb98
Compare
Each create, append and close runs as one script, so the checks are atomic with the write. The shared Store suite runs on Redis standalone and on a three-node cluster, and CI gets a Redis job.
moetemp
force-pushed
the
moe/brook/review/k4-memory-store
branch
from
October 26, 2026 08:00
14b96c0 to
a075c7f
Compare
moetemp
force-pushed
the
moe/brook/review/k5-redis-store
branch
from
October 26, 2026 08:00
efebb98 to
51d00aa
Compare
Owner
Author
|
Moved to |
moetemp
added a commit
to temporal-community/sdk-streams
that referenced
this pull request
Nov 9, 2026
### What changed? This PR adds `temporalio-streams-redis`, the Redis Store of the Stream runtime, with one Lua script each for create, append, close, read and reverse read. - Each script checks every refusal before its first write, since Redis doesn't roll a script back. - A create of an existing Stream fails with `stream_already_exists`, unless it uses the Stream or retries with the request id of the create that made it. - The read scripts run with `EVALSHA_RO`, so a Reader needs only read access to the keys. Each one reads its page and the Producer of a batch that starts before the page in one step. - A read of a closed Stream ends with a `StreamClose` item, which it builds from the Producer that closed it. The Store keeps no owner check and no withdrawal facts, since supersession moves to an opt-in client helper. - `wait_replicas` adds a `WAIT` after each write, and `connect` checks that the user can run it. - Each blocked `XREAD` or `WAIT` gets a dedicated connection from a bounded pool per node. A dropped call closes its connection, since its command still runs on the server. A write on an idle connection that the server closed fails with `stream_storage_unavailable` and doesn't run again, since the server may have run it. - On a cluster, a blocking read gets the slot's primary from the live topology when the seeds are down. - The options are typed, with a `Topology` of one server or a cluster's seeds, and a bad config fails when the bridge builds the runtime. - `connect` refuses Redis before 7.0, and a cluster node that the options give as one server. It warns when the eviction policy isn't `noeviction`. - A `MOVED` from one server is a config error. On a cluster, the client follows redirects, so one that it gives up on is `stream_storage_unavailable`. - Each Producer has one 88-byte `p|` field with binary epoch keys and a 16-byte digest. The scripts compare keys byte by byte, since Lua's `<` collates with the server's locale. - `docs/redis.md` gives the keys, the TTLs, the ACL commands of each role and the known limits. It depends on `moetemp/sdk-rust#227`, which drops the owner and the withdrawal and adds the Stream close item. CI patches in the sdk-rust fork until `temporalio-streams` is released. It replaces `moetemp/sdk-streams#47`. ### Why? Redis is the production Store for now. Its Lua and cluster code need Redis reviewers, so this repo has only the Redis parts. The shared runtime is in sdk-rust. One question is open. Is a 1 s wait right for a blocking read that finds no free connection? ### How did you test it? The shared Store and service suites run on a standalone Redis with a replica, and on a cluster of three primaries with one replica each, with and without `WAIT`. CI runs them on Redis 8 and on Redis 7.0. `tests/acl.rs` runs each role with exactly the documented commands, also on a cluster while a slot migrates. It reads the lists from `docs/redis.md`, and a unit test checks that each role's scripts call exactly those commands. `tests/parity.rs` runs seeded random calls on this Store and on the memory Store, and compares the answers. It also checks that a refused write leaves the Stream's keys as they were. The pool and timing cases are unit tests on tokio's paused clock, with no sleeps. CI also runs a FIPS build. --------- Co-authored-by: Maple Xu <45406854+xumaple@users.noreply.github.com> Co-authored-by: David Hyde <1920464+DABH@users.noreply.github.com>
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 Redis Store, with one Lua script each for create, append, close and read.
XADD *can split a batch's offsets on Redis 7.0.connectrefuses Redis before 7.0 and warns on an eviction policy other thannoeviction.redisjob (standalone and cluster) and aredis-7-0job.Depends on #46 (K4). Review lane is the Redis and Lua reviewers.
Part of the Brook review series.
Why?
Redis is the production Store, and its Lua and cluster client need the Redis reviewers.
Open questions.
redis.callfail after a write, for a wrong type or OOM?EVALSHAis a write to its keys, so a Reader needs write access. UseEVALSHA_RO?volatile-*policy can evict a fence or a dedupe entry. Is a warning at connect enough?How did you test it?
cargo fmt,clippy(both feature sets),cargo teston memory, Redis 8 and 7.0. Fork CI passes.Link to a test plan if any -