Skip to content

bug: one storage write with an unknown outcome stops the server until a manual unlock #1067

Closed

Description

@ragnorc

Actual result

A cluster server running 0.14.0-edge+d883c49 stopped itself during an ordinary write on main and stayed down until an operator released its lock by hand. It ran a single-server deployment against an S3-compatible bucket, with about ten agent writes an hour. The log (the triggering failure itself is not logged):

02:12:51.961 WARN  lance::io::commit: Failed to clean up orphaned transaction file '12770445047132701458-0c410ce6-c45b-4dff-bcf6-b5f652b87aaf.txn': LanceError(IO): Generic owned storage error: a prior storage mutation has an uncertain outcome
02:12:51.963 ERROR omnigraph_server: owned completion uncertain; containing process
02:12:51.963 ERROR tower_http::trace::on_failure: response failed classification=Status code: 500 Internal Server Error latency=6575 ms
02:12:51.972 ERROR omnigraph_server: known owners drained after uncertain completion; terminating process

Afterwards:

  • The process exited nonzero and kept its cluster admission, as docs/user/deployment.md describes for uncertain work. The orchestrator did not restart it; a restart would have been refused anyway while the lock was held.
  • Every graph on the server was unavailable, for reads too, for 48 minutes, until cluster force-unlock <id> and a redeploy.
  • The write that hit the error never published. The last commit is the one before it, so the graph was consistent. The only effect of the uncertainty was the outage.

The same deployment ran v0.13.0 for nine days without this. This is the behavior #899 introduced:

  • lance_access/scoped.rs forces client_max_retries=0 and lance_aimd_max_retries=0.
  • StorageIoScope (omnigraph-storage/src/scoped.rs) marks the whole scope uncertain when any mutation ends without a settled result. The scope then refuses every later mutation, and the server contains the process.

Under v0.13.0 the same transient storage error was retried inside the request and never surfaced.

Mutation::finish treats every error as uncertain, except AlreadyExists and Precondition on a conditional request. That covers responses that are not ambiguous at all: a delete answered NotFound, an authorization failure, throttling. It also includes a 5xx from an idempotent write of a fresh, uniquely named object, whose outcome cannot matter.

Expected result

A storage error affects the operation that issued it, not the process. The operation ends with a definite outcome (published, lost a conflict, or failed before publication) and the client can act on it. Other requests, other branches and other graphs keep serving. A server that stops for any reason can be restarted by the orchestrator without an operator proving anything about the previous process.

Likely by-design fix

The scope exists so that a clean shutdown can prove no request from this owner can still land, and so the next owner never sees a late write. That proof is only needed if a late-landing request could change shared state. The fix is to make that impossible by construction, so no runtime proof is needed:

  1. Classify every storage mutation by the kind of key it writes, enforced by type.
    • Fresh immutable keys: data files, deletion files, index segments, {read_version}-{uuid}.txn transaction files, detached manifests with their random version ids, and Blob sidecars. Applying such a write late, or twice, is harmless. Until a publication names the object it is unreachable (invariant 5), and the collector reclaims it as an orphan. Its outcome never needs proving, and retrying it with the same bytes is safe.
    • Conditional publications: put-if-absent __manifest/_versions/{n}.manifest, and If-Match updates of the cluster state and the lock record. Resolve uncertainty by re-issuing the identical conditional request, then reading the object back. The precondition guarantees at most one copy applies. A late duplicate, from this owner or a dead one, fails its precondition. If the stored object carries this attempt's transaction id or digest, the operation committed; otherwise it lost a conflict. Either way the outcome is definite.
    • Unconditional writes or deletes of a mutable key: the only class where a late request can corrupt state, for example an older write overwriting a newer one. Remove it. Route every such site through a conditional API, and add a source guard (in the style of forbidden_apis.rs) that refuses an unconditional mutation of a non-fresh key. Branch-ref rewrites through Lance's replace_metadata need checking against this: if they are not conditional, they belong here.
  2. Drop the process-wide poison. Once 1 holds, an error is resolved within its own operation, by bounded re-issue and read-back. If resolution's own reads cannot finish, the client gets a typed unknown-outcome error. Only that branch refuses writes until a resolver settles the key; reads and other graphs stay available. Process containment remains for genuine invariant breaks, not transport errors.
  3. Re-enable client retries for idempotent requests: reads, fresh-key puts, and conditional requests re-issued with the same precondition and payload. Keep them off only for the class that 1 removes.
  4. Make restart automatic. A dead owner's late requests can no longer violate correctness: each publication is a conditional write, so two owners can at worst contend, never corrupt. The lock then only coordinates liveness. A successor can take over a dead owner's lock with a conditional update of the lock record, guarded by a lease so a live owner is not displaced. Restart-on-failure becomes safe, and force-unlock remains for exceptional recovery.
  5. Log the trigger. Record the first mutation that left an outcome unknown: operation, key class, error kind and status. Today only the refusal that follows it is logged, so the cause of an outage cannot be recovered from the log (invariant 11).

