Skip to content

[Jetty] Merged Brook #41 and removed the auto close from the native Store. - #42

Open
moetemp wants to merge 27 commits into
moe/jetty/v2/k2-store-takeoversfrom
moe/jetty/v3/k3-no-auto-close
Open

moetemp wants to merge 27 commits into
moe/jetty/v2/k2-store-takeoversfrom
moe/jetty/v3/k3-no-auto-close

Conversation

@moetemp

@moetemp moetemp commented Oct 24, 2026 •

Copy link
Copy Markdown
Owner

This PR merges Brook #41 into the Jetty crate and removes the auto close from the native Store. It also fixes the native findings of an independent review.

Jetty's crate is Brook's crate plus a native Store, which calls the Stream service of the Temporal server. The SDK review dropped auto close at every layer, and the server dropped it in moetemp/temporal#102.

What changed?

Review fixes on native.

  • A FAILED_PRECONDITION with no Stream details or reason prefix maps to Storage. Every Stream error of the server has details, so only framework errors get this mapping.
  • Native create, append and close send a 5 s deadline, the default of the Redis Store, in place of the connection's 30 s.
  • A create conflict returns the server's stream_already_exists error, which names the stored Retention, not the requested one.
  • The service refuses a sequence past 2^53 - 1 on native too. The server stores the sequence as an exact i64, so it would accept it.
  • A test of no-wait filtered reads accepts the match in the first page. The server scans an empty filtered page to the end, which the contract allows.

The PR also pins the Jetty Core, moetemp/sdk-rust#210, at 2de24aef.

Please note that the native Store still doesn't send defer_withdrawal or a close's withdraw_run.

Why?

The review found that native writes used the connection's deadline, and that a conflict named the wrong Retention. It also found that framework errors could map to a Stream reason. The merge of #41 applies the Brook crate fixes to Jetty.

How did you test it?

On abd8955b, cargo test --workspace passes on memory, on Redis 8.10.1 standalone and as a cluster, and on native against a server built from moetemp/temporal#103. tests/native_calls.rs checks the deadline and the create conflict on a fake server. cargo fmt --check and clippy with -D warnings pass. The two commits after it, a merge of the comment pass of #41 and this PR's comment pass, change only comments and docs. They pass the same suites on memory and Redis. The sdk-streams fork CI passes on the head. It has no native lane.

The superproject branch temporalio/internal-ai-prototypes 2026/10/ai198-streams pins abd8955b, the tested commit.

  • Unit Tests
  • Staging
  • End to End Tests

A Stream closes only on a close call. Each script checks `closed` where the auto close step did,
so a create on a closed Stream still answers closed, an append is refused and a close answers
already.
A read writes nothing again, so a Reader needs read access only.
The revert touches only sdk-core, which the crate doesn't use.
…-auto-close

The native Store sends no auto close and has no `Store::auto_close`, and a Workflow create still sends `close_with_run`. The crate pins sdk-rust Jetty c10.
A Local Activity writes before its Workflow Task commits, so its close could outlive a failed task and fail the retried task's create.
A filtered read waited for a new append though entries up to the newest one were never scanned, and max_records 1 made one Store call per entry.
The withdraw-only batch failed on such a Stream, so a retry that only reads it failed for good before user code ran.
A user with the documented ACLs couldn't connect or run the scripts. A test now runs each role with exactly the listed commands.
Redis and memory refuse an empty id. Only the native frontend assigns one.
The new c8 commits touch only the Core CHANGELOG and Core tests, which the crate doesn't use.
…-auto-close

Takes the s23 review fixes. The new tests use the Jetty Store calls, and the crate pins sdk-rust Jetty c10.
The server keeps a sequence as an exact integer, so only the service refuses one past 2^53 - 1 there.
The server scans an empty filtered page to the end, while Redis and memory stop at the scan cap.
@moetemp moetemp changed the title [Jetty] Merged Brook s23 and removed the auto close from the native Store. [Jetty] Merged Brook #41 and removed the auto close from the native Store. Oct 30, 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