mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-09 01:35:18 +02:00
MR-771: demote Run to direct-publish via expected_table_versions CAS
mutate_as and load now write directly to target tables and call the publisher once at the end with per-table expected versions; the Run state machine, _graph_runs.lance writers, __run__ staging branches, and server /runs/* endpoints are removed. Multi-statement mutations remain atomic at the manifest level via an in-memory MutationStaging accumulator that gives read-your-writes within a query and a single publish at the end. Concurrent-writer conflicts surface as ExpectedVersionMismatch (HTTP 409 manifest_conflict) instead of the old DivergentUpdate merge shape. Documents one known limitation in docs/runs.md: a multi-statement mid-query failure where op-N writes a Lance fragment and op-N+1 fails leaves Lance HEAD ahead of the manifest until a follow-up introduces per-table Lance branches. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
4e5374a85e
commit
35be20cb05
28 changed files with 1188 additions and 3216 deletions
89
docs/releases/v0.4.0.md
Normal file
89
docs/releases/v0.4.0.md
Normal file
|
|
@ -0,0 +1,89 @@
|
|||
# Omnigraph v0.4.0
|
||||
|
||||
Omnigraph v0.4.0 demotes the Run state machine to commit metadata via the
|
||||
publisher's CAS, fixing the cancellation hole that motivated MR-771 and
|
||||
reducing the engine's surface area.
|
||||
|
||||
## Highlights
|
||||
|
||||
- **Direct-to-target writes (MR-771)**: `mutate_as` and `load` write
|
||||
directly to the target tables and call
|
||||
`ManifestBatchPublisher::publish` once at the end with
|
||||
`expected_table_versions`. No more `__run__<id>` staging branches, no
|
||||
more `RunRecord` state machine. Cross-table OCC is enforced inside the
|
||||
publisher's row-level CAS on `__manifest`.
|
||||
- **Cancellation safety by construction**: a dropped mutation future
|
||||
leaves no graph-level state — only orphaned Lance fragments, reclaimed
|
||||
by `omnigraph cleanup`. The "zombie run" cascade documented in
|
||||
`.context/zombie-run-investigation.md` is gone.
|
||||
- **Read-your-writes inside multi-statement mutations**: a `.gq` query
|
||||
that inserts and then references a row in the same statement now sees
|
||||
its own writes via an in-process `MutationStaging` cache, even though
|
||||
no manifest commit happens between ops.
|
||||
- **Structured conflict surface**: concurrent writers race through the
|
||||
publisher's CAS; the loser surfaces as
|
||||
`ManifestConflictDetails::ExpectedVersionMismatch { table_key,
|
||||
expected, actual }`. The HTTP server maps this to **409 Conflict** with
|
||||
a structured `manifest_conflict` body so clients can detect-and-retry
|
||||
without parsing the message.
|
||||
|
||||
## Removed
|
||||
|
||||
This is a breaking release. Pre-0.4.0 / no SLA.
|
||||
|
||||
- `omnigraph::db::{RunRecord, RunStatus, RunId}` types and the
|
||||
`_graph_runs.lance` / `_graph_run_actors.lance` Lance datasets.
|
||||
- Engine APIs `begin_run`, `begin_run_as`, `publish_run`,
|
||||
`publish_run_as`, `abort_run`, `fail_run`, `terminate_run`,
|
||||
`list_runs`, `get_run`.
|
||||
- HTTP endpoints: `GET /runs`, `GET /runs/{run_id}`, `POST
|
||||
/runs/{run_id}/publish`, `POST /runs/{run_id}/abort`. The
|
||||
`RunListOutput` and `RunOutput` schemas are removed from the OpenAPI
|
||||
document.
|
||||
- CLI subcommands: `omnigraph run list`, `omnigraph run show`, `omnigraph
|
||||
run publish`, `omnigraph run abort`. Use `omnigraph commit list`
|
||||
reading the commit graph for audit history.
|
||||
- Cedar policy actions `run_publish` and `run_abort`. Existing
|
||||
`policy.yaml` files referencing these actions will fail validation —
|
||||
remove the rules; the `change` action covers the equivalent gating.
|
||||
|
||||
## Behavior changes
|
||||
|
||||
- `mutate_as` / `load` are now **atomic per query, single publish at the
|
||||
end**. A failed mutation leaves the target unchanged with no
|
||||
intermediate manifest commits.
|
||||
- The `OmniError::manifest_conflict` shape produced by concurrent
|
||||
writers is now `ExpectedVersionMismatch` (was `MergeConflict::DivergentUpdate`
|
||||
via the run merge path). Clients that match on the conflict body must
|
||||
switch to inspecting `manifest_conflict.table_key/expected/actual`.
|
||||
|
||||
## Known limitation
|
||||
|
||||
A multi-statement mutation that writes a Lance fragment in op-N and then
|
||||
fails in op-N+1 leaves the touched table with Lance HEAD ahead of the
|
||||
manifest. The next mutation against that table fails with
|
||||
`ExpectedVersionMismatch`. Most validation runs before any Lance write,
|
||||
so single-statement mutations are unaffected; the narrow path is
|
||||
multi-statement queries with late-op failures. Tracked as a follow-up;
|
||||
see [docs/runs.md](../runs.md#known-limitation-mid-query-partial-failure-on-the-same-table)
|
||||
for the workaround.
|
||||
|
||||
## Upgrade notes
|
||||
|
||||
- **Stale `__run__*` branches and `_graph_runs.lance`** in legacy v0.3.x
|
||||
repos are *inert* — the engine no longer reads them — but they remain
|
||||
on disk until production cleanup. MR-770 owns the destructive sweep;
|
||||
this release deliberately does not touch legacy bytes.
|
||||
- The `is_internal_run_branch` predicate is kept as a defense-in-depth
|
||||
guard against users naming a branch `__run__*`. It will be removed in
|
||||
a follow-up alongside MR-770.
|
||||
- External scripts hitting `/runs/*` will now receive 404. Migrate them
|
||||
to `/commits` for audit history; mutation status is implied by the
|
||||
HTTP response on `/change` itself.
|
||||
|
||||
## Included Changes
|
||||
|
||||
- MR-771 — Demote Run: write directly to target via publisher
|
||||
- MR-766 — `ManifestBatchPublisher::publish` accepts per-table
|
||||
`expected_table_versions` (landed earlier; this release wires it in
|
||||
end-to-end)
|
||||
118
docs/runs.md
118
docs/runs.md
|
|
@ -1,38 +1,98 @@
|
|||
# Runs (transactional graph mutations)
|
||||
# Runs — REMOVED (MR-771)
|
||||
|
||||
`db/run_registry.rs` + run lifecycle in `db/omnigraph.rs`. Stored in `_graph_runs.lance` and `_graph_run_actors.lance`.
|
||||
The Run state machine and `__run__<id>` staging branches were removed in
|
||||
MR-771. `mutate_as` and `load` now write **directly to the target table**
|
||||
and call `ManifestBatchPublisher::publish` once at the end with
|
||||
`expected_table_versions` (the per-table manifest versions captured before
|
||||
the first write). Cross-table OCC is enforced inside the publisher; the
|
||||
publisher's row-level CAS on `__manifest` is the single fence.
|
||||
|
||||
## RunRecord
|
||||
## What this means in practice
|
||||
|
||||
```
|
||||
RunRecord {
|
||||
run_id: RunId (ULID),
|
||||
target_branch: String, // where the run will publish
|
||||
run_branch: "__run__<id>", // ephemeral isolation branch
|
||||
base_snapshot_id: String,
|
||||
base_manifest_version: u64,
|
||||
operation_hash: Option<String>, // idempotency key
|
||||
actor_id: Option<String>,
|
||||
status: Running | Published | Failed | Aborted,
|
||||
published_snapshot_id: Option<String>,
|
||||
created_at, updated_at: i64 (microseconds),
|
||||
}
|
||||
```
|
||||
- No `RunRecord`, no `_graph_runs.lance`, no `_graph_run_actors.lance`.
|
||||
- No `omnigraph run *` CLI subcommands and no `/runs/*` HTTP endpoints.
|
||||
- No `__run__<id>` staging branches. (Legacy on-disk artifacts from
|
||||
pre-MR-771 repos are inert; MR-770 sweeps them in production.)
|
||||
- Cancelled mutation futures leave **no graph-level state** — only orphaned
|
||||
Lance fragments, which the existing `omnigraph cleanup` pipe reclaims.
|
||||
|
||||
## Lifecycle
|
||||
## Read-your-writes within a multi-statement mutation
|
||||
|
||||
1. `begin_run(target_branch, op_hash)` / `begin_run_as(target_branch, op_hash, actor_id)` — forks `__run__<id>` from the target's current head, appends a `RunRecord`.
|
||||
2. Mutations on `run_branch` (via the normal write APIs) — isolated from concurrent activity on the target.
|
||||
3. `publish_run(id)` / `publish_run_as(id, actor)`:
|
||||
- **Fast path**: if the target hasn't moved since `base_snapshot_id`, promote the run snapshot directly.
|
||||
- **Merge path**: if it has moved, perform a three-way merge (see [merge.md](merge.md)) into the target.
|
||||
- On success: `status = Published`, `published_snapshot_id` set, run branch cleaned up asynchronously.
|
||||
4. `abort_run(id)` / `fail_run(id)` — terminal; cleans up run branch best-effort.
|
||||
A `.gq` query with multiple ops (e.g. `insert Person … insert Knows …`)
|
||||
must observe earlier ops' writes when validating later ops (referential
|
||||
integrity, edge cardinality). After demotion this is implemented via an
|
||||
in-process `MutationStaging` accumulator in
|
||||
`crates/omnigraph/src/exec/mutation.rs`:
|
||||
|
||||
## Idempotency
|
||||
- On the first touch of each table, the pre-write manifest version is
|
||||
captured into `expected_versions[table_key]`.
|
||||
- Subsequent ops on the same table re-open the dataset at the locally
|
||||
staged Lance version (bypassing the manifest, which has not been
|
||||
committed yet) so they see prior writes.
|
||||
- One `commit_with_expected(updates, expected_versions)` at the end
|
||||
publishes the lot atomically. Cross-table conflicts surface as
|
||||
`ManifestConflictDetails::ExpectedVersionMismatch`.
|
||||
|
||||
`operation_hash` is an optional field clients can use to detect a duplicate `begin_run` retry.
|
||||
This upholds [docs/invariants.md §VI.23](invariants.md) (atomicity per
|
||||
query) and §VI.25 (read-your-writes within a multi-statement mutation —
|
||||
previously aspirational, now upheld).
|
||||
|
||||
## Cleanup
|
||||
## Conflict shape
|
||||
|
||||
`cleanup_terminal_run_branches_for_target(branch)` is called as branches change; failures are swallowed (lazy cleanup on next branch op).
|
||||
Concurrent writers to the same `(table, branch)` produce exactly one
|
||||
success and one failure. The losing writer's error is
|
||||
`OmniError::Manifest` with kind `Conflict` and details
|
||||
`ManifestConflictDetails::ExpectedVersionMismatch { table_key, expected,
|
||||
actual }`. The HTTP server maps this to **409 Conflict** with body
|
||||
`{"error": "...", "code": "conflict", "manifest_conflict": { "table_key":
|
||||
"...", "expected": N, "actual": M }}` — see [docs/server.md](server.md).
|
||||
|
||||
## Audit
|
||||
|
||||
`actor_id` lands in `_graph_commits.lance` via `record_graph_commit` (no
|
||||
intermediate run record). Audit history is queried via `omnigraph commit
|
||||
list`.
|
||||
|
||||
## Migration code
|
||||
|
||||
`db/manifest/migrations.rs` does not change. Active deletion of
|
||||
`_graph_runs.lance` belongs in MR-770 (the production sweep) — this PR
|
||||
stops *creating* run state but does not destroy legacy bytes on disk.
|
||||
|
||||
## Known limitation: mid-query partial failure on the same table
|
||||
|
||||
A multi-statement `.gq` mutation where op-N writes a Lance fragment
|
||||
successfully and op-N+1 then fails leaves the touched table at
|
||||
`Lance HEAD = manifest_version + 1`. The query is atomic at the manifest
|
||||
level (the publisher never publishes, so reads at the pinned manifest
|
||||
version do *not* see op-N's data), but the *next* mutation against the
|
||||
same table fails loudly with
|
||||
`ManifestConflictDetails::ExpectedVersionMismatch` because
|
||||
`ensure_expected_version` enforces strict equality between Lance HEAD and
|
||||
the manifest's pinned version.
|
||||
|
||||
**Why the engine doesn't auto-rollback**: Lance's `Dataset::restore()` is
|
||||
*not* a rewind — it appends a new commit (containing the desired
|
||||
historical version's data) and advances HEAD further. There is no Lance
|
||||
API to delete a committed version. A proper fix requires writing each
|
||||
mutation's per-table fragments to a *transient Lance branch* on the
|
||||
sub-table, then fast-forwarding main on success or dropping the branch
|
||||
on failure. That work is tracked as a follow-up to MR-771; in the
|
||||
meantime:
|
||||
|
||||
- **In practice this is rare.** Most schema-language validation
|
||||
(`@key`, `@enum`, `@range`, intra-batch uniqueness, edge-endpoint
|
||||
existence) runs *before* any Lance write inside the failing op, so
|
||||
single-statement mutations never trip this. The narrow path is
|
||||
multi-statement queries (`insert ... insert ...`,
|
||||
`insert ... update ...`) where a late op fails on validation that
|
||||
depends on earlier ops' staged data.
|
||||
- **Workaround**: callers that hit this should refresh the handle and
|
||||
retry the mutation; if Lance HEAD remains drifted the
|
||||
`omnigraph cleanup` command will GC the orphan version once a later
|
||||
successful commit on the same table moves HEAD past it. (`cleanup`
|
||||
cannot reclaim an orphan that *is* the current Lance HEAD; that case
|
||||
needs the per-table-branch follow-up to fully heal.)
|
||||
|
||||
The cancellation case (future drop mid-mutation) has the same shape and
|
||||
the same workaround.
|
||||
|
|
|
|||
|
|
@ -19,10 +19,6 @@ Axum 0.8 + tokio + utoipa-generated OpenAPI. Single repo per process; deploy mul
|
|||
| POST | `/branches` | bearer + `branch_create` | create | `server_branch_create` |
|
||||
| DELETE | `/branches/{branch}` | bearer + `branch_delete` | delete | `server_branch_delete` |
|
||||
| POST | `/branches/merge` | bearer + `branch_merge` | merge `source → target` | `server_branch_merge` |
|
||||
| GET | `/runs` | bearer + `read` | list | `server_run_list` |
|
||||
| GET | `/runs/{run_id}` | bearer + `read` | show | `server_run_show` |
|
||||
| POST | `/runs/{run_id}/publish` | bearer + `run_publish` | publish | `server_run_publish` |
|
||||
| POST | `/runs/{run_id}/abort` | bearer + `run_abort` | abort | `server_run_abort` |
|
||||
| GET | `/commits?branch=` | bearer + `read` | list | `server_commit_list` |
|
||||
| GET | `/commits/{commit_id}` | bearer + `read` | show | `server_commit_show` |
|
||||
|
||||
|
|
@ -32,7 +28,14 @@ Only `/export` streams (`application/x-ndjson`, MPSC channel + `Body::from_strea
|
|||
|
||||
## Error model
|
||||
|
||||
Uniform `ErrorOutput { error, code?, merge_conflicts[] }` with `code ∈ unauthorized | forbidden | bad_request | not_found | conflict | internal`. Merge conflicts attach structured `MergeConflictOutput { table_key, row_id?, kind, message }`.
|
||||
Uniform `ErrorOutput { error, code?, merge_conflicts[], manifest_conflict? }` with `code ∈ unauthorized | forbidden | bad_request | not_found | conflict | internal`. Merge conflicts attach structured `MergeConflictOutput { table_key, row_id?, kind, message }`.
|
||||
|
||||
`manifest_conflict` is set on **publisher CAS rejections** (HTTP 409): the
|
||||
caller's pre-write view of one table's manifest version was stale.
|
||||
`ManifestConflictOutput { table_key, expected, actual }` tells the client
|
||||
which table to refresh and retry. This is the conflict shape produced by
|
||||
concurrent `/change` or `/ingest` calls landing the same `(table, branch)`
|
||||
race (MR-771 / MR-766).
|
||||
|
||||
HTTP status codes used: 200, 400, 401, 403, 404, 409, 500.
|
||||
|
||||
|
|
@ -64,5 +67,5 @@ See [deployment.md](deployment.md) for token-source operational details.
|
|||
|
||||
- CORS — not configured; add `tower_http::cors` if needed.
|
||||
- Rate limiting — none.
|
||||
- Pagination — none (commits/branches/runs return everything; export streams).
|
||||
- Pagination — none (commits/branches return everything; export streams).
|
||||
- Multi-tenant routing — one repo per process.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue