Skip to content

custodian: backfill reads a record at one inode key and CASes it at another #698

Description

@eduralph

Carved out of #695 during a re-plan (2026-08-07) so that slice stays one logical change. Latent
today, but it is a real cross-record write and a real permanent-stall path, so it wants its own
slice rather than a footnote.

The defect

custodian::backfill::reconcile reads a record at one key and CASes it at another, and conditions
that CAS on a re-encoding rather than on the bytes it read. Two independent halves, both on
origin/main @ 339da46:

1. The write key is re-derived from a parse, not taken from the store.

  • crates/custodian/src/backfill.rs:84-86 parses the scanned key to an InodeId via
    parse_inode_key (:64-70), which is str::parse::<u64> — so it accepts non-canonical
    spellings: inode:007 and inode:+3 both parse.
  • crates/custodian/src/backfill.rs:142-145 then re-derives the write key as
    metadata::inode_key(inode_id) — which can only ever spell inode:7.

So a row stored under inode:007 is read at inode:007 and written at inode:7: the pass
version-bumps and rewrites the placement of a different record than the one it classified.

2. The CAS precondition is a re-encoding, not the stored bytes.

backfill.rs:144 conditions on metadata::encode(&record) — a re-encode of what decode produced —
rather than on the value the scan returned. decode → encode is byte-identical only while every
field round-trips to the stored spelling. A record written by a build whose encoding differs in any
way therefore loses this CAS on every pass, forever: its empty placement is never filled and the
drain gauge never reaches zero, with no error and no operator signal.

Reachability

Not reachable through any in-tree path today: metadata::inode_key (crates/core/src/metadata.rs:33-36)
is the sole writer of the inode: prefix, so no code produces a non-canonical spelling. Half 2 is
reachable the moment the record encoding changes between builds. Both are latent, which is why this
is filed rather than folded into #695.

What a fix has to settle (not prescriptive)

  • Read, write and name a record under exactly the key the store gave it, and condition the CAS on
    the bytes the scan returned. Precedent: ReferenceSet::unresolvable keys objects by the store's own
    key bytes, and says why a rendering is not a substitute — crates/custodian/src/gc.rs:278-294.
  • Decide what a row the namespace cannot attribute deserves. Removing the parse also removes the
    silent skip it currently provides, so a committed row under inode:not-an-id becomes eligible to be
    filled and mutated like any legitimate object. The gateway's startup recovery already names this exact
    shape in this exact namespace rather than dropping it in silence
    (crates/core/src/metadata.rs:2153-2173, attribute_unaccounted_inode_row / UNPARSABLE_INODE_KEY).
  • Mind the ordering. custodian: backfill reads through the resolver, contained (635.4b.1) #695 round 5 found that validating the key after decoding misreports an
    undecodable non-canonical row as an object-data fault instead of the namespace fault it is. Checking
    the key first, however, changes which rows are named (a non-committed row under a bad key is skipped
    today). That interaction is the substance of this slice — it is what made the concern unfixable as a
    side quest inside custodian: backfill reads through the resolver, contained (635.4b.1) #695.

Scope note

Whoever takes this should expect to touch the same lines #695 deliberately froze. Land it after
#695 to avoid a conflicting edit to backfill.rs. The same re-derived-key pattern is worth checking in
the sibling loops (rebalance.rs, reconstruction.rs) while the context is loaded — #696 round 4
found the matching gap there.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions