Repository navigation
Conversation
write_text_if_absent and write_text_if_match re-issue an ambiguous answer unchanged, up to four times, and return a refusal at once. After a conflict an earlier copy may explain, a refused re-issue or the last lost answer, they read the key back: equal bytes are this request's, a key that shows the precondition failed is a conflict, and one that still satisfies it is unresolved. An unscoped S3 or Azure client retries internally, so its first conflict is read back too. A local filesystem error is a refusal. The storage scope settles a request the store refused before applying it. An unsettled key records the precondition that left it there and admits its owner's re-issue; only the re-issue's success settles it, since S3 answers 409 (reported as already-exists) while the earlier copy may still be in flight. The adapter's read-back settles the rest.
|
Recommendation: revise. WhyA lost answer on a conditional control-object write failed on the first error. A create that did land was not recognized. The key could stay unsettled for the life of the server. What
HowA precondition failure, a create that finds the object, and an if-match on a missing object are conflicts. A local error is a refusal, because the local store finishes the put before it answers. Any other error is ambiguous. An unscoped S3 or Azure client retries inside ImpactA lost acknowledgement can now resolve for a writer-unique lock, a lease beat, and a state or resource object whose bytes are the effect. It cannot resolve exclusive ownership for a payload that two callers share. The prepared graph-create claim is that payload. BlockingEqual bytes are treated as "this call created the object" (
I ran a temporary case at this head and then removed it. An unscoped S3 adapter, a foreign object with the same bytes, and one put returned The same comparison applies after any ambiguous answer, including a scoped single-attempt client. A second initializer whose answer is lost sees the first initializer's identical claim and also gets The unprepared init claim carries a ULID and stays exclusive. Lock and bootstrap payloads carry a unique lock id. The if-match read-back uses the same byte test ( Two initializers can both observe success. That breaks the loud-integrity rule for the exclusive claim. The lost-ack failure this change fixes is the other class: a request that landed must not be reported as a hard failure, and a retry must stay bounded. The adapter is the right owner for that class. Byte equality does not hold exclusive ownership for a shared payload. Change one of these, and add a regression next to the
A lost acknowledgement of a unique payload can still return Optional
Checked, and left as soundThe lease keeper selects the beat future against the owner deadline ( A create cannot apply twice while DST injects faults above this adapter. The in-memory codec is not S3 or Azure and has no scope, so a conflict returns on the first answer and adds no GET. I did not run DST. The cost golden is unchanged on inspection. GQT cannot express this. The behavior is a storage answer, not a query row. The Open PRs 1175, 1168, 1171, 1178, and 818 do not add another control-object re-issue. PR 1177 (served recovery) should call this resolver when it must settle the same class of lost answer. A third copy would drift. A future Lance release can drop this loop only when the store reports whether a conditional put was sent. Tradeoff and liabilityCorrectness of the lost-ack case comes first. The copied budget is a smaller diff, and it can drift from the manifest resolver. One source of truth should own the count, the pause, and the classification. The The change removes a fail-closed lost acknowledgement and a key that stayed unsettled. It adds a second policy and a success result that two initializers can share. The diff is about +945/−71 lines in 7 files. The line count does not show lower liability. ValidationHead: Merge base, isolated worktree, temporary test, then removed:
FAILED. 0 passed, 1 failed, 80 filtered out. The applied create whose answer was a generic "connection reset" returned Head, the PR tests plus one temporary case, then removed:
ok. 24 passed, 0 failed, 63 filtered out, 0.02s. The PR lost-answer tests pass, so that applied create now returns Not run here: DST, GQT, Inline comment,
A temporary case at this head showed that: foreign equal bytes, S3 codec, no scope, one put, result Keep Inline comment, This treats permission and authentication errors as refused before the put could apply. The manifest classifier treats the same errors as ambiguous, because a client that retries can apply on an earlier attempt and then surface 403 or 401. That matches a scoped client ( |
…h-create claim The adapter reads a lost create's answer back by the stored bytes, so equal bytes must identify the writer. A prepared graph-create claim was a digest of the token, the same bytes for every apply of it, so two applies could both read the claim as their own. Each apply's claim now carries a fresh attempt id (version 3). Settlement recognizes the token's claim by digest, graph commit id and root, and still accepts the version-2 claim earlier releases wrote. omnigraph_storage::conditional owns the re-issue count, the pauses, the failure classification and which answer ends a request, for control-object writes and the manifest commit handler alike. An authorization failure is a refusal only on a single-attempt client; an unscoped S3 or Azure client repeats requests, so its 403 or 401 is re-issued and read back. A manifest re-issue that meets a held version is now unresolved rather than held: on S3 the conflict can answer the earlier copy still in flight, so a missing version on Lance's read-back must not let it delete the transaction file that copy names.
|
Recommendation: ready. WhyA lost answer on a conditional control-object write failed on the first error. The review of WhatCommit How
A single-attempt client treats the first conflict as final. It also treats an authorization failure as a refusal. A client that retries inside
A manifest re-issue that then meets a conflict is recorded as unresolved. Lance 13 ImpactA lost acknowledgement of a writer-unique payload can resolve as this call. A second apply of one prepared token stays excluded, including when the client reads a create conflict back by the stored bytes. A manifest commit whose re-issue meets a conflict stays unknown until a read shows the version. Prior findingsBlocking. Addressed. The adapter still returns Optional 1. Addressed. One policy owns the count, the pauses, and the failure classes. The pauses are 0.2 s, 0.4 s, 0.8 s, and 1.6 s. Authorization failures are refusals only on a single-attempt client. The manifest handler cannot see whether Lance's client retries, so it keeps the repeating-client rule. That was already the manifest rule. Optional 2. Addressed on the user page. A direct command's client retries up to Optional 3. Addressed. New findingsNo blocking findings. No optional findings. Tradeoff and liabilityThe lost-ack fix has to return success when this call's bytes are already stored. A rule that every existing object is The change removes a second copy of the re-issue budget and a claim that every apply of one token shared. It adds one policy module, one attempt-id field, and the parser settlement uses. Five more conditional writes should call this policy. A new exclusive create must put a fresh id in its bytes. The trait text on Two differences stay, and each has an owner. A local control-object error is a refusal, because that put finishes before it answers. A local manifest error stays ambiguous, because Lance's Windows rename can create the manifest and then fail. The manifest handler also settles the first conflict, because Lance reads the version back. Other open workNo other open pull request adds a second re-issue loop. PR 1177 creates a maintenance journal with ValidationHead Head, isolated checkout:
ok. 26 passed, 0 failed, 64 filtered out, 0.02s.
ok. 5 passed, 0 failed, 80 filtered out, 4.63s. A lost manifest answer followed by a conflict stays unresolved.
ok. 1 passed, 0 failed, 555 filtered out, 0.01s. A second apply of one token is
exit 0. Merge base, isolated checkout, temporary test, then removed:
FAILED. 0 passed, 1 failed, 80 filtered out. The applied create whose answer was Previous reviewed head, isolated checkout, temporary test, then removed:
FAILED. 0 passed, 1 failed, 555 filtered out. Two GQT cannot express this. The behavior is a storage answer and an init claim, not a query row. The Not run here: DST, the workspace suite, Checked, and left as soundThe lease beat treats A clean conflict on a scoped client returns |
|
Thanks. All three points are addressed in 94691fa. Blocking: equal bytes from a shared payload. Confirmed. The prepared claim was a digest of the token, so every apply of one token wrote the same bytes. I took the second option: each apply's claim now names the apply. Version 3 carries the token digest, the graph commit id, the root and a fresh I checked every other Regression: Optional 1: one resolver. New
The single-attempt exception is explicit: an authorization failure is a refusal only for Moving the manifest loop surfaced the trap this PR fixed for control objects. A re-issue that met a held version was recorded Optional 2: the user page. Optional 3: store facts.
The Azurite 409 and real Azure 412 difference ends in the same read-back, as you note. Garage-style stores that ignore PR #1177 should resolve through Validation at 94691fa:
The workspace suite runs in CI. |
|
Recommendation: ready. WhyA lost answer on a conditional control-object write failed on the first error. The review of Commit What
How
A single-attempt client treats the first conflict as final. It also treats an authorization failure as a refusal. A client that retries inside
A manifest re-issue that then meets a conflict is recorded as unresolved. Lance 13 ImpactA lost acknowledgement of a writer-unique payload can resolve as this call. A second apply of one prepared token stays excluded, including when the client reads a create conflict back by the stored bytes. The guard fails if a second production conditional put appears in the storage adapter without a new registration. Runtime behavior is the same as at BlockingNo blocking findings. OptionalNo optional findings. Tradeoff and liabilityThe lost-ack fix has to return success when this call's bytes are already stored. A rule that every existing object is The change removes a lost acknowledgement reported as a hard failure, a key that stayed unsettled, a claim that every apply of one token shared, and a second copy of the re-issue budget. It also stops a manifest re-issue's conflict from letting Lance delete the transaction file that an earlier copy may still name. It adds one policy module, one attempt-id field, and the parser settlement uses. The guard update is the existing inventory, pointed at the one primitive. Five more conditional writes should call this policy. A new exclusive create must put a fresh id in its bytes. A new Two differences stay, and each has an owner. A local control-object error is a refusal, because that put finishes before it answers when fsync is off. A local manifest error stays ambiguous, because Lance's Windows rename can create the manifest and then fail. The manifest handler also settles the first conflict, because Lance reads that version back. A direct writer's retrying Lance client can repeat that copy itself. The handler cannot see those copies. Other open workNo other open pull request adds a second re-issue loop. PR 1177 adds served maintenance recovery and does not add its own conditional-put policy. It also edits ValidationHead GQT cannot express this. The behavior is a storage answer and an init claim, not a query row. The Head, isolated checkout,
ok.
ok. 1 passed, 0 failed, 35 filtered out, 1.13s. Merge base, isolated checkout, temporary test, then removed:
FAILED. 0 passed, 1 failed, 80 filtered out. The create applied. The object held Exact-commit CI on Checked, and left as soundThe lease beat treats A clean conflict on a scoped client returns A re-issue's 409 does not clear an unsettled key. |
Record the intent nonce and resolve by commit_id_answers; keep an in-doubt publication non-terminal; release the writer gates before the artifact scan; add a resume position, resource ceilings and per-table failure isolation; require maintain on every route; depend on ModernRelay#1175 and ModernRelay#1176.
* rfc: remote optimize through the serving owner A bounded, manually requested optimize of a served graph's main branch, executed by the process that serves and publishes it: today's gate shape and one publication, a gate-hold bound with an exact outcome per table, a durable operation record under a caller-chosen id before any effect, lookup by that id after a lost response, and restart resolution from the graph's history. Replaces the closed served-maintenance proposal in the form its review asked for. * rfc: link the remote optimize RFC to its discussion * rfc: revise remote optimize after review Record the intent nonce and resolve by commit_id_answers; keep an in-doubt publication non-terminal; release the writer gates before the artifact scan; add a resume position, resource ceilings and per-table failure isolation; require maintain on every route; depend on ModernRelay#1175 and ModernRelay#1176. * rfc: settle remote optimize resume, ceilings and resolution after second review * rfc: settle remote optimize resolution reads and the artifact bound after third review * rfc: resolve through the buffered commit lookup and count both offset sides after fourth review * rfc: state the history cost of a released commit and reuse the collector's path resolution after fifth review
Implements the control-object part of §2 of the accepted RFC, storage outcomes and restartable ownership. Conditional control-object writes now resolve an ambiguous answer by re-issue and read-back, as manifest commits already do (#1092). A definite refusal no longer leaves its key unsettled.
Storage adapter
write_text_if_absentandwrite_text_if_matchused to return on the first error. They now resolve each answer throughomnigraph_storage::conditional. That module is the one policy for a conditional request whose answer can be lost, and the manifest commit handler uses it too. It owns the re-issue count, the pauses, the failure classification and which answer ends a request. Each answer is classified:CONDITIONAL_REQUEST_REISSUES(four) times, with pauses of 0.2 s doubling to 1.6 s. A refused re-issue is re-issued too.The key is read back after either of these:
Reading the key back resolves the write:
state.jsonand content-addressed resources are the effect. The trait text states the requirement. The prepared graph-create claim did not meet it (see below).An unscoped S3 or Azure client retries internally, so its first conflict is read back too: it may be answering its own earlier copy.
S3 answers 409 while another conditional write to the key is in flight, and
object_storereports it as "already exists" (client/retry.rs). That earlier write can be our own copy, so:Ok(false);On a local root an error is a refusal.
LocalFileSystemfinishes a put before it answers, and without fsync (its default) nothing follows the publishing link or rename. Local behaviour is therefore unchanged.Storage scope
Nonefor an unconditional request). A request with the same precondition, the owner's re-issue, is admitted; every other request stays refused.Prepared graph-create claim
A prepared claim was a digest of the token, so every apply of one token wrote the same bytes, and the read-back could tell two initializers that both held the claim. Each apply's claim now names the apply. Version 3 carries the token digest, graph commit id, root and a fresh
attempt_id.settle_prepared_graph_create_after_quiescencerecognizes the token's claim by the first three, whichever apply wrote it, and still accepts the exact version-2 claim earlier releases wrote.Manifest commits
The manifest handler (
lance_clone) now runs on the shared policy, and its classification is unchanged: it cannot tell Lance's provider from a scope's client, so it takes the client to repeat requests. One outcome changes. A re-issue that meets a held version is recordedUnresolved, notHeld. On S3 that 409 can answer the earlier copy still in flight, andHeldlet Lance treat a missing version as definitive and delete the transaction file that copy names. The first copy's conflict staysHeld.Callers
The beneficiaries are the cluster lock and release marker, the init claim and create-if-absent probe, deployment resources,
state.json, and lease beats and claims.The lease keeper fences on its own timer, so a beat that re-issues inside the adapter cannot postpone the fence. The 409 handling also stops a lease beat or claim from counting a copy still in flight as present.
Tests
scoped::tests::a_conditional_reissue_is_admitted_issue_1067scoped::tests::a_refused_request_leaves_its_key_settled_issue_1067tests::write_text_if_absent_resolves_lost_answers_issue_1067tests::write_text_if_match_resolves_lost_answers_issue_1067tests::repeating_client_reads_back_its_own_copy_issue_1067tests::scoped_conditional_writes_settle_by_reissue_issue_1067tests::the_client_decides_what_a_failed_copy_says_issue_1067: a local error is not re-issued; a repeating client's permission error is re-issued, a scope's is refused; an unscoped first 409 over an absent object stays unresolved; and equal bytes read back as this writer's.conditional::tests: pauses, the single-attempt rule and which answers end a request.lance_clone::tests::manifest_request_is_re_issued_until_the_store_answers_issue_1067: a first conflict is held, and a re-issue's conflict is unresolved.db::omnigraph::tests::one_prepared_token_has_one_init_claim_owner_issue_1067: two claims of one token through an adapter that reads a create's conflict back by bytes; the second is refused. Settlement recognizes any apply's claim and the legacy version-2 claim, and refuses another token's.The adapter tests use a scripted store that applies-then-loses, loses before applying, refuses, answers 409, lets another writer in first, or repeats a put. They run on paused tokio time.
Docs
docs/user/operations/storage-failures.md: Retries and Logs.docs/dev/control-plane.md: the scope and adapter rules, and the shared policy.docs/dev/lance.md: the manifest re-issue's conflict, and the first-copy limit.docs/dev/recovery.md: the prepared claim's attempt id.control-object-writes-reissue-lost-answers.fixed.md.Verification
At 2aaae98: the six-package suite (engine, server, cluster, core, storage, catalog, with the failpoint features) passed every binary, and
cargo fmt --all --check, both workspace clippy graphs,check-docs.pyandtyposwere all clean.At 94691fa, after the review:
issue_1067,conditional::,manifest_requestand init-race tests across storage, core, catalog and engine (43 tests);-D warnings. The local toolchain is 1.98 and the repository pins 1.97.1. Two 1.98-only lints, on code this PR does not touch, were allowed locally; CI's 1.97.1 gate covers them.cargo fmt,check-docs.pyandtypos.CI runs the workspace suite.
ObjectStorageAdapter::in_memory(), whose only errors are conflicts, and it injects faults above the adapter, so neither its paths nor the cost golden change.