LEP: Unify store get/query/exist interfaces and a define a contract for the store trait - #164
Closed
mjansson wants to merge 6 commits into
Closed
LEP: Unify store get/query/exist interfaces and a define a contract for the store trait#164mjansson wants to merge 6 commits into
mjansson wants to merge 6 commits into
Conversation
mjansson
requested review from
ayyyee,
matt-hoffman-epic,
peter-lockhart-pub and
ragnarula
August 9, 2026 19:32
`ImmutableStore` had four ways to ask whether a store held an address, each written eighteen times, and they did not agree. The AWS store resolved the same address two ways and only one of them knew what obliteration was; the replicas sent one request under two names; the remote store downloaded a payload to answer a question about metadata; the composite carried four copies of one fan-out. Nothing said what any of it meant, so each implementation decided for itself, which is why they differed. They become one batched `query` reporting the best match a store establishes, beside the metadata read that was already there. `StoreMatch` stops being an input: a caller cannot ask at a level, because the level is the answer. Scope stops being a process-wide flag threaded through every call and becomes two properties of the store, because serving and reporting are not the same question. A read hands over bytes and no protocol carries the level alongside them, so whatever a store across a trust boundary serves is read as an association of the caller's own - a store that isolates partitions therefore serves the exact association and nothing else, as a server always has, while a client's disk store still serves any hash it holds. Existence reaches further, to a partition match, because that is a level to act on with a copy rather than bytes owed here. Both reads answer with one `StoreGetData` carrying the fragment, the level and an optional payload, so a caller that loads content learns whether the association was its own. `lore_storage_get_metadata` takes any match the store made rather than only a full one: it asks what a payload is, and a weaker level names the same bytes under the same hash, so the description is the one the round trip it used to make would have fetched. Every match also names the partition it was found in, preferring the one asked about. That is what a copy needs: a client holding content already stored elsewhere can name a source rather than transfer bytes the repository has. Nothing on either wire grows a field, because a hash match never crosses a trust boundary and both protocols are partition-bounded. The contract is written into the trait and enforced by a battery every store runs: never over-report, may under-report, obliterated never matches, reads do not under-serve and agree with each other, and a match names where it was found. It holds each store to the scopes it declares rather than to a fixed expectation, and stores register the clauses they fail, so a fix that is not delisted fails the build - which is how four defects surfaced here: a local store claiming durability for content it had never seen, an AWS batch losing every duplicate address but the last, a remote read reporting a full match for an association the server never had, and `get_metadata` describing content whose payload had been obliterated, because `lookup` searches addresses and never reads flags so every path answering a caller has to refuse a tombstone for itself. Every store passes every clause now and none declares a violation. The AWS store answers existence from associations alone, paying for it in obliteration instead: the reference is deleted before the hash is marked, because the reference is the obligation and the mark is bookkeeping for the reclamation that follows. While the legacy metadata table is configured every association is confirmed against the state table and, where no state row exists, against the metadata row - all batched, because that path runs once per fragment during a push. Its reads cost one lookup, since a store reading only exact associations never runs the query that would reach a sibling context. On the replication wire the metadata operation gets a request of its own rather than borrowing the existence payload and rejecting all but one address, and is labelled and traced as itself rather than as a query. Retired level bytes stay reserved and retired commands stay unassigned, so no layout moves and a peer that still sends one is rejected rather than misread. Signed-off-by: Mattias Jansson <mjansson@gmail.com>
mjansson
force-pushed
the
mjansson/query-exist-metadata
branch
from
August 9, 2026 20:43
ab96966 to
ae0822f
Compare
The gRPC storage transport multiplexes every read of a session onto one bidirectional stream and reports a missing fragment as that stream's terminal status. Tonic ends the RPC at the first `Err` item, so one caller's miss finishes the stream for every address on it. The client treats the error as a per-address failure and keeps looping, falls out when the stream ends, and leaves its `mpsc::Sender` cached in a `OnceCell` that is never invalidated - the receiver still lives in the h2 request body, so the sender still reports open. Every later read on that session is sent into nothing and waits forever, with no timeout. That is what wedged `ObliterationLeavesSiblingsReadable`: reading the obliterated reference is the miss, and the surviving reference is the read that never returns. The check was parked behind `over_wire`, which named the wrong thing - a wire is not the problem, this transport is. Carry the battery over QUIC instead, which frames each command separately and has no such coupling. A `lore://` connection resolves its environment over gRPC, so both harnesses share one address: gRPC on TCP, storage on UDP. The gRPC run keeps its coverage and declares the defect through `miss_poisons_session`, which skips only the checks that read after a miss, so the parked case names its cause and points at the fix. Fixing the proto so a miss is in-band rather than terminal is left for its own change. Signed-off-by: Mattias Jansson <mjansson@gmail.com>
…s not Clause 1 claimed a full match means the payload is retrievable. It does not, and the local store is the reason: it takes a `put` with no payload and keeps the representation alone, which is what a composite configured `with_local_metadata_only` writes it. Such an entry reports a full match because the association is real, describes itself because the fragment is there, and fails `get` - and that failure is load-bearing, being how the layer above knows to fetch the payload from upstream. So the level says the association exists and nothing more; whether the bytes can be served from here is `stored_local` and `stored_durable`, which is what the write path already gates on. Correct the clause where it is stated - the trait, the battery's module docs and the LEP - rather than the store that was right all along. Pin the behaviour with a battery case, because the failing `get` looks like a defect and reporting `MatchNone` instead would look like a fix while silently breaking the upstream fallback. It is opt-in through a capability, since only the local store accepts a payload-less put: the durable store refuses one outright and the replicas take no puts at all. Signed-off-by: Mattias Jansson <mjansson@gmail.com>
Obliterating a fragment written before the state table existed returns early with no state row to act on, reporting success while leaving the association in place - and the legacy fallback on the resolution path goes on reporting that association as a full match. Clause 3 therefore does not hold for legacy-era content while that table is configured. The early return predates this proposal; what is new is the fallback that keeps such content matchable. Accepted for now: it is bounded by the legacy table's lifetime and retires with it. Write it into the LEP's risks rather than leaving it implicit, and note at the early return what it leaves behind, since the code there reads as though there were nothing to do. Signed-off-by: Mattias Jansson <mjansson@gmail.com>
The note on the battery test described `exist` and `query` as two resolutions that only one of which knows about obliteration. `exist` no longer exists, so the caveat documented a divergence this work removed. Replace it with the limitation that is real: the store under test is built without the legacy metadata table, so the battery never reaches the fallback resolution that table turns on, nor the obliteration gap that comes with it. Those have tests of their own. Also record why the legacy-era gap is accepted - it reaches only deployments still holding content from before the state table, and the recommended path for those is migrating to the new representation. Signed-off-by: Mattias Jansson <mjansson@gmail.com>
`lore file dump` printed the tombstone's flags for an obliterated address, because the local store's resolution returned the entry's flags without consulting them. Clause 3 stops that: the read reports no match, and the dump prints zeroes. Describing obliterated content is precisely what the clause forbids, so this is the point rather than a casualty - but it is a printed value changing, so say so under Compatibility and move the smoke test onto the property that now holds. The test asserted the flag; it now asserts the address does not resolve and that the flag is not exposed. Three further corrections where the document claimed more than the code does. Only `stored_durable` is merged across responders - `stored_local` stays as the local store left it, since a replica's disk is not ours. The caller-owned slice removes the allocation for single-address callers, the majority, but a batching caller still allocates one and neither of the two in the tree reuses it across chunks, so the slice buys them the option rather than the saving. And the AWS resolution path carried a span name and a doc link naming a method called `resolve`, which this branch renamed to `query` before either was shipped. Signed-off-by: Mattias Jansson <mjansson@gmail.com>
mjansson
added
ready-to-import
epic-lore-bot
Bot
added
imported
|
Imported as Lore CR-315. |
ayyyee
approved these changes
Aug 10, 2026
epic-lore-bot Bot
pushed a commit
that referenced
this pull request
Aug 11, 2026
…or the store trait ## Problem `ImmutableStore` had four ways to ask whether a store held an address, each written many times for the store types, and they did not agree. The AWS store resolved the same address two ways and only one of them knew what obliteration was; the replicas sent one request under two names; the remote store downloaded a payload to answer a question about metadata; the composite carried four copies of one fan-out. Nothing said what any of it meant, so each implementation decided for itself, which is why they differed. ## Proposal Query is unified with exist and becomes a batch interface, responding the best match level found in the store, with a clear contract what the store must uphold in terms of data partitioning. Both get and metadata reads lose the match level parameter as it was mostly unused and better served by an answer of the best level found - with one `StoreGetData` carrying the fragment, the level and an optional payload, so a caller that loads content learns whether the association was its own. The store contract is written into the trait and enforced by a battery every store runs: never over-report, may under-report, obliterated never matches, reads do not under-serve and agree with each other, and a match names where it was found. ## Validation This change also carries the implementation to show what it looks like in practice - the reduced and unified store interface along with the fixes the contract enforcement caught. ``` Imported-PR: #164 Imported-From: 0813eec Imported-Base: d09b123 Imported-Merge: b83f073 Imported-Author: Mattias Jansson (mjansson) Signed-off-by: Mattias Jansson <mjansson@gmail.com> GH-URL: #164 ``` Lore-RevId: 555 Lore-Signature: 2acc021e9fee932a40cb45fd89d98db1175bdc3a98380a1cba9e3b78c542f5f7
Collaborator
Author
|
Merged |
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.
Problem
ImmutableStorehad four ways to ask whether a store held an address, each written many times for the store types, and they did not agree. The AWS store resolved the same address two ways and only one of them knew what obliteration was; the replicas sent one request under two names; the remote store downloaded a payload to answer a question about metadata; the composite carried four copies of one fan-out. Nothing said what any of it meant, so each implementation decided for itself, which is why they differed.Proposal
Query is unified with exist and becomes a batch interface, responding the best match level found in the store, with a clear contract what the store must uphold in terms of data partitioning.
Both get and metadata reads lose the match level parameter as it was mostly unused and better served by an answer of the best level found - with one
StoreGetDatacarrying the fragment, the level and an optional payload, so a caller that loads content learns whether the association was its own.The store contract is written into the trait and enforced by a battery every store runs: never over-report, may under-report, obliterated never matches, reads do not under-serve and agree with each other, and a match names where it was found.
Validation
This change also carries the implementation to show what it looks like in practice - the reduced and unified store interface along with the fixes the contract enforcement caught.