Each step holds on its own. A narrower first change would do steps 5 and 2, plus classifying definite responses as definite. That removes the outage without the full redesign.

GQT reproduction or limitation

Not attached. DST can express the fault: inject one 5xx, then separately a response lost after the request was applied, on a data-file put during a served write. Then assert three things: the write gets a definite outcome, a concurrent write to another graph succeeds, and the server keeps serving. A second fault (an abandoned conditional put to the manifest that lands after the client gave up) checks the resolution rule in step 1.

Version

omnigraph 0.14.0-edge+d883c49 (internal-schema 16, http-contract 0.13), from the edge-20261017-d883c49 release archive.

Execution path

omnigraph-server --cluster <s3 root> --require-all-graphs, one replica, served change writes on main from several clients.

Environment

Linux x86_64 container (Debian trixie) on Railway; S3-compatible object storage (Tigris). OBJECT_STORE_CLIENT_MAX_RETRIES unset.

Reproduction output

The four log lines above are the complete output from the triggering write. The previous 2,000 log lines contain no other ERROR and one unrelated WARN (a schema-identity rename hint), apart from 503s answered during startup.

Activity

  1. ragnorc commented on Oct 18, 2026

    @ragnorc
    ContributorAuthor

    Two parts of the expected behavior the proposal above leaves implicit: what the client is told, and what the server does while storage itself is failing.

    6. Report an unknown outcome as its own result, on every surface

    Each write ends in exactly one of three results, and the client must be able to tell them apart:

    • committed, with the graph commit it produced;
    • not committed: rejected before publication, lost a conflict, or failed with a definite storage answer. Retrying from a fresh read is safe.
    • outcome unknown: the write may have published.

    MCP already returns "outcome_unknown": true. The HTTP data routes and the CLI do not: the write in this report came back as a plain 500, which reads as "not committed" and invites a blind replay. The fix:

    • One typed result everywhere. HTTP, the CLI and MCP all carry the same result. On HTTP that is a distinct error code in the body; on the CLI, a distinct exit code next to the precondition-failure code.
    • A way to check. The result carries the identity the client needs to find out what happened: the branch and its head before the write, and the attempt's transaction id. A follow-up read can then answer "did it land" without guessing from timestamps.
    • Unknown is the last resort. It is used only when the resolution in step 1 could not finish, never for a definite storage error and never for a failure before publication.

    Precedents: FoundationDB's commit_unknown_result asks clients to make transactions idempotent or check before retrying. Iceberg's CommitStateUnknownException is raised only after checkCommitStatus re-reads the table and still cannot decide, and an unknown commit does not delete its files, because it may have succeeded.

    7. While storage is failing, refuse writes quickly and keep reads

    A single failed request should affect only its operation (step 2). Storage can also fail broadly: an outage, sustained throttling, or credentials that stopped working. For that case the server needs a circuit breaker, not process exit:

    • Trip on evidence. Consecutive definite failures, or unknown outcomes that resolution cannot settle within its deadline, trip the breaker for the affected storage root, or for the graph if the failures are confined to one.
    • While open:
      • write admission fails fast with a typed storage_unavailable error that carries a retry-after hint;
      • reads keep serving;
      • nothing new is admitted that would add unresolved state.
    • Recover automatically. A probe resumes writes: a put and delete of a fresh key, plus resolution of any pending conditional write. There is no restart and no operator step.
    • Readiness reports it. /readyz reports writes as degraded per graph, rather than reporting the whole server not ready. An orchestrator must not cycle a server that is still serving reads.

    This is the same scoping PostgreSQL applies around its one deliberate crash case. Since 2018 it panics when fsync fails, because its in-memory pages can no longer be trusted. That crash is safe only because the postmaster restarts automatically (restart_after_crash) and WAL replay reaches a known state with no operator. A failed request that leaves the process's state trustworthy does not justify a crash. One that does needs automatic recovery (step 4), or the crash becomes an outage that lasts until someone notices it. This one lasted 48 minutes.

    Tests

    The DST cases in the original report extend naturally:

    • An unknown outcome that resolution settles returns a definite result.
    • One it cannot settle returns outcome unknown on HTTP, the CLI and MCP alike, and a client check on the returned identity agrees with the eventual state.
    • A sustained storage fault trips the breaker: writes get storage_unavailable while reads succeed, and writes resume after the fault clears, without a restart.
  2. ragnorc commented on Oct 18, 2026

    @ragnorc
    ContributorAuthor

    Three findings from the research behind #1017 bear on this proposal.

    1. Branch-ref rewrites are unconditional, which answers step 1's open question. In Lance 11.0.0, Branches::replace_metadata (lance-11.0.0/src/dataset/refs.rs:528-554) checks existence with a HEAD, reads the ref, edits it, and writes it back with a plain put. There is no ETag or version check. Branches::delete (:560) is unconditional too, and object_store has no conditional delete. So OmniGraph's retirement-marker rewrite and ref unlink fall in the third class. A late marker rewrite can even recreate a ref the delete has already unlinked.

    #1017 proposes making branch retirement a zero-retry publication on the branch's own __manifest chain, a tombstone version. The marker, archive and unlink then become follow-ups of that publication, and a late or repeated landing changes nothing logical: a recreated ref is retired and unregistered, and cleanup reclaims it. A guard like step 1's could admit these writes only in that role.

    2. Local roots have no conditional update, which affects steps 1 and 4.

    • object_store's LocalFileSystem returns NotImplemented for PutMode::Update (object_store-0.13.2/src/local.rs:336).
    • write_text_if_match on a local root compares content and then replaces the object through a staged write. Its own doc says this is "not a cross-process barrier by itself" (crates/omnigraph-storage/src/lib.rs:1117-1127).
    • So "If-Match updates of the cluster state and the lock record" are conditional publications on S3 and Azure, but not on local roots.
    • Step 4's takeover by a conditional update of the lock record cannot work there as written.

    The one compare-and-swap that holds on every backend is the one manifests use: a numbered object written only if absent. Locally that is a hard link that fails on an existing target; on S3 and Azure it is If-None-Match: *. A lease could use lock generations in that form.

    3. A retried conditional create can misreport its own success, which affects step 3. object_store retries a conditional PUT on 5xx, 429 and 408 responses, and Lance's AIMD layer retries throttling.

    • If a retried create's first attempt landed, the retry returns AlreadyExists.
    • Lance's put_ref_if_absent (refs.rs:147-163) then reports RefConflict without checking whose ref it is.
    • Manifest commits avoid this through verify_commit_outcome (lance-11.0.0/src/io/commit.rs:200), which compares the landed transaction with its own.

    Re-enabling client retries for conditional requests needs that read-back for every conditional create, not only for manifests. For branch refs it is cheap: each native name carries its creator's own ULID, so a conflict on that name can only be the creator's own earlier request.

    On step 6: #1017 now carries this result surface as its outcome model, so the two do not build parallel shapes:

    • one registry of codes;
    • an outcome object {execution, effects, action} on every error body;
    • outcome_unknown with an outcome.check of the branch, its head before the write and the attempt's transaction id;
    • storage_unavailable;
    • exit 2 for an unknown outcome, beside exit 4 for a failed precondition.

    #1017's read-write local servers wait for steps 2 and 5 here, so one storage error cannot take a graph away from a Studio session.

  3. ragnorc commented on Oct 18, 2026

    @ragnorc
    ContributorAuthor

    Reproduced on main 9575b044a11ad3b65074bf2fadfcb9dc45d06107. One storage mutation that returns a definite error poisons the process-local scope. Later mutations fail closed. An owned server then exits 2 and keeps the cluster lock. The triggering write in the reproduction did not publish.

    I read the two comments on this issue, both from the reporter: steps 6 and 7 and the three findings from #1017. No maintainer reproduction and no fix pull request are on the issue. This comment includes those findings.

    What failed

    The reported server was 0.14.0-edge+d883c49. That commit is an ancestor of this main. The scope, the retry override, and the fatal server path are unchanged between them.

    The reproduction ran on Linux x86_64 with rustc 1.97.1. The store was a local file object store behind the dst seam. It was not Railway and not Tigris.

    Setup:

    1. Create a scoped graph and insert one row. That write publishes.
    2. Arm a store fault. The next PutMode::Create of a detached *.manifest returns Generic with the text injected HTTP 500 on a conditional manifest put.
    3. Insert a second row.
    4. Insert a third row, and read the branch.

    The engine test fails on current main. These are the recorded values:

    • Fault path: .../_versions/d10739700813669506189.manifest, mode Create.
    • First error: storage: LanceError(IO): Generic issue-1067 error: injected HTTP 500 on a conditional manifest put, at lance-table-11.0.0/src/io/commit.rs:1592.
    • Storage class: Unknown. Completion evidence: none. Scope uncertain: true.
    • Head before and after: hb1.01M56HGDV8FVYD2XGJ7V3DVGK2.1.01M56HGDV8FVYD2XGJ7V3DVGK2. The write did not publish.
    • Next write: Generic owned storage error: a prior storage mutation has an uncertain outcome, after 10.315µs, at lance-io-11.0.0/src/object_writer.rs:625.
    • Read after the fault: succeeded.
    • Server classification: this error is OmniError::Storage with no BeforeEffect evidence, so the owned write is uncertain and serve_config exits 2.

    A storage test on the same main shows the same poison for two smaller cases. A delete that returns NotFound poisons the scope. A PutMode::Create of a fresh key that returns one HTTP 500 poisons the scope. AlreadyExists and Precondition still settle, which matches the current code.

    Differences from the live report:

    • The fault was injected. The run did not observe a Tigris 5xx, a response lost after the put landed, or the reported 6575 ms latency.
    • The Lance cleanup warning was not captured. The engine test has no log subscriber. The warning text is the same sentence the next mutation returns, and the cleanup delete is the path that prints it.
    • The process exit is the serving classification plus the exit at lib.rs:2998-3003. This run did not start omnigraph-server and did not call cluster force-unlock.

    Cause

    StorageIoScope is process-local settlement evidence. It is not a writer fence.

    Mutation::finish marks a mutation settled only on Ok, or on a conditional put or copy whose error is AlreadyExists or Precondition. Drop of any other unsettled mutation sets uncertain for the life of the process. The next begin returns a prior storage mutation has an uncertain outcome. Reads do not pass through that gate, so reads can continue on a poisoned handle until the process exits.

    The scoped client forces client_max_retries=0 and lance_aimd_max_retries=0 (scoped.rs:71-73). Deletes are unconditional (scoped.rs:219-239). A NotFound delete therefore poisons the owner.

    The server maps OmniError::Storage to an internal error with completion_uncertain, unless the evidence is BeforeEffect. The fatal path logs owned completion uncertain; containing process, then known owners drained after uncertain completion; terminating process, and calls std::process::exit(2) before release. release_settled also refuses when the scope is uncertain, with accepted storage work is uncertain; retaining cluster admission.

    The production warning is Lance's best-effort delete of {version}-{uuid}.txn after the commit has already failed (lance-11.0.0/src/io/commit.rs:137-165). That delete hits the poisoned scope. The first mutation error is not logged.

    This covers every scoped mutation error on local and S3 serving owners, direct CLI writers that use the scope, and bootstrap, except conditional AlreadyExists and Precondition. The class includes data-file puts, transaction-file puts, manifest Create puts that return 5xx, timeout, 403, 429, or Generic, deletes including NotFound, multipart errors, copy and rename errors, and the Lance cleanup delete.

    The behavior conflicts with these invariants:

    • One transport error stops every graph, and the triggering error is not logged.
    • A fresh-key or conditional failure that did not publish still poisons the owner. Pre-publication work must stay unreachable, and a retry must start from a fresh read.
    • A definite NotFound, permission error, or throttle is treated as an unknown outcome.

    The ban on silent retries explains why client retries are 0. It does not require process death for a request that cannot change shared state.

    Fix

    The durable fix is to classify each mutation by the key it writes.

    1. Fresh immutable keys are safe to retry and must not poison the scope. This class is data files, deletion files, index segments, {read_version}-{uuid}.txn, detached manifests with a new version id, and blob sidecars.
    2. A conditional publication resolves by sending the same conditional request again and reading the result back. Manifest commits already do this through verify_commit_outcome (lance-11.0.0/src/io/commit.rs:200). Every other conditional create needs that read-back before a retry can be treated as success. A conflict on a native branch ref is cheap to decide: the native name is {logical}.{ulid} (branch_names.rs), so a conflict on that exact name is the creator's own earlier request.
    3. An unconditional put or delete of a mutable key is the class that can corrupt state. Remove that class from publication. The finding in the second comment is right for Lance 11.0.0. Branches::replace_metadata (refs.rs:528-554) writes the ref with a plain put. Branches::delete (refs.rs:560-611) unlinks it with a plain delete. object_store 0.13.2 has no conditional delete. A late marker rewrite can recreate a ref that delete has already removed. Make branch retirement a zero-retry tombstone on that branch's own __manifest chain, as #1017 describes. The marker, the archive, and the unlink then follow that publication. A guard admits those writes only in that role.
    4. Drop the process-wide poison. A failure that did not publish returns a definite result for that operation. If the resolution read cannot finish, return outcome unknown for that operation and that branch only.
    5. Use one three-way result on HTTP, the CLI, and MCP, in the shape #1017 already records, so the two changes share one contract:
      • committed, with the graph commit
      • not committed, when the write was rejected, lost a conflict, or received a definite storage answer
      • outcome unknown, only when resolution cannot finish
        The result carries the branch, the head before the write, and the attempt's transaction id. HTTP carries a distinct body code. The CLI carries a distinct exit code beside 4 (EXIT_PRECONDITION_FAILED). MCP already has outcome_unknown, and it sets that flag for every uncertain storage error (mcp.rs:357-361). OmniError::Storage is uncertain unless the evidence is BeforeEffect, so MCP currently marks definite storage failures as unknown. The typed result corrects that. HTTP ErrorOutput has no outcome_unknown field. A dispatched CLI write that is not a typed refusal exits 1, with an internal Execution::Unknown, which a client reads as an ordinary failure.
    6. Keep process exit 2 for containment only, for as long as containment still exists. A client unknown-outcome code needs its own number. Sharing 2 would make a typed client result and a dead process look like the same event to an orchestrator.
    7. Add a circuit breaker for a sustained storage failure. Trip it on consecutive definite failures, or on unknown outcomes that cannot settle before the deadline. While it is open, writes fail fast with storage_unavailable and a retry-after hint, reads keep serving, and new work cannot add unresolved state. A probe resumes writes: put and delete of a fresh key, plus resolution of any pending conditional write. /readyz reports writes degraded per graph. Today a closed admission makes the whole server draining with HTTP 503.
    8. Log the first unsettled mutation: operation, key class, error kind, and status.
    9. Turn client retries back on only for idempotent classes. That is reads and fresh-key puts. #1068 is one reader that needs the read retry. A conditional create stays single-attempt until its read-back exists. object_store retries a conditional PUT on 5xx, 429, and 408. If the first attempt landed, the retry returns AlreadyExists. Lance put_ref_if_absent (refs.rs:147-163) then reports RefConflict and does not read the object.

    The local-root finding is also right, and it changes step 4. LocalFileSystem returns NotImplemented for PutMode::Update (object_store-0.13.2/src/local.rs:336). write_text_if_match compares content and then replaces the object through a staged write. Its own doc says this is not a cross-process barrier. An If-Match update of the lock is a conditional publication on S3 and Azure, and a different mechanism on local disk. The compare-and-swap that exists on every backend is a numbered object written only if absent. A lease can use lock generations in that form. Automatic takeover of a dead owner's lock stays with #1018. It is the largest new liability, and it should not land inside this fix.

    The smallest first change is narrower. Log the triggering mutation. Treat a definite storage answer as a definite result. Do not exit the process for a failure that did not publish. That removes the outage that lasted until cluster force-unlock. It leaves restart safety and the three-way wire result for the full design. Keep the fail-closed rule when a conditional publication's read-back cannot finish.

    Liability removed: one storage error no longer takes every graph offline until an operator unlocks the cluster. Liability kept: an unresolved conditional publication stays unknown and is not acknowledged. Liability deferred: taking over another process's lock.

    Lance

    Pinned Lance is crates.io 11.0.0, published 2026-08-30. Current stable is v13.0.0 (2026-10-07). Current pre-release is v14.0.0-beta.19 (2026-10-16). cleanup_transaction_file and the same warning exist in 11.0.0, in v13.0.0, and on Lance main. lance#6319 added that cleanup and closed lance#6125. The warning is a symptom. A Lance upgrade does not stop OmniGraph from poisoning the scope.

    verify_commit_outcome is already in 11.0.0. The object store guide documents client_max_retries default 3. OmniGraph sets that value to 0 on purpose.

    These open Lance changes touch commits or retries and do not remove this poison: lance#10033, lance#8774, lance#8773, lance#8801, lance#9763, lance#6544, lance#8363, lance#9691, lance#10008.

    Related work

    This issue stays open. Nothing else has the same defect with a broader reproduction.

    • #899 added the scope and the settled release. That is the change that introduced this stop.
    • #1017 is a documentation RFC. It adopts the outcome model and says read-write local servers wait for the first change here. It does not implement the scope change.
    • #1068 is a cold commit list cost bug. It asks for the read-retry part of step 3. It is a different defect.
    • #983 is the docs gap that a restart needs force-unlock.
    • #1018 is the cross-process lease and takeover RFC. Lock takeover belongs there.
    • No open pull request changes StorageIoScope or the fatal exit.

    Limits

    The failing tests were local proof and are not in the tree. The run did not reproduce a lost acknowledgement after a successful put, a live object-store 5xx, or the 48-minute operator wait. The server exit is deduced from the error class and the serve path, and the cleanup warning was not captured in a log subscriber.

  4. ragnorc commented on Oct 18, 2026

    @ragnorc
    ContributorAuthor

    Investigation notes, read against main at 9575b04 with the Lance 11.0.0 and object_store 0.13.2 sources.

    What happened, step by step

    1. The failing request was a graph table's detached commit, not the __manifest publish. The orphaned transaction file 12770445047132701458-…txn has a read version with the detached bit set (0xb139…, above 2^63), which only a table commit chained on its pin has. Lance writes the transaction file before the manifest, so the transaction put succeeded and the put-if-absent of _versions/d<random>.manifest is the request that returned an unsettled error.
    2. Since feat(server): release native admission after settled clean shutdown #899 the scoped Lance client runs with client_max_retries=0 and lance_aimd_max_retries=0 (crates/omnigraph-core/src/lance_access/scoped.rs). The option is per client, so served reads (GET, HEAD, LIST) are single-attempt as well. Lance's defaults, which 0.13 ran with, are 3 client retries plus 3 throttling retries. The scope refuses OBJECT_STORE_CLIENT_MAX_RETRIES, so an operator cannot restore them.
    3. Mutation::finish (crates/omnigraph-storage/src/scoped.rs) settles only an Ok result, or AlreadyExists / Precondition on a conditional request. A connection error that object_store itself classifies as "request not sent", or a 503 SlowDown, which S3 documents as a normal response to retry, poisons the one scope shared by every graph's Lance stores and the cluster control store.
    4. Lance had already resolved the outcome. verify_commit_outcome (lance-11.0.0 src/io/commit.rs) read the version back, found it absent, which is definitive for ConditionalPutCommitHandler, and tried to delete the orphaned transaction file. The poisoned scope refused that delete; the refusal is the WARN line. The original put error is logged by nothing.
    5. The engine returns OmniError::Storage with no completion evidence. commit_all (crates/omnigraph/src/exec/staging.rs) had not reached the publish call, so the graph had not moved, and the error does not say so.
    6. ApiError::internal (crates/omnigraph-server/src/lib.rs) sets completion_uncertain: true by construction. It is identical in v0.13.0, and operations.rs::typed_engine_uncertainty_controls_capacity_independently_of_status pins a bare I/O error on a write as uncertain. That flag, not the scope, fires OperationRuntime::fatal() and "owned completion uncertain; containing process".
    7. Containment closes admission for every graph, reads included, drains, and calls std::process::exit(2). The clean-release code sits below that line and would have refused anyway because the scope was uncertain. __cluster/lock.json has no lease or heartbeat. 0.13 retained the lock on every exit (docs: operations/server.md doesn't say a restart needs cluster force-unlock #983), so it would have needed the same unlock had the error ever surfaced there.

    Corrections to the proposal

    • Re-enabling retries alone lowers the frequency. Any 500 from an owned write still contains the process; that rule predates feat(server): release native admission after settled clean shutdown #899.
    • Served reads regressed as well (point 2). That deserves its own line in this issue.
    • The HTTP error body has no outcome field at all: ErrorOutput carries error, code and the typed details. The CLI maps a 500 to execution: unknown, effects: unknown, action: reconcile, so it does not invite a blind replay, but it cannot tell a definite failure from an unknown one either.
    • The unconditional mutable-key class is small: Branches::replace_metadata for the retirement marker and the ref unlink (crates/omnigraph-core/src/branch_control.rs), tag deletes (names carry commit ids, so delete-once), the lock delete on local and Azure roots, and the local write_text_if_match, which compares content and overwrites. Lance's version hint is written only on stores without lexically ordered listing, so not on S3.
    • DST cannot drive the server: GQT refuses omnigraph-server-dst and the DST crate has no server dependency. The Lance-realm injector has lose_ack and a random error rate but no targeted per-operation rule. "The write gets a definite outcome" can be an engine DST case; "the server keeps serving" needs a server integration test in the boot_settings style.
    • Step 3 can land independently of step 1. A retry re-sends the same bytes computed from the same read, so it adds no hazard to the mutable-key class beyond the late-duplicate hazard a lost acknowledgement already has. Lance's read-back covers a retried manifest put that lands, the publisher reads tags back, and branch creation on a ULID name should read back on RefConflict, which it does not today.
    • Step 1's classes are right, but enforcing them "by type" at the scoped store is not possible as written: it sees only a path and a PutMode, and Lance writes fresh data and transaction files with plain overwrite mode. The practical fence is to poison only unsettled overwrites and deletes outside Lance's immutable-file directories, with a source guard for OmniGraph's own call sites. Lance's table layout is a format contract, not identity inference.

    First change

    Three parts, one PR:

    1. Log the first unsettled mutation at the scope (operation, path, conditionality, error variant) before the scope turns uncertain.
    2. A third completion evidence between BeforeEffect and Uncertain: durable detached effects may exist, no publication was attempted, the graph did not move. The engine attaches it to every failure before the publish call. The publisher keeps Lance's CommitStatusUnknown in doubt and marks a storage error that Lance has read back as absent or foreign as definite.
    3. The server derives uncertainty from that evidence (Uncertain, manifest_publish_in_doubt, RecoveryRequired, panic, cancellation). The scope stops poisoning on a conditional request's error or on an unsettled write to a fresh key. A served write that fails definitely returns a 500 with the server still serving every graph, and a clean SIGTERM still releases the lock.

    Not in that change: client retries (step 3), the lease and automatic restart (step 4), the HTTP outcome registry (step 6) and the breaker (step 7). Until step 3 lands, an unsettled __manifest put still ends as in doubt and still contains the process, since nothing re-issues the conditional request; with retries restored, object_store's retry is that re-issue and Lance's read-back resolves it.

  5. ragnorc commented on Oct 19, 2026

    @ragnorc
    ContributorAuthor

    Another production containment on 0.14.0-edge+0a62bb7, which includes #1070. The deployment was down for about 16 hours because nothing restarts a contained owner.

    02:14:27.065 WARN omnigraph_storage::scoped: storage request failed on an immutable key; the physical scope remains settled
      op="put" path=…/graphs/personal.omni/__manifest/_transactions/5268-7cf53bd3-….txn conditional=false
      error=Generic S3 error: … <Code>SlowDown</Code><Message>Please reduce your request rate.</Message>
    02:14:27.166 ERROR omnigraph_server: owned completion uncertain; containing process
    02:14:27.166 ERROR tower_http::trace::on_failure: response failed … 500 Internal Server Error latency=12883 ms
    02:14:27.177 ERROR omnigraph_server: known owners drained after uncertain completion; terminating process
    
    • The request. A POST /graphs/personal/mutate on main. The bucket was quiet: three small mutates and a handful of reads in the preceding two minutes. The provider throttled a single PUT.
    • The storage layer was right. It classified the failure as settled: a fresh immutable key, not written.
    • The server still contained. The failed PUT is the __manifest publication's transaction file. fix(server): keep serving after a definite storage failure before publication #1070 attaches Unpublished evidence on the table-staging paths (table_store.rs) but not on the catalog publication path. With no evidence, the server's 500 default (completion_uncertain) applies and the process contains, although the manifest version cannot have been written: the transaction file precedes the put-if-absent of _versions/{n}.manifest.
    • A retry would have succeeded. SlowDown is a retryable 503 throttle, but retries are off (client_max_retries=0), so one throttle became a terminal containment.

    This supports steps 2 and 3 of the issue. Classify a publisher failure before the manifest put-if-absent as Unpublished, or resolve an opaque publisher error by reading back the branch head's next version. And retry idempotent and fresh-key requests on throttling.

  6. ragnorc commented on Oct 20, 2026

    @ragnorc
    ContributorAuthor

    PRs for steps 1–5.

    Cause of the 2026-10-19 outage (main 9e2425d2):

    • Lance writes __manifest/_transactions/*.txn before the manifest put-if-absent and returns its error with a bare ? (lance-11.0.0/src/io/commit.rs:1397). So the SlowDown meant no manifest request was ever sent.
    • The publisher still answered manifest_publish_in_doubt, because it treats absence as definite only for a failure "before any store request" (publisher.rs:1455).
    • The server then contained (lib.rs:2899, exit(2) at lib.rs:3048) and kept a lock with no lease.

    The full chain for both incidents is in the RFC's Motivation.

    PR Closes
    #1092 fix(catalog): a publication that never sent its manifest is unpublished This incident class, within today's contract. The commit handler re-issues an ambiguous manifest put until the store answers; evidence comes from a per-commit request record. Also part of step 5: contained writes log their class, never a path.
    #1093 draft RFC Storage outcomes per operation and restartable ownership The decisions steps 2–4 need: they change the owned-operations and engine-settlement contracts and the lock record.
    #1096 (draft, stacked, waits for the RFC) Steps 1, 2, 3 and 5. A registry guard covers reused-key requests; clients retry by request class; the scope tracks unsettled keys instead of one flag; an unsettled publication is pending on its branch (503 there, resolver re-issues); scoped outcomes no longer contain the process.
    #1097 (draft, stacked, waits for the RFC) Step 4. The lock record is version 3 with a lease and heartbeats. The owner fences after two missed beats. A restarted server takes over after a lease and a half of silence, on local and S3 roots.

    Still open:

  7. ragnorc commented on Oct 20, 2026

    @ragnorc
    ContributorAuthor

    Update to the PR summary above. All four PRs are now ready for review.

    Order: #1092 merges first. #1096 and #1097 follow once the RFC is accepted; #1067 closes with them.

    Still open after that, to become separate issues:

  8. ragnorc commented on Oct 21, 2026

    @ragnorc
    ContributorAuthor

    Closing: the fix set for this issue is on main.

    What landed

    • fix(server): keep serving after a definite storage failure before publication #1070 (0a62bb7b): a storage failure that is definite before publication keeps the server serving. The storage scope settles failed requests to fresh, immutable keys.
    • fix(catalog): a publication that never sent its manifest is unpublished #1092 (d4aa7fe9): every dataset's commit handler re-issues an ambiguous manifest request until the store answers it, and records what each commit's request established. A publication whose manifest request was never sent, or was refused outright, is Unpublished. Only that record lets Lance delete a transaction file.
    • rfc: storage outcomes per operation and restartable ownership #1093 (0c72cfab): the accepted RFC, Storage outcomes per operation and restartable ownership. It adds disposition notes to the owned server operations and engine settlement RFCs.
    • fix(server): scope an unknown storage outcome to its key and branch #1096 (626cdfb9): an unknown storage outcome is scoped to its key and branch.
      • A publication in doubt answers outcome_unknown. Its version key fences it: it lands before the branch's next publication or never.
      • The storage scope holds only the mutable keys whose outcome is unknown, instead of one process-wide flag.
      • Reads and fresh writes retry within Lance's bounds.
      • The server keeps serving. A panic, a lost write owner, RecoveryRequired, an opaque DataFusion failure and an uncertain activation still contain the process.
    • feat(cluster)!: take over a silent server's S3 lock through a lease #1097 (888223b7): a server's S3 cluster lock carries a lease, so a restarted server takes over a dead owner's lock after a lease and a half of silence, with no force-unlock.
      • The owner fences itself, and its fence cancels every storage request it has in flight.
      • Only the if-match on lock.json decides the next owner.
      • Local and Azure locks, and CLI writers' locks, keep the exact-ID unlock.

    To benefit from #1097, set the server's restart policy to restart on failure (Railway ON_FAILURE). On 2026-10-19 it was NEVER.

    Still open

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingneeds-triageAwaiting maintainer triage

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions