docs(architecture): ADR-018 — trait signatures + storage error model (review 002 R-1/R-2)
- Pin the object-storage trait signatures (GitRefs.list_refs/apply_updates, GitPackGen.generate/common_haves, GitPackIngest.prepare) — the A-2 amendment's unfinished half (review 001 pinned only the registry pair) - repo parameter is &RepoRecord (record carries storage_root; no handle type, no second lookup) - Shared seam types pinned: RefLine (unborn-symref shape for the N-5 rider), RefUpdate, RefOutcome, PreparedPush, PushOptions - StorageError for traits 3-5; RegistryError re-scoped to the registry family (the 'every failure the trait family can produce' claim corrected) - backend.md: pinned-signatures mirror section, concurrency-model ownership bridge, boxed-stream parameter-ownership rule Verification: cargo doc/test/clippy/fmt clean (docs-only change)
This commit is contained in:
1 parent
b6040d36a9
commit
0f7d5c7309
2 files changed
+352
-6
No files matched your search
@@ -0,0 +1,240 @@
|
||||
# ADR-018: Backend trait signatures and the storage error model
|
||||
|
||||
## Status
|
||||
|
||||
Accepted (resolves review 002 R-1/R-2; amends the scope of ADR-012 §1's
|
||||
A-2 amendment ("all five traits' signatures") and corrects backend.md's
|
||||
`RegistryError` over-claim ("every failure the trait family can
|
||||
produce"))
|
||||
|
||||
## Context
|
||||
|
||||
Review 001 A-2 pinned `#[async_trait]` on all five backend traits and
|
||||
its remediation row claims "signatures per ADR-012 §1" — but ADR-012 §1
|
||||
contains only the *registry* pair (`GitRegistry`, `GitRegistryStore`).
|
||||
Review 002 (R-1) found traits 3–5 (`GitRefs`, `GitPackGen`,
|
||||
`GitPackIngest`) have no pinned signatures anywhere in the corpus: no
|
||||
method names, no shape for the `repo` parameter, no type for the
|
||||
prepared-updates handoff that `atomic` depends on, no representation
|
||||
for object ids or push options. These are exactly the
|
||||
implementation-fork hazards review 001's criticals describe: the wire
|
||||
layer and the gix impls are separate workstreams, every invented shape
|
||||
silently enters OQ-03's freeze inventory, and the trait signatures are
|
||||
the first thing both touch.
|
||||
|
||||
Review 002 (R-2) found the N-3 pin over-claims in the other direction:
|
||||
`RegistryError`'s five variants cover the *registry* family (traits
|
||||
1–2) well, but not the object-storage traits' characteristic failures
|
||||
(missing-object aborts, fsck/unpack failures, CAS-denied refs). The
|
||||
"no catch-all, every failure the trait family can produce" sentence
|
||||
cannot hold for a family it does not describe.
|
||||
|
||||
## Decision
|
||||
|
||||
### 1. The registry traits stand as pinned
|
||||
|
||||
`GitRegistry`/`GitRegistryStore` signatures are ADR-012 §1 as amended;
|
||||
nothing here changes them. `RegistryError` is re-scoped as the error
|
||||
type of the *registry family only* (see 7).
|
||||
|
||||
### 2. The `repo` parameter is `&RepoRecord` on every object-storage method
|
||||
|
||||
The wire layer resolves the repo once (ADR-007 step 2), holds the
|
||||
`RepoRecord`, and passes it. The record carries `storage_root` — the
|
||||
server-side path the gix impls open (`gix-discover` +
|
||||
`gix_odb::Store::at`, backend.md) — so the trait needs no second
|
||||
registry lookup, no registry handle on the object traits, and no new
|
||||
handle type. The never-on-the-wire rule (ADR-008) governs *wire*
|
||||
surfaces and op responses (N-3's exclusion is an op-response rule); the
|
||||
serving path is in-process by definition and already flows the record.
|
||||
|
||||
### 3. The object-id representation is `gix_hash::ObjectId`
|
||||
|
||||
`gix-hash` is always-on in the manifest (the wire layer parses pkt-line
|
||||
hex into it), so the trait family shares it rather than inventing a
|
||||
parallel oid type. The type is hash-format-parameterized internally
|
||||
(the enum carries the format tag), so the representation survives an
|
||||
OQ-05 policy change; sha1 is the only constructed format in v1.
|
||||
|
||||
### 4. Signatures
|
||||
|
||||
```
|
||||
#[async_trait]
|
||||
pub trait GitRefs: Send + Sync {
|
||||
/// Ref listing — advertisement + ls-refs response data.
|
||||
/// The full set; ref-prefix filtering is wire-side (client-driven).
|
||||
async fn list_refs(&self, repo: &RepoRecord) -> Result<Vec<RefLine>, StorageError>;
|
||||
|
||||
/// One CAS transaction per call (the atomic-correctness home, ADR-013 §7).
|
||||
/// Runs the per-ref CAS; per-ref denials are outcomes (RefOutcome), not errors.
|
||||
async fn apply_updates(&self, repo: &RepoRecord, updates: Vec<RefUpdate>, atomic: bool) -> Result<Vec<RefOutcome>, StorageError>;
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
pub trait GitPackGen: Send + Sync {
|
||||
/// closure(wants) − closure(boundary_haves) → pack streamed into sink
|
||||
/// (ADR-004's pipeline; missing objects abort per ADR-004 — never a broken pack).
|
||||
async fn generate(&self, repo: &RepoRecord, wants: Vec<ObjectId>, boundary_haves: Vec<ObjectId>, limits: Limits, sink: Box<dyn io::Write + Send>) -> Result<(), StorageError>;
|
||||
|
||||
/// The negotiation ack seam (ADR-014 §5, §2's done-round amendment).
|
||||
/// A have is recognized iff it exists in the object store; the is-commit
|
||||
/// refinement is the wire layer's ACK-line rule, not this method's.
|
||||
async fn common_haves(&self, repo: &RepoRecord, haves: Vec<ObjectId>) -> Result<Vec<ObjectId>, StorageError>;
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
pub trait GitPackIngest: Send + Sync {
|
||||
/// Unpack + index + fsck; prepares the ref updates. Never applies them
|
||||
/// (the single CAS home is `GitRefs::apply_updates` — ADR-013 §6–7).
|
||||
async fn prepare(&self, repo: &RepoRecord, pack: Box<dyn io::Read + Send>, push_options: Option<PushOptions>, limits: Limits) -> Result<PreparedPush, StorageError>;
|
||||
}
|
||||
```
|
||||
|
||||
Owned-value parameters (`Vec`, `Limits`, the boxed streams) are the
|
||||
spawn_blocking-friendly shape: the gix impls move them into their
|
||||
internal blocking tasks (backend.md concurrency model — the permit
|
||||
stays wire-side, the threads stay impl-side). Boxed
|
||||
`Send + 'static` streams are the blocking-side ends the substrate
|
||||
bridges produce (the sideband sink's `io::Write` end; the request
|
||||
body's `io::Read` end — the parked-channel bridges are ADR-005
|
||||
substrate structure, wire-layer-constructed).
|
||||
|
||||
### 5. The shared object-storage types
|
||||
|
||||
```
|
||||
/// One listed ref. `oid: None` is the unborn symref line (the N-5 rider's
|
||||
/// "symref-target with no oid"; the wire layer maps it to the zero-oid
|
||||
/// advertisement/ls-refs line). Advertisement projects oid+name only —
|
||||
/// peeled tags and symref targets are ls-refs-arg data.
|
||||
pub struct RefLine {
|
||||
pub name: String,
|
||||
pub oid: Option<ObjectId>,
|
||||
pub peeled: Option<ObjectId>,
|
||||
pub symref_target: Option<String>,
|
||||
}
|
||||
|
||||
/// One prepared/applied ref update. `expected: None` = must-not-exist
|
||||
/// (create); `new: None` = delete. Identical in the prepare handoff and
|
||||
/// the CAS call — one type across the seam (R-1's fork point).
|
||||
pub struct RefUpdate {
|
||||
pub name: String,
|
||||
pub expected: Option<ObjectId>,
|
||||
pub new: Option<ObjectId>,
|
||||
}
|
||||
|
||||
/// Per-ref CAS outcome (an outcome, not an error — a stale ref is a
|
||||
/// business result of receive-pack, ADR-013 §7–8). The reason string is
|
||||
/// the client-displayed `ng <ref> <reason>` text (server-chosen, verbatim).
|
||||
pub struct RefOutcome {
|
||||
pub name: String,
|
||||
pub result: Result<(), String>,
|
||||
}
|
||||
|
||||
/// The prepare output. `unpack` is the report's unpack leg (ok, or the
|
||||
/// `unpack ng <reason>` text); pack-level validation failures (parse,
|
||||
/// index, fsck connectivity) land here as the ng leg, not as Err —
|
||||
/// the session continues to the report per ADR-013 §8. `Err` is for
|
||||
/// substrate/infrastructure failure only (7).
|
||||
pub struct PreparedPush {
|
||||
pub unpack: Result<(), String>,
|
||||
pub updates: Vec<RefUpdate>,
|
||||
}
|
||||
|
||||
/// Parsed push-options — `(key, value)` pairs verbatim, un-interpreted
|
||||
/// (ADR-013 §11; repeated keys preserved). `None` on `prepare` until the
|
||||
/// config gate opens; opening the gate later is value-additive.
|
||||
pub struct PushOptions {
|
||||
pub pairs: Vec<(String, String)>,
|
||||
}
|
||||
```
|
||||
|
||||
Ref-name validation is *not* in these types: the deny-list and the
|
||||
`funny refname` mapping live in the transport layer (ADR-013 §9); the
|
||||
transport consumes `PreparedPush.updates`, validates names, and builds
|
||||
the `apply_updates` call.
|
||||
|
||||
### 6. The atomic flag is the call's, not the list's
|
||||
|
||||
`apply_updates(..., atomic: bool)`: non-atomic applies each update and
|
||||
reports per-ref outcomes; atomic prepares the whole transaction and
|
||||
commits it only if every update succeeds, mapping the abort to `ng
|
||||
<ref> atomic push failure` per ADR-013 §10. One flag because both
|
||||
behaviors share one transaction mechanism (§4 above); a per-update
|
||||
policy flag would be a trait redesign for a case ADR-013 already
|
||||
settles.
|
||||
|
||||
### 7. The storage error model: `StorageError` for traits 3–5
|
||||
|
||||
`RegistryError` (backend.md §Registry types and schemas) is re-scoped:
|
||||
it is the error type of the registry family (traits 1–2) — the "every
|
||||
failure the trait family can produce" sentence now reads "every failure
|
||||
the registry family can produce." The object-storage family gets its
|
||||
own error type:
|
||||
|
||||
```
|
||||
/// Object-storage family error (GitRefs / GitPackGen / GitPackIngest).
|
||||
pub enum StorageError {
|
||||
/// A requested object does not exist. Generation aborts per ADR-004
|
||||
/// (never a broken pack); fsck-level missing reachability is reported
|
||||
/// through `PreparedPush.unpack`, not this variant.
|
||||
ObjectMissing { oid: ObjectId },
|
||||
/// Structurally invalid backend input (malformed pack stream reaching
|
||||
/// the impl, an unborn/misconfigured storage root). Session-op error.
|
||||
Invalid(String),
|
||||
/// Backing-store/filesystem failure (message-only — not
|
||||
/// `std::io::Error`, which is not a stable surface; same rule as
|
||||
/// `RegistryError::Io`).
|
||||
Io(String),
|
||||
}
|
||||
```
|
||||
|
||||
| Variant | Wire mapping |
|
||||
|---|---|
|
||||
| `ObjectMissing` | sideband error band (fetch) — reason text may include the oid (the client computed it; no oracle risk) |
|
||||
| `Invalid` | protocol error band (duplex) or 4xx-class body (stateless) |
|
||||
| `Io` | session/op error, substrate-appropriate — same treatment as `RegistryError::Io` |
|
||||
|
||||
Per-ref CAS failures are `RefOutcome` values, not errors (5 above); fsck
|
||||
failures and pack-level corruption are `PreparedPush.unpack` ng legs
|
||||
(5) — both are protocol reports the session survives, which is exactly
|
||||
what separates them from the error type.
|
||||
|
||||
## Consequences
|
||||
|
||||
- **Positive:** the trait surface is fully pinned in one place
|
||||
(backend.md remains authoritative per ADR-004's amendment note) — the
|
||||
wire layer, gix impls, and any third-backend embedder now implement
|
||||
against the same signatures; the prepared-updates handoff and the
|
||||
`atomic` mechanism are type-expressible; the freeze inventory gains
|
||||
real shapes instead of prose (OQ-03); `PushOptions`' additive path is
|
||||
concrete.
|
||||
- **Negative:** frozen Rust-adjacent shapes grow (the five types and
|
||||
the method set join OQ-03's inventory — first-publish compat surface);
|
||||
`RefOutcome`/`unpacked` reason strings are the client-visible text
|
||||
surface, which is exactly as ADR-013 §8 planned but is now typed
|
||||
here.
|
||||
- **Neutral:** the manifest needs no change (`gix-hash`,
|
||||
`async-trait`, and the substrate's channel/parking primitives are all
|
||||
already carried); parameter ownership (boxed streams, owned vecs) is
|
||||
the implementation-fork prevention for the A-6 execution model.
|
||||
|
||||
## References
|
||||
|
||||
- Review 002 R-1, R-2 (the triggers), R-3/R-5/R-6 (the op-surface
|
||||
decisions this ADR's siblings pin in backend.md)
|
||||
- Review 001 A-2 (the `#[async_trait]` amendment whose scope is
|
||||
completed here), A-6 (the execution model these signatures imply),
|
||||
N-4 (the `PushOptions` parameter this ADR types)
|
||||
- ADR-012 §1 (the registry signatures — unchanged), §3 (ops over the
|
||||
store), ADR-004 (the generation/ingestion pipelines these signatures
|
||||
wrap; backend.md authoritative), ADR-013 §6–9, §11 (prepare/CAS/
|
||||
atomic/validation/push-options semantics), ADR-014 §2, §5
|
||||
(`common_haves`, done-round boundary), ADR-005 (the substrate bridges
|
||||
the boxed streams terminate), ADR-002 (the session tuples carrying
|
||||
the resolution these methods consume), ADR-008 (never-on-the-wire
|
||||
scoping), ADR-009 (`Limits` in the signatures)
|
||||
- backend.md §"The trait family" (mirror), §"Registry types and
|
||||
schemas" (the re-scoped `RegistryError` + `StorageError` table),
|
||||
OQ-03 (freeze inventory these shapes enter)
|
||||
- gitoxide: `gix_hash::ObjectId` (the shared oid type), `gix-ref`
|
||||
transactions (the CAS `apply_updates` wraps)
|
||||
Reference in new issue
Block a user