Repository navigation
fix(server): keep serving after a definite storage failure before publication - #1070
Conversation
…nRelay#1066) * query: refuse full-text calls without a full-text index (T27, FullTextIndexRequired) A full-text call (`search`, `fuzzy`, `match_text`, `bm25`) reads its property's full-text index, whose analyzer folds case and stems; without one Lance scans with a bare, case-sensitive tokenizer, so the same call answered differently on an indexed and an unindexed property. - T27 at type checking: a full-text call on a String node property with no one-column full-text `@index`/`@key` is refused, from the schema alone. `PropType::index_kind` moves to the compiler and index maintenance uses it, so the type checker and the index builder cannot drift. An enum's fix points at `=`; a non-String field keeps its operand error. - FullTextIndexRequired at planning (HTTP 409, `full_text_index_required` in the error body): the planner reads the coverage of every property a full-text call names, filters included, and refuses `absent`. An untrained segment, which every write now declares, is `partial` and answers; a table with no fragment, or a type the snapshot does not hold, counts as covered. Acceptance refuses a plan that records no coverage, or `absent`, for a call, and replay re-derives every recorded coverage from the pinned dataset. Regressions: cases/v2/issue_747_full_text_calls_need_a_full_text_index.gqt and search.rs::a_full_text_index_with_no_segment_refuses_the_call_issue_747, both red with their check removed. * docs: rescope RFC 0047 step 3 and document the full-text index requirement RFC 0047 records step 3 as refusing only an index with no segment, now that every write declares a declared column's analyzer, with a decision-log entry, and its rollout table records steps 2 and 2a as merged. The search guide, troubleshooting page and execution guide describe T27 and the 409; the release notes call T27 breaking and tell operators to validate stored queries before upgrading.
…lication A served write that fails after staging its detached table effects and before its publication left the process: the engine returned a bare storage error, `ApiError::internal` treats every 500 as an unknown outcome, and the owned-write runtime contained the process with the cluster lock held. Add `CompletionEvidence::Unpublished` and attach it in the mutation, load, Blob and merge writers from the first staged file to the publisher call, and in the reads before `publish_prepared`. The server reads it as definite, like `BeforeEffect`; a write error without evidence still fails closed. Classify every request the storage scope accounts for by the key it names: Lance's write-once table files and put-if-absent manifest versions settle with their definite error, since a late or duplicate landing is unreachable or refused; refs, the version hint, control objects and unknown keys keep the sticky rule. Log the first request whose outcome is unknown with its key and error, and every definite failure of an immutable key.
* feat(query): the ranked binding roots its component A nearest(), bm25() or rrf() order on a binding a traversal reaches was refused at planning unless that binding was declared first, and a ranked binding no declaration introduced could not be ranked at all. Lowering now roots the top-level component holding the bindings the leading order key ranks at that binding; a ranked binding only a traversal introduces gets a NodeScan of its checked type, and every other binding of the component is reached by a forward or reverse expand. An rrf() whose arms rank two bindings one traversal connects is the type error T59. The planner codes P001 and P005 are withdrawn; the planner keeps both checks as internal errors. * docs(search): rank a traversal-reached binding
|
This change lets the server stay in service after a storage failure that happens before publication. The failure is definite. The graph did not move. Admission stays open. A clean shutdown can release the cluster lock.
The incident in #1067 is a lost acknowledgement on a detached table manifest. That object is small. Lance writes it with one PUT. This change does settle that request, and the server can keep serving. Two holes remain. Both break the settlement rule in this pull request. Recommendation: request changes. Blocking
This pull request says a read-back cannot exclude a late landing of an unsettled Keep the scope uncertain for a manifest put until Lance can no longer delete the transaction file. A late manifest is not unreclaimed garbage after that delete.
I added a temporary test on this head. A lost complete of Optional
The current Lance blob guide stores managed payloads at Schema apply, optimize ( The index rule matches a path segment of length 36. It does not parse a UUID. Another spelling stays contained. Every immutable failure logs TradeoffThe change removes one outage. A definite failure of a small single-put write-once object, before publication, no longer stops the server or holds the lock. It adds two liabilities. The scope no longer stops Lance from deleting a transaction file after an unacknowledged manifest put. A multipart complete failure leaves the scope uncertain while the HTTP error says the failure is definite. Each writer also attaches the evidence itself. The next writer must repeat that call. Five more changes of this kind add more key spellings and more writer calls. The cleanup rule and the multipart lifetime stay outside the classifier. Open pull request #1017 is the serving RFC. It should consume this one completion fact. A second outcome type would split the contract. ValidationGQT cannot express scope poison, completion evidence, or process exit. Rows, counts, and query errors do not show them. The owner is the storage scope test. Before, commit After, commit I did not run the boot journey, the server integration test, or a Tigris lost-acknowledgement drill. I did not use CI as proof. I read Lance 11.0.0 Inline comment on Inline comment on |
…acknowledged write After a manifest put the store did not acknowledge, Lance reads the version back once, takes one "not found" as definitive and deletes the attempt's transaction file; a manifest landing later would name a missing file. The scope now refuses every delete under that dataset's `_transactions/` for its own lifetime, a settled refusal Lance logs and moves past, and the collector reclaims the orphan from another process. A conditional conflict holds nothing. An abandoned or cancelled request on an immutable key settles instead of poisoning, which covers a multipart upload's lifetime: a lost `complete`, a lost `abort` or a dropped upload of a data file leaves the scope settled. A delete answered "not found" is definite on any key. `_bt/` tree objects and `_blobs/` payloads are immutable, and the index rule parses a UUID. The table store marks an opaque error of a staged write or a detached commit `Unpublished` at the source, so a writer that stages through it gets the mark without a call of its own.
|
Addressed in Both blocking findings are right, and both are now handled inside the scope rather than by poisoning it. Pushed as a second commit on this branch. 1. Transaction file after an unacknowledged manifest put. The scope now records, for a put of 2. Multipart Optional points.
Not changed: the |
|
Recommendation: approve. This change lets the server stay in service after a storage failure that happens before publication. The failure is definite. The graph did not move. Head What this commit doesAfter a put of An abandoned or cancelled request on an immutable key settles in Earlier findingsBlocking 1. A lost manifest acknowledgement deletes the transaction file. Fixed.
A conditional conflict ( The owner test is Blocking 2. A lost multipart complete poisons the scope. Fixed.
Lance uses multipart above 5 MiB ( The owner test is Optional findings from the previous review
The index rule parses a hyphenated 8-4-4-4-12 hex UUID. Fixed. A delete answered "not found" settles on any key and is not logged. Fixed.
Optional
The version hint ( Class of casesOmniGraph accepts local, S3, and Azure URIs. Unknown schemes are refused. GCS,
The hold lasts until the process exits. A long-running server that sees one lost manifest acknowledgement refuses every TradeoffThe change removes two liabilities. A definite failure of a small write-once object, before publication, no longer stops the server or holds the cluster lock. A late manifest no longer names a missing transaction file. It adds one Lance-shaped rule in the scope. The scope assumes that Lance deletes the transaction file only after the manifest put returns, and only after a definitive not-found. A future Lance that deletes a different path, deletes before the put returns, or stops treating not-found as definitive would make this hold wrong. The rule sits in the layer that admits deletes. That is the right layer. Five more changes of this kind would add more path spellings and more hold rules beside
An unacknowledged linear
ValidationGQT cannot express scope poison, completion evidence, or process exit. Rows, counts, and query errors do not show them. The owner is the storage scope test. I did not add a GQT case. Before, commit
Result: FAILED. After, commit The scope test calls I did not run the boot journey, the server integration test, or a live lost-acknowledgement drill. CI on this exact head passed, including Test Workspace, Clippy, Format, RustFS S3, Azurite, and GQT ordinary. That is CI evidence. It is not the before/after proof above. Inline comments
Optional. The hold starts only when
OmniGraph serves Linux Local
Optional. This refusal is a settled
The hold is the whole directory, for the life of the process. The same process cannot reclaim any transaction file of that dataset until it exits. Another process can. This is safe. It is broader than the one file a late manifest names. |
|
Recommendation: approve. This change lets the server stay in service after a storage failure that happens before publication. The failure is definite. The graph did not move. Head What this delta does
A request settles only for Lance 11 fresh files the classifier names:
Earlier findingsBlocking findings from the review of
|
What & why
A served write could fail before graph publication, poison its storage scope, and shut down the entire server. This fixes the containment part of #1067: a definitely unpublished write returns its failure while admission stays open. Genuinely uncertain publication still contains the process and retains its lock.
The engine records
CompletionEvidence::Unpublishedat staged-write and detached-commit sites and across the mutation, load, Blob, and merge envelopes. Typed refusals and existing uncertainty retain their meaning. The HTTP boundary reads this evidence independently of the 500 status; the publisher still owns read-back and publication uncertainty.Storage accounting now distinguishes fresh immutable Lance 11 files from mutable keys and conditional replacements. The classification is anchored to known path tails. Rename accounts for source deletion, and recursive deletion stays conservative. An unacknowledged manifest put, copy, rename, or multipart completion sets one bounded, sticky transaction-retention flag. Successful abort cannot erase an earlier unknown completion. A replacement scope may reclaim retained files only through normal collector authority, reachability, and age checks.
The change preserves the original PR's commits and adds the cancellation, source-delete, conditional-update, prefix-classification, and multipart regressions found during review. It does not add retries, change leases, or broaden recovery. Other opaque schema/maintenance preparation failures remain conservative.
Backing issue / RFC
Addresses the definite pre-publication failure path in #1067. Other recovery work in that issue remains outside this change.
Checklist
Local verification
The original real-server regression was reproduced on main: the first storage failure returned 500 and the next connection was refused. Eleven added scoped regressions also failed against the original PR before correction.
cargo test -p omnigraph-storage --locked scoped— 23 passed, no ignored tests.cargo test -p omnigraph-engine -p omnigraph-server -p omnigraph-cluster \ -p omnigraph-core -p omnigraph-storage \ --features omnigraph-engine/failpoints,omnigraph-cluster/failpoints --locked--lib error::tests— 19 core tests passed.--lib operations::tests— 14 server tests passed.--test boot_settings owned_shutdown— 15 passed, one subprocess helper ignored. The real failure journey now returns 500, accepts the next write, serves reads, and releases its lock on clean shutdown; existing uncertain-outcome containment journeys still pass.--test lance_surface_guards --test writes --test detached_commit_matrix— 68, 63, and 29 passed respectively. Four existing entries remain ignored: two subprocess helpers, an unsupported-on-Lance11 guard, and a heavy 8192-statement reproduction.cargo fmt --all --check, documentation checker (338 files), AGENTS/index, and whitespace checks — passed.Full workspace tests and Clippy are left to this updated PR's CI; the local results above are focused qualification.
The three updated branches (#1070, #1071, #1036) also merge cleanly. Their combined code passed five file/scoped-file/S3/Azure collector owners without skips, the real server failure journey, range unit owners, both setup/dead-staging DST pins with replay, and all four range-filter GQT cases. Combined documentation (341 files), formatting, links, required CI names, and merge-group checks passed. Local validation is retained on
codex/release-014-integrated-validation; the final additional merge changes only the clarified testing-guide wording.