feat: add the Redis Store with one Lua script per operation by moetemp · Pull Request #1 · temporal-community/sdk-streams

moetemp · 2026-10-27T07:26:12Z

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.

Each create, append and close runs as one script, so the checks are atomic with the write. The Redis runtime builds a `temporalio-streams` runtime on it. The shared Store and service suites run on Redis standalone and on a three-node cluster, and CI gets Redis jobs.

The Store trait still returns Stream errors, so the runtime and lang get the same reasons on every backend.

moetemp

@xumaple Thanks for the comments.

The Store trait no longer has default_retention, and nothing called the inherent one. No bridge selects a runtime by REDIS_RUNTIME_NAME.

The From conversion skipped the drop of the other dead idle connections. Stale now carries its error, so the write site returns it with no second try.

A redirect that the cluster client gave up on is a passing state, so it is StorageUnavailable. A standalone connection that gets MOVED is at a cluster node, so it stays a config error.

Redis gives NULL for a node whose endpoint it doesn't know. The Store can't tell which node answered, so such a node isn't reachable, and the seeds stay.

A cluster with unknown endpoints redirects with an empty host, and a blocking read tried to connect to it. A ? host now fails the read, since no connect can reach it.

moetemp

Thanks @xumaple and @DABH. Replied inline.

David's review shaped the format check, the reverse read script and the cluster error classes of this PR. Co-authored-by: David Hyde <1920464+DABH@users.noreply.github.com>

The ACL tests now read the role lists from docs/redis.md, and a unit test checks that each role's scripts call exactly those commands. So a script that calls a new command fails until the docs list it.

A cluster refuses a script with CROSSSLOT when a key falls outside the hash tag. The ids take the characters that a tag or a key uses.

…rror A user with no INFO gets past connect, and then each key of another node gets MOVED. The case checks a write, a read and a blocking read.

Each seed runs the same random calls on both Stores and compares the answers. A refused write must also leave the Stream's Redis keys as they were, since Redis doesn't roll a script back.

The wire moved a write's Producer and withdrawal fields into StreamWriter. The Lua scripts keep the encoded facts as they are, so only the test changes.

The Redis Store lives outside sdk-core, so its config stays out of coresdk.streams. Its shared fields keep the numbers of MemoryStreamConfig.

DABH

Might be good to get @xumaple 's +1 as well, but my comments were satisfactorily addressed - thanks

The stream protos moved each epoch into a per-kind oneof on the Producer, so the Store reads the epoch key from that oneof and its tests build Producers in the new shape.

Supersession moves to an opt-in client helper, so the Store keeps no owner check and no withdrawal or task facts. A read ends a closed Stream with a StreamClose item that it builds from the close. This pins the Stream crate that has these.

xumaple · 2026-11-09T16:33:53Z

+
+/// The URL with `***` in place of its user info and its query. A socket URL gives its password in
+/// the query.
+fn redacted(url: &str) -> String {


can we take a Url here and allow the library to do the parsing instead of hand writing string parsing?

This one redacts the URL for the debug output, so it shouldn't show the password. A Redis socket URL isn't a standard URL, and url::Url can reject it.

I don't agree. You shouldn't be building the Url here just for parsing purposes, it should be built at the beginning and every url passed around should be that object instead of a random string.

redis-rs parses with url::Url itself, so you have no excuse here.

Fair enough. bf2bbc1 parses each server URL once, so the Store has Topology<Url>.

The redaction parses by hand so a Redis socket URL cannot leak its password through a strict URL parser. The blocking pool borrows raw keys so it stays free of the Stream key layout.

The options parse a server URL now, so a wrong URL fails early and the Store carries a typed Url. The debug redaction uses the Url API, not a hand parse. redis-rs already parses with url::Url, so this adds no new parser.

### What changed? This PR runs the writer suite of `temporalio-streams` on the Redis Store, standalone and on a cluster. The resend cases wrap each backend's Store in a fault-injecting Store, so they run on Redis too. The test runtime has a 5-minute Retention, so the big-record case doesn't keep megabytes on a local server for 7 days. `scripts/patch-sdk-rust.sh` now patches in `moetemp/sdk-rust#227`. It depends on #1 and on `moetemp/sdk-rust#227`. ### Why? The writers resend a batch after a Store failure, and the Store must write it once. This checks that on Redis. ### How did you test it? The writer suite runs with `cargo test -- --include-ignored` on the servers of `scripts/redis-test-servers.sh`, standalone with a replica and on a cluster of three primaries with one replica each. CI also runs Redis 7.0 and a FIPS build. --------- Co-authored-by: Maple Xu <45406854+xumaple@users.noreply.github.com>

moetemp mentioned this pull request Oct 27, 2026

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

Closed

moetemp closed this Oct 27, 2026

moetemp reopened this Oct 27, 2026

moetemp force-pushed the moe/brook/review/r1-redis-store branch 9 times, most recently from 4281b5a to 6adb884 Compare October 29, 2026 15:17

moetemp force-pushed the moe/brook/review/r1-redis-store branch from 6adb884 to 63486ac Compare October 29, 2026 16:13

moetemp changed the title ~~[Brook R1] Added the Redis Store with one Lua script per operation.~~ feat: add the Redis Store with one Lua script per operation Oct 29, 2026

xumaple reviewed Oct 29, 2026

View reviewed changes

Comment thread crates/streams-redis/src/store.rs Resolved

xumaple reviewed Oct 29, 2026

View reviewed changes

Comment thread crates/streams-redis/src/redis/errors.rs Resolved

Gave the crate its own error type and moved redis/mod.rs to redis.rs.

ae17136

The Store trait still returns Stream errors, so the runtime and lang get the same reasons on every backend.

xumaple reviewed Oct 29, 2026

View reviewed changes

Comment thread crates/streams-redis/src/lib.rs Resolved

Renamed the redis module to store, so it doesn't shadow the redis crate.

ac62122

moetemp commented Oct 30, 2026

View reviewed changes

Comment thread crates/streams-redis/src/store.rs Resolved

Comment thread crates/streams-redis/src/redis/errors.rs Resolved

Comment thread crates/streams-redis/src/lib.rs Resolved

moetemp requested a review from xumaple October 30, 2026 05:02

Removed default_retention from the Redis Store, and fixed a name doc.

3a29fcf

The Store trait no longer has default_retention, and nothing called the inherent one. No bridge selects a runtime by REDIS_RUNTIME_NAME.

xumaple reviewed Oct 30, 2026

View reviewed changes