runtime-local: extract the local task-drive layer from flowctl - #3416
Merged
Conversation
This was referenced Aug 27, 2026
jacobmarble
previously approved these changes
Aug 28, 2026
| /// frontier: a [`TxnState::new`] -> [`push_binding`] -> [`finish_txn`] sequence | ||
| /// over `items`, each of which carries the binding it feeds and its clock. | ||
| #[allow(clippy::too_many_arguments)] | ||
| pub fn write_transaction_for_bindings( |
Contributor
There was a problem hiding this comment.
This must be called by a subsequent change in the stack, I can't find any call to it in this change.
jgraettinger
force-pushed
the
johnny/v2-tests-2-runtime-local
branch
from
August 30, 2026 17:56
38a589b to
a2d44ab
Compare
jgraettinger
force-pushed
the
johnny/v2-tests-2-runtime-local
branch
3 times, most recently
from
August 31, 2026 22:19
e392a76 to
a4ad7c4
Compare
jgraettinger
force-pushed
the
johnny/v2-tests-2-runtime-local
branch
2 times, most recently
from
September 1, 2026 00:51
2d160a7 to
13ac318
Compare
Member
Author
|
@jacobmarble PTAL |
jgraettinger
force-pushed
the
johnny/v2-tests-2-runtime-local
branch
from
September 1, 2026 04:46
13ac318 to
bf1c380
Compare
Relocate the `flowctl preview` snapshot tests into `tests/preview/`, a sibling
of `tests/soak/`, and re-point them at `flowctl raw preview-next`.
Three mechanical consequences of the swap:
* Fixtures become NDJSON: `raw preview-next` reads `["collection",<doc>]` lines
separated by `{"commit": true}` markers, where legacy `preview` read a nested
JSON array of transactions. Transaction boundaries are preserved, and the
files are renamed `.ndjson`.
* `pytest-insta` derives snapshot names from the test node ID, so the added
path segment renames every snapshot.
* Legacy `preview` printed bare documents for derivations while
`raw preview-next` uses `["collection",<doc>]` framing everywhere, so the two
derive snapshots are regenerated. Stripping the new wrapper reproduces the old
files exactly.
`tests/README.md` becomes a roadmap over `preview/` and `soak/`.
Four places computed the same even split of the `u32` key space from scratch: `activate`'s initial-splits path, and three shuffle test harnesses which were literally identical. `even_splits` returns the range, its encoded LabelSet, and the full shard ID for each of `key_splits * rclock_splits` shards tiling the cross product, ordered key-major — which is also the lexicographic order of their IDs. Callers that want only one dimension pass 1 for the other. The copies' `if i == 0` / `if i == count-1` end guards go away: computing widths in `u64` over a `u32::MAX + 1` space makes the tiling exact at both ends on its own, for any count. A snapshot test pins the concrete IDs and ranges, and a loop asserts the general property. Behavior-preserving at every call site.
A `SessionLoop`'s descriptor now hands its directory over. The serve loop removes it on every exit path — graceful stop, session error, and failed open alike — always after the RocksDB is torn down (`shard::rocksdb::OwnedDir`). An absent descriptor gets a tempdir the shard makes and owns identically, so ownership is unconditional and a shard is drop-safe: unwinding at any point takes its storage with it. Go's `removeRecoveryDir` is then a no-op wherever a descriptor was sent. What remains are the cases with no handover: derive-sqlite, which keeps its directory and reaches Rust through `Task.sqlite_vfs_uri`, and a task which died before its SessionLoop was consumed. The cost is that a controller can no longer open a shard's RocksDB by path once its stream ends — which is how `flowctl preview` seeded `--initial-state` and read back `--output-state`. Both move onto the protocol: - `SessionLoop.initial_connector_state_json` establishes a base document before any recovery scan can observe it. - `SessionLoop.report_final_state` attaches the shard's reduced connector state to each `Stopped` it sends its controller. A harness now names no path at all and takes the built-in tempdir handling. `Stopped` gains a field and so loses `Copy`, which is the churn in the leader and shard actors. `shard/service.rs` also states the rule its controllers depend on: only the `spawn_*` adapters put an `Err` on a response stream, and only once `serve` has returned, so a stream's `Err` is the whole loop's outcome rather than a recoverable per-session hiccup. `flowctl raw preview-next` does not build at this commit; the next commit extracts that layer and adopts all of this.
`flowctl raw preview-next` and the forthcoming catalog-test runner need the same thing — run a runtime-next task locally with synthetic shards, no Gazette broker, no etcd, no Go consumer — and differ only in the three runtime-next host seams. Moved out of crates/flowctl/src/raw/preview_next/: the three drivers, the run-scoped services, the shard topology, and the segment-writing half of fixture.rs (as segments.rs). flowctl is migrated onto the crate here, in the same commit, so no divergent copy ever exists. Two seams had to be generalized so the layer could stop knowing about preview: - `Controls` is now generic over the publisher and logger factories. - `start_with_shuffle_leader` takes a `build_shuffle` callback instead of a `fixture: bool`. This is what keeps journal *authorization* in flowctl: reading live journals needs a logged-in user token, and only flowctl has one. The split of fixture.rs follows the same line: flowctl keeps the file format and session planning, runtime-local takes the writer. Clock policy stays with the caller — `write_transaction_for_bindings` accepts each document's clock — because preview must keep reproducing the legacy harness's `3600 * ordinal + index` stamping while a test runner paces clocks against its own synthetic time. That routine also tags documents per *binding* rather than per collection, which the catalog-test harness needs to honor per-transform read delays. The synthetic topology is rebuilt on shared pieces: ranges from `labels::shard::even_splits`, and shard IDs from `assemble::shard_id_prefix` under generation zero, so a local run's shards take the shape they would carry in production rather than `preview-derive/<task>/000`. Each driver likewise names its runtime task after the task's own catalog name, which is what ops logs attribute against. `Controls` carries `--initial-state` and `--output-state` on shard zero's `SessionLoop` rather than reaching into RocksDB by path, so no `Run` hosts a RocksDB directory and each driver returns the final state its shard zero reported at `Stopped`. `tests/connector_state.rs` covers that round trip. Otherwise behavior-preserving, but for one defect the move made legible. `Run` documented that its server task aborts on drop, but `_server_task` is a plain `JoinHandle`, and dropping one *detaches* its task rather than aborting it — so the tonic server outlived every `Run`. Add the `Drop` the comment assumed.
jgraettinger
force-pushed
the
johnny/v2-tests-2-runtime-local
branch
from
September 1, 2026 15:34
bf1c380 to
8426df8
Compare
jacobmarble
approved these changes
Sep 1, 2026
github-actions
Bot
added
pending:agent
github-actions
Bot
removed
pending:agent-api
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
2 participants
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.
Description:
Stack 2 of 5.
flowctl raw preview-nextand the forthcoming catalog-test runner need the same thing — run a runtime-next task locally with synthetic shards, no Gazette broker, no etcd, no Go consumer — and differ only in three runtime-next host seams. This moves that machinery into a newruntime-localcrate rather than let a second copy grow beside it, migratingflowctlonto it in the same commit so no divergent copy ever exists.Four commits:
tests: move preview snapshots to tests/preview— re-points theflowctl previewsnapshot tests atraw preview-next. The baseline the extraction is checked against.labels: hoist even shard splitting into labels::shard::even_splits— four places computed the same even split of theu32key space.runtime-localis the fifth caller, so the helper lands first.runtime-next: shard handlers own their RocksDB directory— a shard now owns and removes its own storage, which is what makes a dropped local run safe, plus the protocol changes that follow from it.runtime-local: extract the local task-drive layer from flowctl— the move itself.Workflow steps:
flowctl raw preview-nextbehaves identically.poetry run pytest tests/previewruns the snapshot suite — it is not wired into CI, so it's a developer-run baseline; CI coverage for this stack arrives in #3417.Documentation links affected:
None.
crates/runtime-local/README.mdis new;tests/README.mdbecomes a roadmap overpreview/andsoak/, with the detailed instructions moving totests/preview/README.md.Notes for reviewers:
flowctl raw preview-nextis broken against the newSessionLoopuntil the commit after it, which extracts that layer and adopts all of it. Teaching a soon-to-be-replaced copy semantics it will never use wasn't worth it, so the two are separable stories but not separable builds. Review them as a pair; the PR tip builds.Ownership is the change with consequences outside this stack: a controller can no longer open a shard's RocksDB by path once its stream ends, so
--initial-stateand--output-statemove onto the protocol,StoppedlosesCopy, and Go'sremoveRecoveryDirbecomes a no-op wherever a descriptor was sent.The extraction is otherwise behavior-preserving. Three things worth a close look:
Controlsover its publisher and logger factories, andbuild_shufflereplacingfixture: bool. The latter is what keeps journal authorization in flowctl: reading live journals needs a logged-in user token, and only flowctl has one.3600 * ordinal + indexstamping while the test runner paces clocks against its own synthetic time.Rundocumented that its server task aborts on drop, but a plainJoinHandledetaches rather than aborts, so the tonic server outlived everyRun.Stack — each PR is based on the one above it, so review only the top commits of each.
flowctl raw test#3417 — catalog-tests: the harness, andflowctl raw test