Skip to content

[Brook] Took the Store parts of Dan's Jetty into Redis and memory. - #38

Closed
moetemp wants to merge 7 commits into
moe/brook/v4/s20-orphan-local-activitiesfrom
moe/brook/v4/s21-store-takeovers
Closed

moetemp wants to merge 7 commits into
moe/brook/v4/s20-orphan-local-activitiesfrom
moe/brook/v4/s21-store-takeovers

Conversation

@moetemp

@moetemp moetemp commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner

What changed?

This PR takes the Store parts of Dan's Jetty into Brook's Redis and memory Stores.

  • S117: each dedupe entry keeps a SHA-256 content digest of its batch. A resend with the last sequence and other content fails with seq_divergent (bug 7). testdata/digests.json pins the digest for the server and every SDK.
  • S114: only the owner closes a Stream. A Workflow-owned Stream closes from a Local Activity or a Workflow Producer, and a ref that names another run chain is refused. A client-created Stream closes only from a client. Others fail with stream_not_owner. The Store write calls take the whole StreamRef, as Jetty's do.
  • The crate vendors api a3 (780768b5) byte for byte. The Stream WIT marks the a3 fields that Brook's Stores don't read as @nexus.omit, so the SDK regen passes, and two stale docs of the Workflow buffer are fixed.
  • S115: a close Record has close_status. A caller can't set TERMINATED or TIMED_OUT. An auto close is COMPLETED.
  • S116: last reads the newest Records that pass the filter, oldest first, and skips withdrawn ones. Redis reads it with XREVRANGE.
  • S112: max_bytes caps a page's bytes. One larger Record still comes alone.

Why?

Dan's Jetty does these better, and the comparison took them over (S112, S114 to S117). A reused sequence with other content went missing with no error, anyone could close any Stream, a reader could not tell a failed close from a canceled one, and a UI had no way to paint the newest Records first.

Redis can't tie a run to its run chain, so a run of another chain that holds the owner's ref can still close the Stream. The native server checks the run. The Redis docs list it under known limits.

How did you test it?

  • built
  • run locally and tested manually
  • covered by existing tests
  • added new unit test(s)
  • added new functional test(s)

The Store suite runs on a standalone Redis 8.10, a three-primary cluster and the memory Store. New cases show a divergent resend, the owner check, the close status, a last read past a withdrawn Record and a capped page. cargo fmt --check and cargo clippy --workspace --tests -D warnings pass.

A reused sequence with other content was taken as a resend, so its Records went missing with no error. The dedupe entry keeps a content digest, and the shared vectors pin it for the server.
The owner check of a close needs the chain that the ref names. Jetty's Stores take the same signatures.
Anyone could close any Stream with a made-up result. The Store keeps the owner kind and run chain from the create and refuses other closes with stream_not_owner.
The crate's copy must equal the api's, so every SDK and the server read the same fields. It brings the close status, last-N reads, the page byte cap and the task tokens.
A reader could tell a failed close from a completed one only by parsing the failure. A caller's close gets COMPLETED, FAILED or CANCELED, and only the server's owner end sets TERMINATED or TIMED_OUT.
A chat UI paints the newest Records first, and a Workflow page must fit its marker. A last read skips Records that a newer Superseded item withdraws, and one larger Record still comes alone.
nexgen needs every proto field of a record declared or omitted, so the SDK regen failed on the a3 fields that Brook's Stores don't read. Two docs still described the Workflow buffer that S108 removed.
@moetemp

moetemp commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Closing as stale. This layer is folded into the v5 tip, #41, and the upstream review uses a fresh series instead. Plan: https://www.notion.so/4048fc567738814990b7c796ac28fe3f

@moetemp moetemp closed this Oct 25, 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