chadtmiller.com/knotless

Switch repository

Loading…

What changes

One unreadable record no longer stops a space's sync. A record in a member's repo whose value does not encode to the CID it was written with, or whose bytes are not a DRISL map, is left out of knotless-serve's index of the space (taken out, if an earlier read put it there). It is logged at warn as space record unreadable, with the space, the member's DID, the record's path (key) and why. The rest of that repo, and of the space, is synced as before.

Reading on from the oplog (space_sync::ReadOn::changes) now gives back the records that do not check out against their write's CID, rather than failing the read. A last write with no value is still a race with a later write, and still sends the repo to be read whole, as before.

Reading whole (space_sync::star) no longer fails on a record that is not a DRISL map. It hashes the record's bytes as it does any other, sets it aside as unreadable, and leaves the check against the commit to decide.

A repo whose STAR archive does not come to its commit is now read from the start of its oplog (listRepoOps with no since, from the empty set hash). Its records replace those the repo gave before, as the archive's would. This is logged as space repo read from its oplog. space_sync::read_on takes since as an Option for this.

docs/design/access.md says all this, and why.

Why

A pds.js encoder bug, fixed now, stored some dev.knotless.check records cut short: integers of 2^32 or more lost their bytes. The runner's repo in group-host's space holds 96 such records. Their stored bytes neither decode nor hash to their CIDs. Each sync then went:

1. Read on from the oplog: a record that does not come to its CID, so read whole.
2. Read whole: a record that is not a DRISL map, so the repo failed.

The space's cursor then stayed where it was, so no new check reached the repo's pulls page. One bad record hid that whole repo.

Verification. The commit, vouched for by the MAC, holds the digest of a set hash over each record's (path, CID), not over its bytes. So the question is where each CID comes from:

• From the oplog, each op carries its CID apart from the record's value. The ops' CIDs, folded into the set hash, must still come to the vouched commit, whatever the values. Each value is then checked on its own against its op's CID. A value that does not match can just be left out: nothing about the other records' check depends on it, and nothing unvouched gets in.
• From the STAR archive, each record's CID is only the hash of its bytes. One spoiled record makes the whole archive's set hash wrong, and the archive alone cannot show which record it was, or what its real CID is. So an archive that does not come to its commit is still refused, as before. In its place the oplog is read from the start. There, every CID is vouched for by the commit apart from the bytes, and bad values drop out as above.
• That read comes to the commit only if the oplog still holds every write the repo was made of. The alpha's lexicon says a host may compact it. If it was compacted, the repo is not read, which is what happens today.

An archive that does come to its commit but holds a non-map record is verified as it is. That record is simply not indexed.

I considered going record by record with getRecord's per-record MAC. knotless does not check that MAC yet. It vouches for one record's CID, not for the set the repo holds, so we would also need a listing we could trust. The oplog already gives both under one commit MAC.

Nothing here loosens what a host can pass off. Every record indexed has a value whose DRISL encoding is a CID inside a set that a MAC'd commit vouches for. The checks against the commit are unchanged.

Changeset: none

How it was checked

• space_sync unit tests:
  • Ops whose values do not come to their CID, or are not DRISL, are left out as though deleted, reported as unreadable, and the rest returned.
  • A STAR archive holding a record cut short (an integer of 5·10^9 missing its last bytes) and a non-map record reads the third record, and sets those two aside, hashed by their bytes.
• private_repositories_indexed_here (gateway, over the spaces alpha), which the request asked for first:
  • The member files check p2, which syncs.
  • The test spoils p2's bytes in the alpha's own store (space_record.value), keeping its CID, and forces the repo off its oplog as the test already does.
  • The member files p3.
  • The sync's whole read fails against the commit. It reads from the oplog's start, leaves p2 out, and indexes p3 next to p1.
  • To reach the store, harness/pds.mjs now prints the PDS's actor store directory (actors).
• telemetry notes test for the two new log lines.
• Built and tested on the runner: check and test (the coverage gate). Nothing was built locally.

Shortcomings

• The test spoils p2 with another valid map, not with undecodable bytes. The alpha's own listRepoOps fails on bytes it cannot decode, so it cannot serve that case. The undecodable case is covered by the star unit test. pds.js does serve such values (as the ones that do not come to their CID that prod logged).
• Prod's case, a record cut short in ops read on from a kept revision, goes through ReadOn::changes. It is covered by the unit test, not end to end: a test cannot spoil a write between its landing and the sync that reads it.
• A repo whose archive is spoiled and whose oplog has been compacted is still not read. Nothing it serves can vouch for its records then.
• Every sync of a repo with a spoiled archive pays a failed whole read before the oplog read, until the record is fixed or deleted. That only happens when the read on fails, too.

Review

Start with ReadOn::changes and star in crates/knotless/src/space_sync.rs, then member and from_start in crates/knotless-serve/src/spaces.rs. The verification argument above is the part to check.

🤖 Generated with Claude Code

I read the whole diff but did not run it or read the checks. I find the verification argument sound, so I approve.

• ReadOn::changes returns a mismatched value as a no-value change (deleted from the index) and reports it as unreadable. The set hash is still folded over the op CIDs and checked against the MAC'd commit in read_on, so dropping a bad value loosens nothing.
• star now hashes a non-map record by its bytes and sets it aside. An archive that does come to its commit still verifies. A spoiled archive fails the commit check, and member then calls from_start.
• from_start reads the oplog from the empty hash. A compacted oplog fails the commit check, so the repo is not read, as today.
• The last-write-with-no-value race still errors in changes and sends the repo to be read whole.

Minor, not blocking: member falls back to from_start on any error from whole, including transient network errors. That only costs an extra oplog read, and the commit check is just as strict there.

The docs page is updated, and the Changeset: none line is on a line of its own.

I still approve. Since my earlier review (cbd810e) the only change is in crates/knotless-serve/src/spaces.rs. whole and from_start now pass the commit rev to replace as &str (rev.as_str()) instead of &String. This is a type fix and changes no behavior, so the verification argument I checked before still holds. My earlier non-blocking note about falling back to from_start on any error from whole still applies. The check, test, e2e and docs runs for this commit were still queued or running when I looked, so I haven't seen them pass.